Repository navigation
feat: implement DVM allocation method for complete ranks without core… - #266
Closed
markcoletti wants to merge 1 commit into
Closed
markcoletti wants to merge 1 commit into
markcoletti wants to merge 1 commit into
Conversation
markcoletti
requested review from
Lance-Drane
and
a balanced review from Copilot
October 30, 2026 01:09
There was a problem hiding this comment.
🟡 Changes recommended
Critical issues conflate process slots with physical cores, override hardware-thread allocation settings, and leak reserved resources when task-pool validation fails.
3 open issues
What changed in this PR
This PR propagates task_cpp to ensemble prun launches and adds DVM-specific resource accounting for complete MPI ranks. The separate multi-rank launch limitation remains outside this PR’s scope.
Changes:
- Add DVM allocation planning that reserves cores per rank and checks capacity before allocation.
- Pass explicit core mapping and binding options to
prun. - Update parameter documentation and test allocation, exhaustion, validation, and release.
| File | Description |
|---|---|
| tests/new/test_task_manager.py | Tests DVM core budgets and generated launch commands across allocation modes. |
| tests/new/test_resource_manager.py | Tests complete-rank allocation, fragmented capacity, and invalid core counts. |
| ipsframework/task_manager.py | Adds prun mapping and binding options for task_cpp. |
| ipsframework/services.py | Removes the srun-only restriction from task_cpp documentation. |
| ipsframework/resource_manager.py | Adds DVM-specific allocation planning and core accounting. |
Review effort: Balanced
Comment on lines
+562
to
+563
| if nproc < 1 or not isinstance(task_cpp, int) or task_cpp < 0: | ||
| raise ValueError('DVM tasks require positive nproc and nonnegative integer task_cpp') |
| raise ValueError('DVM tasks require positive nproc and nonnegative integer task_cpp') | ||
| cpp = task_cpp or 1 | ||
| ppn = min(ppn, self.cores_per_node // cpp) | ||
| total_capacity = sum(min(ppn, node.total_cores // cpp) for node in self.nodes.values()) |
| ] | ||
| ) | ||
| if cpp: | ||
| cmd += f' --map-by slot:PE={cpp}:CORECPUS --bind-to core' |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes the problem whereby specifying
task_cppforlaunch_taskwas not properly propagated toprunwhen using ensembles.Note that specifying
nproc> 1 (i.e., more than one MPI rank) andtask_cpp> 1 (i.e., more than one core per rank) will not generate more than one rank. Nonetheless, this will be fine for Fastran to allocate more than one core per the single rank that it appears to use. Repairing this problem will be the focus of a separate issue.