Add support for PBS/Torque scheduler #1159
Conversation
| expected_ranks = v.n_nodes * v.n_ranks_per_node | ||
| if v.n_ranks != expected_ranks: |
There was a problem hiding this comment.
Might get a complaint given that you are
if not v.n_ranks_per_node and v.n_ranks and v.n_nodes:
v.n_ranks_per_node = math.ceil(v.n_ranks / v.n_nodes)
|
|
||
| # Calculate n_ranks_per_node if not explicitly set | ||
| if not v.n_ranks_per_node and v.n_ranks and v.n_nodes: | ||
| v.n_ranks_per_node = math.ceil(v.n_ranks / v.n_nodes) |
There was a problem hiding this comment.
If pbs always requires setting n_ranks_per_node (or in particular when n_ranks and n_nodes are set) I think that might best be done in pbs_instructions (based on reading it I assume it is not required, and in that case as well I think it might be best to skip this): this function implicitly ensures that either n_nodes or n_ranks is set by the end of it, and scheduler option functions tend to work with that; if this also sets n_ranks_per_node itself, it could over-specify (e.g. the potential mismatch from calling ceil).
Perhaps if n_nodes is set but not n_ranks/n_ranks_per_node then I should be setting n_ranks_per_node = sys_cores_per_node in this function. I assumed schedulers were doing that automatically; nearly all experiments in benchpark set n_ranks.
| node_spec += f":ppn={v.n_ranks_per_node}" | ||
|
|
||
| if v.n_gpus and v.sys_gpus_per_node: | ||
| if v.n_gpus > v.n_nodes * v.sys_gpus_per_node: |
There was a problem hiding this comment.
This seems like a check I should have added to determine_allocation - would you mind moving this there?
|
Sorry for the slow turnaround time. I wanted to implement the review comments and verify that the updated PR works as expected, but I haven't had time for that yet. |
2 participants
I'd like to try
benchparkon a system with OpenPBS, butbenchparkdid not support such scheduler. This PR adds compatibility with it (to the best of my understanding ofbenchpark).