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

[WIP] added missing 'partition' argument to slurm launcher #25

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

kkmann
Copy link

@kkmann kkmann commented Oct 11, 2023

Prework

Related GitHub issues and pull requests

Summary

Make the slurm partition configurable for crew_controller_slurm()

@kkmann kkmann changed the title added missing 'partition' argument to slurm launcher [WIP] added missing 'partition' argument to slurm launcher Oct 11, 2023
@kkmann
Copy link
Author

kkmann commented Oct 11, 2023

Creation of the extended class work fine, but the map() function complains about non-matching character lengths on an attribute check. Did not get to the bottom of it.
image

@@ -49,6 +49,8 @@
#' SLURM cluster. `slurm_time_minutes = 60` translates to a line of
#' `#SBATCH --time=60` in the SLURM job script. `slurm_time_minutes = NULL`
#' omits this line.
#' @param slurm_partition Character of length 1, name of the slurm partition to
#' create workers on.
Copy link
Owner

Choose a reason for hiding this comment

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

Would be great to have a mention of how this translates into the --partition flag, as in the above @param descriptions. Also how a NULL value omits the flag.

@wlandau
Copy link
Owner

wlandau commented Oct 11, 2023

Creation of the extended class work fine, but the map() function complains about non-matching character lengths on an attribute check. Did not get to the bottom of it.

You might need development crew.

@wlandau
Copy link
Owner

wlandau commented Oct 11, 2023

I am actually gearing up for a set of releases, so I will merge this now. Thanks!

@wlandau wlandau merged commit 757840f into wlandau:main Oct 11, 2023
2 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants