perf(evolution): ⚡ halve the follower-marking epoch stamp - #259
perf(evolution): ⚡ halve the follower-marking epoch stamp#259diagonal-hamiltonian wants to merge 1 commit into
Conversation
github-actions
Bot
added
documentation
|
Docs preview: https://pr-259.monoprop-docs.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #259 +/- ##
=======================================
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. |
|
🤖 AI text below 🤖 A/B campaign at this SHACampaign The result is memory, and it is consistent: peak RSS falls in 16 of 16 memory cells. 15 of them at unanimous 10/10 rep agreement.
Kernel peak RSS from Statistics. A 10/10 sign test is p = 2·(1/2)^10 = 0.00195, the floor at this rep count. Holm step-down across the 16-test memory family: 15 of 16 survive, largest surviving adjusted p = 0.031. Only the pauli B/N=2 Hubbard beats pauli, which is what a per-term saving must do. Implied bytes/term at N=1, from the node-sum delta over the cell's term count:
The stamp width saves exactly 2 B/term at Timing is flat, as intended. 0 of 24 timing tests resolved; every operation lands between 0.98x and 1.01x and nothing clears Holm at m = 24. Placement verified per cell before any of the above was read: both arms pinned 128 threads/rank at layout A and 16 at layout B, in all 12 cells, with identical term counts arm-to-arm. Slurm-confined ranks have silently defeated the engine's pinning before and every timing above it was junk. Two honest limits on this campaign1. Both arms are 2. This campaign does not measure the ReproducibilityNot reproducible from this diff. Deucalion, 2× AMD EPYC 7742 / 128 cores / SMT off / NPS4 / 242 GiB per node; |
`detail::MatchedEpochSet` carried a `uint32_t` stamp per term for a counter that never leaves the struct. This narrows it to `uint16_t` and keeps the wrap discipline intact. One commit on origin/main 48cadcb. It contains no profiling code: monoprop_PROF, profile:: and Profile.h appear zero times in the diff. RE-CUT TWICE, AND THE SECOND TIME BACK ONTO MAIN. The original was authored on perf/profile-graph-coverage @ 991042f, whose base carried the profiling instrument; its MonomialPropagator.inl hunk called profile::sample_store(...) and its AGENTS.md hunk carried two placement bullets inherited from that base. Both dropped. It then briefly based on the graph-memory PR, because that PR's tests/test_memory_breakdown.py asserts the operator breakdown key set exactly and this change adds two keys to it. Those graph counters have since moved to the profiling-tool branch -- their only consumers are the private harness collators -- so the test is not on main, and the 6-line classification hunk belongs on the tool branch beside it. The mechanism has never changed across any of this. WHAT IS DELIBERATELY NOT HERE: `matched_scratch_.shrink_to_fit()`. An earlier revision called it at the end of `initialize_operator_caches_()`, on the premise that the array "grows only when the term count does". That is false on Hubbard -- the function runs after EVERY `build_graph()` and `propagate()`, so 29 Trotter steps reach it 29 times against a term count that grows at each step. Its A/B read +0.2821 GiB of peak RSS (+3.12 B/term), 6/6, non-overlapping, at the 1x16 hubbard cell. Removing it read -0.24 GiB, 6/6. Separately, the shrink cannot free resident bytes at all: `resize(n, 0)` never writes past `n`, so the capacity it releases was never faulted in. `MatchedEpochSet::shrink_to_fit()` is retained but uncalled, and the comment says so. MEASURED AT THIS MECHANISM, campaign pr7v2, 12 cells x 10 interleaved reps: kernel peak RSS falls in 16 of 16 memory cells, 15 at unanimous 10/10 agreement, 15 surviving Holm across the memory family at adjusted p = 0.031. Hubbard propagate 0.96-0.97x, the hubbard build_graph rung 0.97-0.98x, pauli 0.99-1.00x -- hubbard beating pauli is what a per-term saving must do. Timing flat: 0 of 24 tests resolved, everything inside 0.98-1.01x. Both arms were ENABLE_PROFILE=ON builds, so the instrument was common-mode and the ratios isolate this change; the absolute figures are a profiling build's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
f1f3067 to
b01e6fa
Compare
|
🤖 AI text below 🤖 Rebased back onto Consequence for this PR: it is back to one commit on One correction to the A/B comment above, in the interest of not overclaiming: I said the arm-to-arm diff was line-for-line identical to the shipping diff. With the classification hunk no longer in this PR, the measured arms differ by this diff plus those six lines in a Python test file. A Python test cannot reach the compiled extension, so the two binaries still differ in exactly the nine files here — the ratios stand unchanged — but "identical" is now the wrong word for the diff. Gates on this tree: |
|
1 participant
🤖 AI text below 🤖
Halve the follower-marking epoch stamp (u32 → u16), and release the init-operator map
Branch
perf/epoch-stamp-noshrink-v2@b01e6fa, one commit onorigin/main48cadcb.9 files, +283/−25.
Supersedes
perf/epoch-stamp-noshrink@b29864c, which was authored on the pre-rebase PR 1(
991042f) and is provenance only. This is a re-cut, not a rebase: that base carried the profilinginstrument, so the old commit's
MonomialPropagator.inlhunk calledprofile::sample_store(...)andits
AGENTS.mdhunk carried two placement bullets belonging to another change. Both are gone. Themechanism is untouched, and this branch contains no profiling code at all —
monoprop_PROF,profile::andProfile.happear zero times in the diff.What this changes
1.
detail::MatchedEpochSet's stamp goesuint32_t→uint16_t. One stamp per term, for acounter that never leaves the struct — it is never serialised, never exchanged between ranks, and
never compared against anything but
cur_. Width is the only thing that changes. Worth 2 B/term.2. The wrap branch becomes live, and that is the reviewer's main correctness question. It now
fires once per 65535 gate applications instead of once per 2^32, so any long circuit reaches it
repeatedly. The
std::fillis what keeps it correct: without it a slot last stamped at epoch ealiases onto the next epoch e and reads as marked. Two tests pin this —
matched_epoch_stamp_wrap_resetsandmatched_epoch_stamp_wrap_reached_by_gate_count, the lattercycling a whole period rather than assigning to
cur_, which is the only way to prove the fill runsrather than that the branch is reachable.
3.
get_operator()releasesinit_op_mapinstead of draining it.erase/clearboth keepbucket_count(), andbucket_count()is whatinit_operator_bytesreports — so a fully drained mapused to hold its entire bucket array for the life of the run. Now: bind, then swap with an empty map
if everything bound, or rebuild sized to what is still pending if not. This also drops the
std::vector<Monomial>of keys the old collect-then-erase needed. What it is worth tracks theobservable, not the operator, so the two must be quoted apart: 1,189 bytes total across the Hubbard
and Pauli anchors, whose single-site observables never grow the map past its initial buckets — but
39.58 B/term, about 1.15 GB, on a workload with a 7M-term observable.
4.
matched_scratch_bytesjoins the memory breakdown, and is insidetotal_bytes(). The stamparray is propagator-owned, so
estimate_memory_usage()cannot see it and onlyMonomialPropagator::operator_memory_usage()can fill it in.init_operator_entriesis added besideit as a diagnostic, outside the sum.
total_bytes()non-comparable across the boundary. A build without the fieldreports a total lower by ~
sizeof(Stamp)per term while holding the same or more resident memory —the array existed, it was counted nowhere. Any benchmark series keyed on
total_bytesthat straddlesthis commit reads a reporting fix as a memory regression. Subtract
matched_scratch_bytes, orre-measure the baseline.
A note on where this has been based
This briefly based on #253, which added five graph memory counters plus
tests/test_memory_breakdown.py— a test that asserts the bound breakdown key sets exactly. Thischange adds two keys, so it had to classify them there, in a file that only existed on that branch.
#253 has since been withdrawn: the counters' only consumers are collators in the private benchmark
harness, so they moved to the profiling-tool branch rather than to
main. That test is therefore noton
main, and this branch is back to one commit on48cadcbwith the 6-line classification hunkliving on the tool branch beside the test it satisfies. The mechanism has not changed across any of
this — the tree here is identical to the version originally cut against
main.What is deliberately not here:
shrink_to_fit()MatchedEpochSet::shrink_to_fit()exists, is correctly shaped (gated on 12.5% dead capacity so anexact fit taken mid-growth does not realloc on the next appending gate), and is never called. An
earlier revision called it from
initialize_operator_caches_()on the premise that the array growsonly when the term count does. That is false on Hubbard: the function runs after every
build_graph()andpropagate(), so 29 Trotter steps reach it 29 times against a term count thatgrows at each step. It measured +0.2821 GiB peak RSS (+3.12 B/term), 6/6, non-overlapping, at the
1×16 hubbard cell; removing it read −0.24 GiB, 6/6.
Underneath that, the shrink cannot free resident bytes at all —
resize(n, 0)never writes pastn,so the capacity it releases was never faulted in. The earlier claim of "6–7.6 B/term of slack
recovered" was virtual. 2 B/term was always the ceiling. So the array keeps its geometric
overshoot on purpose, and the comment says so, so the next reader does not re-wire it into the gate
loop.
Measurement
Peak RSS falls in 16 of 16 memory cells, 15 at unanimous 10/10 rep agreement, with 15 of 16
surviving Holm across the memory family (largest adjusted p = 0.031). Hubbard
propagateis0.96–0.97x, the Hubbard
build_graphrung 0.97–0.98x, pauli 0.99–1.00x — hubbard beating pauliis what a per-term saving must do. Timing is flat: 0 of 24 tests resolved, everything inside
0.98–1.01x. Full tables, the implied B/term arithmetic and the caveats are in a comment on this PR.
The old table is not reused: it measured a
991042f-based pair (3326f524vs438bf9d4) and theearlier per-term figures were taken at
1053a19/4f790d8. Neither is this base, and a result belongsto the commit it measured.
The A/B pair is built so the comparison is honest about the one thing that could contaminate it:
Both arms are
ENABLE_PROFILE=ONbuilds. The stack this PR came from was developed on aninstrumented base, so rather than pretend otherwise the instrument is put in both arms, where it is
common-mode and the paired ratio isolates the stamp width. The diff between the two arms is
this diff plus six lines in one Python test file — the breakdown-key classification that the tool
branch's
test_memory_breakdown.pyrequires andmaindoes not have. A Python test file cannot reachthe compiled extension, so the two arms differ in exactly the nine files above and nothing else.
The consequence, stated plainly: the ratios are valid, the absolute wall times and GiB are a
profiling build's and are not production figures.
monoprop_PROFILEis left unset on timed cells, sothe timers never fire — the arms differ from a default build in codegen, not in armed
instrumentation.
Grid: layout A = 1 rank/node × 128 partitions, layout B = 8 × 16; N = 1 and 2; hubbard c10/
1.25e-5,pauli c14/
5e-5, plus abuild_graph[hubbard]rung at--hubbard-trotter-steps=2. 16 cells × 10interleaved reps in one allocation, order flipped per (rep, cell),
--cpu-bind=cores --distribution=block:block --cpus-per-task=128/rankson both arms. Paired per-rep ratios, median ofratios — never a ratio of medians — two-sided sign test, Holm across the family, agree count beside
every ratio.
Peak RSS is kernel
/usr/bin/time -vper rank summed over the node, measured outside the code undertest. Where the engine's own byte ledger has disagreed with the kernel before, the kernel won.
PR 3 — which adds no library code — run on this identical grid resolved 0 of 24. That is the
false-positive control.
Scope — open question for the reviewer
Three independent mechanisms in one PR: the stamp width, the
init_op_maprelease, and theledger field. They cannot be reverted along one axis. The ledger field is arguably a prerequisite for
honestly reporting the first, but the map release is genuinely separate work that happens to touch
the same header. Splitting is a judgement call, not a defect — flagging it rather than hiding it.
Gates
ctest -L unit221/221 and-L serial220/220 green on an OFF build of this tree(
6f14ecb15d5436912125f5d488f1b411), plus-L mpiand the four Python MPI layouts at 1×1, 1×16, 2×8and 8×16 — 600 passed at every layout, all suites green.
clang-format --output-replacements-xmlreports 0 replacements on every changed C++ file. The two ON measurement arms are gated the same way.
Reproducibility
Not reproducible from this diff. Deucalion, 2× AMD EPYC 7742 / 128 cores / SMT off / NPS4 /
242 GiB per node; private benchmark harness that does not ship; two prebuilt venvs with the extension
hashes above.