Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for specifying slurm output file #194

Merged
merged 2 commits into from
Mar 2, 2024

Conversation

cnrrobertson
Copy link
Contributor

As seen in #185, if an output argument (-o or --output) is added to the srun arguments, the process fails. This checks to see if an output name has been specified and if not, uses the previous default name format.

@kescobo kescobo added the SLURM label Dec 19, 2023
Copy link
Collaborator

@kescobo kescobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to me! Tests are failing, but what else is new? Thanks for adding some for this change!

src/slurm.jl Outdated Show resolved Hide resolved
Co-authored-by: Kevin Bonham <[email protected]>
@cnrrobertson
Copy link
Contributor Author

Yeah, I wasn't sure about the error message on the CI. Seemed like it is related to allocating slurm workers?

 connecting to worker 1 out of 1
srun: error: get_addr_info: getaddrinfo() failed: Name or service not known
srun: error: slurm_set_addr: Unable to resolve "slurmctl"
srun: error: Unable to establish control machine address
srun: error: Unable to allocate resources: No such file or directory

I can tell you it ran on the cluster I was using fwiw.

@kescobo
Copy link
Collaborator

kescobo commented Dec 20, 2023

Yeah, tests are failing on master too - no worries.

I'm happy to merge in ~a week if no one objects.

@kescobo kescobo merged commit ccf72ba into JuliaParallel:master Mar 2, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants