perf(mpi): ⚡ route every term through one GF(2)-linear router - #351
Draft
diagonal-hamiltonian wants to merge 6 commits into
Draft
diagonal-hamiltonian wants to merge 6 commits into
diagonal-hamiltonian wants to merge 6 commits into
Conversation
monoprop_ROUTING and monoprop_ROUTE_SEED select and seed the rank routing (detail/mpi/Routing.h, next commit); monoprop_COMMPROF switches the exchange's report line. All three go through EnvConfig so the environment is parsed once per process and read the same way everywhere. Invariant: these three THROW on a malformed value instead of falling back. Each silently changes the transport or what is reported, so a typo that defaulted would stay invisible until a postmortem -- unlike monoprop_NUM_THREADS, where ignoring a bad value only loses a hint. Assisted-by: ClaudeCode:claude-opus-5
… ranks
Routing was two inline copies of `monomial_hash(M) % P` -- one in Scan.h's query
emission, one in find_rank -- which have to agree exactly or ownership splits
silently. Both now go through routing::Router::dest and nothing else; the
rank-count overload of find_rank is deleted rather than kept, because it would
answer splitmix during a linear run, which is that split.
The reason for the abstraction is that splitmix's full avalanche is what makes
the exchange dense: a gate maps M to M^G, so with an avalanching owner function
a rank's queries for ONE generator spray across all R ranks and the message
count grows as R*(R-1). Taking the rank index from a GF(2)-linear function of
the support instead gives h(M^G) = h(M) ^ h(G), so every query a rank emits for
one generator lands on one peer. Nothing exploits that yet -- the transport is
still the dense pair -- this makes the structure exist and proves it is free.
Two levels, because they cost differently: across ranks the message count is
what hurts, within a rank partitions talk through shared memory where fanout is
free and only balance matters. rank = fp & (R-1); part = the next log2(S) bits
of fp when S is 2^k, else mix64(fp) % S.
Invariants:
* linear or not is a switch, not a dial: the rank takes every bit of the
linear hash or none of them. None of them is splitmix, bit-for-bit the map
this commit replaces, and R == 1 is that case by construction -- so a
single-rank run is unchanged, and so is any run under
monoprop_ROUTING=splitmix.
* R > 1 under linear routing must be a power of two, or the geometry is
REJECTED (UnroutableGeometry) at construction. A fallback taken on some
ranks only is a deadlocked exchange, not a wrong answer.
* check_routing_agreement lands in this commit, not a later one, for the same
reason: a knob that did not reach one rank deadlocks the first exchange, so
it is converted into an exception by two allreduces at construction. Two
independent digests, because allreduce_sum is the only collective in the
tree and a sum is not an equality test.
At R > 1 with linear routing a term's owning rank changes, so per-term values
are identical and the row layout differs; term counts, being
topology-independent, do not move.
mpi::geometry() is new because size() alone cannot tell an inter-rank message (a
network hop) from an inter-partition one (a memcpy), and the split is exactly
what the routing needs. It is exported: public template chains reach it from
headers across the hidden-visibility boundary.
Assisted-by: ClaudeCode:claude-opus-5
Linear routing rests on one condition: the per-generator rank shifts must span log2(R) dimensions over GF(2). If they span only rho < log2(R), the ranks a query can reach are a coset of a rho-dimensional subspace and R - 2^rho ranks receive nothing at all. That is a load-balance failure and not a wrong answer -- every term still lands on exactly one owner -- so it is a stderr warning (COMMROUTE, one greppable rank-identified line) and not a throw, and it sits at the head of the gate loop rather than beside check_routing_agreement, because at construction the gate list does not exist yet. One line per rank per propagator: only partition 0 speaks, and only once whatever the outcome. Measured on the production point it holds with room to spare: rank 32 over the 60-site Hubbard's 416 distinct shifts, against the 7 bits R = 128 needs. Assisted-by: ClaudeCode:claude-opus-5
…nerator An empty Majorana generator anticommutes with nothing, so the scan returns on its empty fold-column set having produced no query, no cosine block and no swept coefficient -- work done to discover it was not needed -- and run_exchange's three collectives per pass fire on payload size zero all the same, two passes per gate. These generators are not an edge case: a gate whose every term falls below its atol expands to the identity monomial, and a zero chemical potential alone contributes 60 of the 60-site Hubbard's 476 generators per Trotter layer. `gen.any()` is unanimous -- the generator list is replicated on every rank -- so skipping needs no agreement and no collective to decide it. This is not gate fusion: no gate is merged, a no-op gate is simply not exchanged for, and the layer is still built, so the graph's gate and parameter bookkeeping is untouched. finish() reads neither the query nor the response streams, and an untouched CosineWordBuilder finishes to exactly the empty CosMask the skipped scan would have produced, so the skipped path lands in the state the empty-payload exchange left it in -- bit-identically. Assisted-by: ClaudeCode:claude-opus-5
The routing change flips a shipped default and adds two environment variables, none of which appeared anywhere in docs/. The rank hash is a homomorphism of the group the gate acts by, so state the identity, what follows from it (fanout, the involution, cosets of the kernel), and the one condition it rests on -- that the generator shifts span the rank space, which is a load-balance property and not a correctness one. Distinguishes the scheme from the block-sum-mod-N map of [@Broers2025-or]: that sum is additive modulo the rank count while the gate acts by XOR, so carries leave the destination dependent on the operand's bits and only bound the fanout. Assisted-by: ClaudeCode:claude-opus-5
check_routing_agreement was called near the top of the constructor, before the schrodinger_cutoff ceiling check. Every partition rejects that setting, so one could be unwinding its own throw while its peers were still inside the agreement collective: ShmComm poisons on a peer throwing inside a collective, and the caller was then told "ShmComm poisoned: a peer partition threw during a collective" instead of which setting was wrong. Intermittent, ~1 run in 9 of the Python suite, and only with more than one partition -- the propagator built on MPI.COMM_SELF takes one partition per core, so it is the default path on a 16-core box. The check now runs as the constructor's last act, after every deterministic validation. It is still before the first exchange, which is all its contract needs: seeding communicates nothing, so a rank that resolved a different router has merely claimed the wrong share of the initial terms by the time this throws. Invariant, and the reason for the comment at the call site: a collective in the constructor must follow every validation the constructor can fail on. The regression test constructs at four in-process partitions and asserts the message, eight times over: the ordering is a race, so one attempt is not evidence. Assisted-by: ClaudeCode:claude-opus-5
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
documentation
|
Docs preview: https://pr-351.monoprop-docs.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## perf/stack-1-chunked-store #351 +/- ##
===========================================================
Coverage 97.70% 97.70%
===========================================================
Files 14 14
Lines 742 742
Branches 98 98
===========================================================
Hits 725 725
Misses 12 12
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
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
1 participant
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
Rank routing was duplicated inline at more than one call site. This PR consolidates it into a
single
routing::Router(detail/mpi/Routing.h) with one default scheme: a GF(2)-linearterm→slot map, resolved per call from a
fingerprint_positionsfold of the monomial's positions.monoprop_ROUTING=splitmixstays a live fallback (monomial_hash % (R × S)) for a rank count thatis not a power of two, where the linear scheme has no XOR structure to use; under the new default,
such a rank count now throws instead of silently routing.
EnvConfig.hcentralizes the knobs the engine reads at runtime (ROUTING,ROUTE_SEED,COMMPROF,NUM_THREADS,PARTITIONS— deliberately notMEMPROF/TIMEPROF, which stay localdiagnostics per the owner's decision and do not land in this stack). The router is resolved per
call (
build_layer, propagator construction,update_initial_operator, the coverage report)rather than held as a
MonomialPropagatormember, so downstream signatures are unchanged and PR 3and PR 4's diffs against this PR stay small.
This is PR 2 of 7, based on
perf/stack-1-chunked-store. The router is neutral atR=1and thetip of every A/B row below confirms it. The next PR (
perf/stack-3-sparse-exchange) uses therouter's peer geometry to narrow the exchange to reachable peers.
Changes
Engine
cpp/monoprop/detail/EnvConfig.h(new, 140 lines): parses the routing/thread/partition knobsin one place.
cpp/monoprop/detail/mpi/Routing.h(new, 279 lines):Router,linear_hash,fingerprint_positions,gf2_rank,UnroutableGeometry.cpp/monoprop/detail/mpi/MPIUtils.h:find_rank(mono, Router),router_for,check_routing_agreement(a collective that confirms every rank resolved the same geometry).cpp/monoprop/detail/mpi/MPICompat.h/.cpp: geometry export used by the agreement check.cpp/monoprop/detail/evolution/layer_build/Engine.h,Scan.h: destination calls take theRouter;Scan.htakes it in place of a bare rank count (rank_count = router.flat_world()).cpp/include/monoprop/MonomialPropagator.h/.inl: seeds and resolves the router, reportsrouting coverage;
c310423skips the scan and exchange outright for an identity generator.Tests
cpp/tests/env_config_tests.cpp(new),routing_tests.cpp(new, 429 lines: the linear/splitmixequivalence and the non-power-of-two refusal).
cpp/tests/mpi_utils_tests.cpp: extended to both routers, plus acheck_routing_agreementcaseexercised by the
mpiexec -n 2ctest variant.tests/test_parameter_validation.py:test_schrodinger_cutoff_error_survives_several_partitions(the flake-fix regression test, see Notes below).
Docs
docs/content/docs/features/parallelism.mdx: "### Rank routing" section and env table rows.docs/bibliography.bib,docs/content/docs/references.mdx: the routing citations.AGENTS.md: one Notes bullet documenting thesplitmixfallback and its refusal behavior.Measurements
Gated multiset (not positional) vs origin/main c5e88c8's raw coefficient bits, because routing
moves the row → rank assignment at
R > 1, which moves the raw dump's positional layout eventhough it changes no value and no term set. gate record md5
33275f8b1563c2b646c6d11f5aba7779(rebased onto PR 1 at19bd012).3 interleaved reps, ratios only:
All three rungs run at
R=1, where the router is a no-op by design; st2 is within noise of st1 onevery row (time and peak both), which is the expected result for this PR — it is neutral until a
later level exercises
R>1.Notes for reviewers
"linear routing needs a power-of-two rank count, got 3 ..."— where main previously routedsilently under its inline scheme. This is deliberate and documented (
parallelism.mdx,AGENTS.md);monoprop_ROUTING=splitmixis the escape and is exercised atn=3in--with-mpipytest.EnvConfigparsescomm_profile/parse_bool, but nothing reads it yet — its consumer,report_comm_profile_, is PR 5's — somonoprop_COMMPROFis deliberately not documented inparallelism.mdxuntil then.routing_tests.cppdoes not carrySlotWindow/PeerPlan/WindowVeccases; those types landwith PR 3's
Comm.h.n=4--with-mpipytest run failed with one rankexit 1. Root cause:
check_routing_agreementsat near the top of the constructor, before theschrodinger_cutoffceiling check, so one partition could unwind its throw while peers were stillinside the agreement allreduce, poisoning
ShmCommand reporting "ShmComm poisoned" instead ofthe real validation error. Fixed in
19bd012by moving the agreement collective to theconstructor's last act, after every deterministic validation; the contract is unchanged (it still
fires before the first exchange, and seeding communicates nothing, so a rank with a divergent
router has only claimed the wrong share by the time it throws). Verified with 10/10 clean
mpiexec -n 4--with-mpipytest runs after the fix, against 1 failure in 9 before it.Checklist
docs/,CONTRIBUTING.md) if neededCHANGELOG/ release notes updated if applicable (n/a — the repository has noCHANGELOG)AI/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.