-
Notifications
You must be signed in to change notification settings - Fork 37
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
fix: Multi-node GPU summissions for greatlakes and picotte. #702
Closed
Closed
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
3f1b658
fix: Multi-node GPU summissions for greatlakes and picotte.
b-butler e5799fd
Merge branch 'master' into fix/multinode-gpu-submission
b-butler 4c4fa7b
fix: Template handling when ntasks % n_nodes != 0.
b-butler 71966d0
Merge branch 'master' into fix/multinode-gpu-submission
bdice File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only be correct if the number of nodes is evenly divisible by the number of tasks, right? Would this be an issue? If yes, should we protect against that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the tasks cannot be evenly distributed across an integer number of nodes (e.g.
nranks=13
), then there is no way to request this with--ntasks-per-node
- if you rounded up you would be providing the user with more ranks than they requested. Use--ntasks={nranks}
instead in these cases.Preserve the
--nodes=
,--ntasks-per-node=
, request when tasks can be evenly distributed across nodes. It will provide a more efficient communication pattern.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, so I think we should raise an error in case that the requested configuration cannot be provisioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently round in other templates for GPU partitions e.g.
expanse
. We could change that. Generally that won't matter since we take the ceiling and charges for GPU nodes are usually just for GPUs I if I understand correctly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, they would likely be charged correctly. However, if the users launches their app with
srun
ormpiexec
without arguments, then the autodetected number of tasks detected from the job configuration. If this is rounded up from what the user requested, then the user's script may fail (e.g. when it is coded to work with a specific number of ranks).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, in the case of GPU jobs does it make sense to have CPU tasks not be a multiple of GPUs? I feel that is something we should error at then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible for systems to support a different number of CPU tasks and GPUs. For example, NCSA Delta does:
In this test, it assigned all 4 GPUs on the first node and 1 GPU on the 2nd.
Just because it is possible doesn't need that signac-flow needs to support it. I can't think of any reasonable workflows that would need this. Also, you would need to check each system separately whether it has configured SLURM to allow for this uneven task distribution.