feat(partition): 📈 report the launcher's placement, and delete the pinning knob - #269
Draft
diagonal-hamiltonian wants to merge 1 commit into
Draft
feat(partition): 📈 report the launcher's placement, and delete the pinning knob#269diagonal-hamiltonian wants to merge 1 commit into
diagonal-hamiltonian wants to merge 1 commit into
Conversation
…nning knob
--cpu-bind=none and --cpu-bind=cores give the same thread count and the same partition count, and
differ only in the mask the kernel enforces -- a difference that has already cost this project a
1.45x tax it could not see. A rank holding 16 of a host's 128 CPUs is equally "Slurm gave me my own
16" and "eight of us share these 16", and sampling /proc cannot tell them apart: it describes the
process it runs in, never a peer rank on the same host.
PartitionGroup already exchanges the affinity masks to choose a placement, so monoprop_COMMPLACE
reports what that exchange saw -- one line per rank on stderr, masks=private vs masks=shared being
the distinction the whole thing exists for. Default off, and nothing below the flag is computed when
it is off.
The flag gates PRINTING only. The mask exchange is deliberately not gated on it: the environment is
per-rank, so a predicate over it is not rank-uniform, and one rank skipping a collective its peers
entered is a hang, not a missing diagnostic. emit_place_line takes the flag rather than reading it, so
a test binary launched without it set can still reach the emitting path.
monoprop_PARTITION_PINNING is deleted and pinning is unconditional. Its parser matched only the first
CHARACTER against {0,f,F,n,N}; `o` is not in that set, so `off`, `OFF` and `disabled` all parsed as
ON. No campaign ran on the wrong arm -- no harness script ever set it. parse_env_flag compares whole
words, case-insensitively, against 0|false|no|off, which is the one thing the bug was about; bare `f`
and `n` change from falsey to truthy as a consequence, and a test pins that.
The knob can be deleted rather than fixed because letting the launcher own placement was refuted by
its own falsifier: propagate[hubbard] at layout A/N=1 measured 2.90x slower, 10 of 10.
Mutation-verified: restoring first-character parsing fails the whole-word case.
|
Docs preview: https://pr-269.monoprop-docs.pages.dev |
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.
🤖 AI text below 🤖
Summary
Adds a report-only diagnostic for the CPU placement the launcher actually gave each rank, and
deletes the knob that made pinning optional.
feat/commplace-placement-reportbf61814, 1 commiton
e0c528e, 9 files +464/−59.monoprop_COMMPLACEreports the placement the launcher actually gave each rank. OneCOMMPLACEline per rank on stderr at propagator construction, naming the CPUs the launchergave that rank and whether co-located ranks got disjoint masks (
masks=privatevsmasks=shared); default off, and nothing below the flag is computed when it is off. Thediagnostic earns a knob because
--cpu-bind=noneand--cpu-bind=coresgive the same threadcount and the same partition count, differing only in the mask the kernel enforces — a
difference that has already cost a 1.45x tax nobody could see. A rank holding 16 of a host's
128 CPUs is equally "Slurm gave me my own 16" and "eight of us share these 16", and sampling
/proccannot separate them: it describes the process you are in, never a peer rank on thesame host.
PartitionGroupalready exchanges the masks to pick a placement, so this reportswhat that exchange saw. The flag gates printing only: the mask exchange is deliberately not
gated on it, since the environment is per-rank, so a predicate over it is not rank-uniform, and
one rank skipping a collective its peers entered is a hang, not a missing diagnostic.
emit_place_linetakes the flag rather than reading it, so a test binary launched without itset still reaches the emitting path.
monoprop_PARTITION_PINNINGis deleted; pinning is now unconditional. The old parsermatched only the first character against
{0,f,F,n,N}.ois not in that set, so0/false/noworked whileoff,OFFanddisabledall parsed as ON. Thefirst-character match was deliberate and had a test asserting it — the bug is the vocabulary it
produces.
parse_env_flagnow compares whole words, case-insensitively, against0|false|no|off. Deliberate consequence: barefandnwere falsey and are now truthy; atest pins that, and is mutation-verified — restoring first-character parsing fails it. No
campaign ran on the wrong arm: no harness script ever set the variable, verified across the
whole harness repo, so nothing needs re-auditing.
Not shipped: the placement deletion this was salvaged from. Its pre-registered falsifier was
abandon if >5% cost anywhere, and
propagate[hubbard]at layout A/N=1 came in 2.90x slower, 10of 10, with 11 of 12 layout-A tests slower. Refuted by its own criterion — which is why the knob
can be deleted rather than fixed. (Correcting an earlier draft of that work:
mainplaces atboth layouts, 128 at A and 16 at B.)
Changes
monoprop_COMMPLACE: oneCOMMPLACEline per rank on stderr at propagator construction,reporting the launcher's actual CPU mask and
masks=private/masks=shared; off by default,nothing computed when off.
monoprop_PARTITION_PINNINGdeleted; pinning is now unconditional, andparse_env_flagnowmatches whole words instead of first characters.
docs/features/parallelism.mdxswaps the row and states pinning is not configurable, with thereason.
Measurement
Neither piece can move a number: the diagnostic is report-only and off by default, and the deleted
knob was never set by any harness script. No campaign ran, and none is presented here.
Gates:
ctest -L unit222/222,-L serial221/221, and 592 passed at each of four MPI layouts.Relationship to the open PRs
Fully independent — zero overlapping files with either #259 or #263, so no stacking and no
merge interaction in any order. The refuted placement deletion it is salvaged from is unrelated to
the placement fix that merged as #249.
Checklist
docs/,CONTRIBUTING.md) if neededCHANGELOG/ release notes updated if applicableAI/LLM disclosure
Important
By opening this PR I confirm that I have read CONTRIBUTING.md and I agree to the terms of the Contributor License Agreement.
Warning
If you're contributing on behalf of your employer, contact cla@algorithmiq.fi to arrange a Corporate CLA.