chore(benches): four operations, three measures - #316
Merged
Conversation
diagonal-hamiltonian
requested review from
fpietra,
ludmilaasb and
robertodr
as code owners
August 31, 2026 12:09
github-actions
Bot
added
documentation
This was referenced Aug 31, 2026
diagonal-hamiltonian
marked this pull request as draft
August 31, 2026 12:12
|
Docs preview: https://pr-316.monoprop-docs.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #316 +/- ##
=======================================
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. |
`benches/` carried three benchmarks outside the measured set and a fourth tracked measure nothing reads. Both bench modules now measure the same four operations -- `build_graph`, `propagate`, `energy`, `gradient` -- so a number means the same thing whichever problem produced it. Deleted, never renamed (a benchmark name is Bencher's history key, so the surviving names are byte-identical to main's and no series is orphaned): - `test_random_pare` -- outside the set - `test_random_inplace` -- `propagate` followed by `energy`, both already benchmarked - `test_model` -- `test_model_propagate` plus a trailing `expectation_value` `resting-memory` is dropped from Bencher and its `report.py` section removed, and with it the `memrest` / `membase` recordings: nothing reads them, in this repository or in the out-of-tree A/B harness. The operator-memory ledger (`opbytes`, `opmem`, `opmembreak`, `opmemdelta`, `opmempeak`, `opmembase`) **stays** -- unrendered here but read by that harness, which `benches/results/README.md` now says. `report.py`'s `_display_op` collapsed every `test_model_*[hubbard]` and `test_model_*[pauli]` row to one label, so the two models were indistinguishable in the report. A parameter that is not a picture now names the model and replaces the group. `bench.yml` drops the `resting-memory` threshold and uploads `bmf.json` plus the raw results, so a failed Bencher upload no longer loses the measurement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
diagonal-hamiltonian
force-pushed
the
chore/bench-simplify
branch
from
September 1, 2026 07:52
906e319 to
b57c82a
Compare
diagonal-hamiltonian
marked this pull request as ready for review
September 1, 2026 07:53
robertodr
reviewed
Sep 1, 2026
robertodr
reviewed
Sep 1, 2026
robertodr
reviewed
Sep 1, 2026
robertodr
reviewed
Sep 1, 2026
robertodr
requested changes
Sep 1, 2026
robertodr
left a comment
robertodr
left a comment
Member
There was a problem hiding this comment.
Thanks. Please look at my suggestions.
Co-authored-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com> Signed-off-by: Aaron Miller <61472721+diagonal-hamiltonian@users.noreply.github.com>
Co-authored-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com> Signed-off-by: Aaron Miller <61472721+diagonal-hamiltonian@users.noreply.github.com>
Co-authored-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com> Signed-off-by: Aaron Miller <61472721+diagonal-hamiltonian@users.noreply.github.com>
Co-authored-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com> Signed-off-by: Aaron Miller <61472721+diagonal-hamiltonian@users.noreply.github.com>
robertodr
approved these changes
Sep 1, 2026
|
diagonal-hamiltonian
added a commit
that referenced
this pull request
Sep 2, 2026
🤖 _AI text below_ 🤖
`benches/LADDER.md` — *the benchmarking ladder for sensitive PRs* — is
five groups of benchmark configurations at the sizes the library is
actually used at. Each row gives the exact pytest flags and the `-k`
selector that produce it, so a group is a handful of ordinary `just
bench` invocations. Nothing runs these, and no benchmark gates a PR.
```bash
just bench L1-hubbard-branch --hubbard-cutoff=10 --hubbard-lower-atol=4.2e-05 \
-k "test_model_propagate and hubbard"
```
| group | shape | what it is for |
| --- | --- | --- |
| **L1** — one thread | 1 rank, 1 partition | hubbard and pauli
`propagate` at ~10M terms; random `gradient` at ~20M, paired with and
without `--pare-threshold` |
| **L2a** — one node, one rank, ~1B terms | 1 rank, all cores | all four
operations over three models and both pictures |
| **L2b** — one node of ranks, same problems | `N`=1, `R × P` = cores |
MPI with no network in it |
| **L3** — several nodes, same problems | L2b's `R` and `P`, your `N` |
the same shape at `N` > 1 isolates the network |
| **L4** — strong and weak scaling | your `R`, `P`; `N` = 1…64 | hubbard
`propagate` on one size sequence; both ladders built from it |
## What the review asked for
**Gradient rows at 20M terms, with and without `pare_threshold=1e-10`.**
`--pare-threshold` did not exist; it is now a pytest option threaded
into `expectation_value_functional` and
`expectation_value_and_gradient_functional` for both the random and the
fixed-model benchmarks. Unset reproduces today's behaviour exactly, so
the Bencher series are unbroken. Measured at 1 rank / 1 partition / 1
thread (`_core.so` md5 `b201ec4`), `--obs-terms=295000` landing
19,902,244 terms:
| row | ~s (2 reps) | ~GiB |
| --- | ---: | ---: |
| random `gradient` | 11.05, 11.02 | 2.5 |
| random `gradient`, `--pare-threshold=1e-10` | 0.75, 0.75 | 2.8 |
Hubbard is not a candidate — 29 successive `build_graph` calls retain 29
layer-sets and `skip_if_graph_will_not_fit` skips it — so the gradient
rows use the random problem, whose size is directly dialable. Proved the
knob reaches the engine rather than being swallowed: `1e-10` leaves the
energy bit-identical (the point of a retention cutoff), `1e10` moves it
0.8049 → 0.8307.
**L3's ratios removed.** The "against L2" column is gone; L3 reads like
L2 and L1 — flags, `-k`, terms, `~s`, `~GiB/node` — with the shape those
cells were measured at stated underneath (`N`=4, `R`=8, `P`=16, medians
of two reps) rather than as a prescription.
**L3 and L4 generalised, and the shape made mandatory.** `N`, `R` and
`P` are the caller's throughout, and `benches/conftest.py` now **refuses
to start a session of more than one rank with `monoprop_PARTITIONS`
unset**. The engine's own default is `ranks == 1 ? cores : 1`, so an
unset knob measured one partition per rank at a plausible wall time — a
trap the document used to merely warn about. Every rank raises after the
collective `_nodes()`, so nothing is left in a collective. One-rank runs
— `just bench`, `bench-smoke`, `bench-ci`, every CI workflow — are
untouched.
**The shape recorded where the benchmarks record.** `meta` already
carried `ranks`, `nodes`, `ranks_per_node` and `monoprop_threads`, with
report columns for each — but `partitions_env` was written *only when
the env var was set*, so an undeclared partition count and an unrecorded
one both rendered `—`. It is now recorded unconditionally, `"unset"`
when absent.
## Model sizes are inputs now
| was | now |
| --- | --- |
| `--pauli-num-qubits` accepted any value while `HEAVY_HEX_TOPOLOGY`
stayed the fixed 127-qubit IBM Eagle map — silently a different model,
or an index past the operator | raises, naming the topology as the
reason and pointing at `--pauli-lower-atol` for sizing |
| `--hubbard-observable-site` above `--hubbard-num-sites` — a documented
"trap" that produced a wrong observable | raises |
| a mode count above the extension's compile-time `MAX_NUM_MODES` failed
deep inside the extension | raises, naming the limit and the cmake
define |
| `--pauli-observable-qubit` outside the register | raises |
`LADDER.md` gains a *Model size knobs* table covering these plus
`--obs-terms`, which is an upper bound rather than an exact count:
monomials are drawn independently and duplicates collapse, by about
`obs_terms / 2·C(2·num_modes, gen_length)`. Checked against a direct
count at 142 modes — 0.03/0.19/0.37% measured at 200k/1M/2M draws
against 0.04/0.19/0.38% predicted — giving 0.06% at L1's 295k and 2.8%
at L2's 14.75M. Deterministic for a fixed `--seed`, so the calibration
reproduces.
## Also
- `just bench-mpi` never forwarded `monoprop_PARTITIONS` or
`monoprop_NUM_THREADS` through `mpiexec -x`, though its own doc comment
showed `monoprop_NUM_THREADS=2 just bench-mpi …`. Both are forwarded
now, guarded so an unset one is not an mpiexec error.
## Traps, each from a failure it caught during calibration
- **`srun --cpu-bind=cores` with no `--cpus-per-task` on the `srun`
confines each task to one core.** Measured in one allocation: no flags →
128, `--cpus-per-task=128` → 128, `--cpu-bind=cores` alone → 1. ~100x,
with nothing in the timing to say so.
- **The report runs outside `srun`.** Under it, one process per rank
races to write the same file. The two JSON artifacts are safe: only rank
0 writes them.
- **A two-operation row's peak is the MAX over its operations, never the
sum.** `HighWaterMark` resets `VmHWM` per benchmark, so both windows
contain the same resident operator.
- `--cpu-bind=none` cost 1.45x; `nproc` lies inside a job.
- The `monoprop_PARTITIONS` and observable-site traps are no longer
traps — both now raise.
## The L2a / L2b split
`L2` → `L3` moved two things at once — multiple processes *and* the
network — so anything that
appeared at L3 was attributable to neither. L2 splits: **L2a** is the
old L2 unchanged (one rank,
`partitions = threads = cores`), and **L2b** is L3's shape at `N`=1.
Each edge of the ladder now
moves one thing: partitions, processes, the network, then node count.
L2b is measured at `N`=1, `R`=8, `P`=16 on a 128-core node, two reps
with the cell order flipped
(job 1862701, `_core.so` md5 `b201ec44`). Term counts reproduce L2a's
and L3's **exactly** —
1,001,661,534 / 985,970,588 / 948,937,993 / 597,445,055 — which is the
geometry-independence check
the rung relies on.
`8 × 16` is stated as a choice, not derived as an optimum: one rank per
NUMA domain on this
machine, and L3's shape, so `L2b` → `L3` differs only in the node count.
The section says so rather
than implying a sweep.
Two findings recorded next to the table:
- **The per-rank cost that is not a share of the operator is 3.6–8.1 GiB
on the Heisenberg rows and
0.2–0.6 GiB on the Schrödinger ones**, on one shape, one node and one
binary. The flag sets
differ in the observable — 14.75M terms against 200k. The raw observable
is well under a GiB, so
something indexed by it scales with it; that is not attributed further
here.
- **L2b, not L2a, is the memory-worst rung for the Heisenberg rows**,
because the per-rank cost is
paid `R` times with no extra nodes to spread it over. An earlier draft
of this change claimed a
row fitting L2a fits L2b; the measurement refutes it and the claim is
gone.
`~GiB/node` is a sum over ranks, as L3's column is, so reading it
against L2a's single-process peak
would be a sum-against-max comparison — the error that produced a
phantom 2.3× in this project
before. The table says so. Wall time is *not* compared across the two
rungs: the same 1×128 work
measured 140 s in L2a's job and 176 s in another allocation, which is
exactly why that comparison
needs both arms in one job.
Also in this push: a new trap — `P` above the cores a rank can see,
which is what carrying L2a's
`monoprop_PARTITIONS` into an L2b launch does — and the multi-rank
skeleton now names the
interpreter directly, because `uv run` re-resolves the environment in
every rank.
**Stacked on top:** #323 rebuilds the continuous benchmarking around
these rungs.
## Verification
- `uv run pytest`: **622 passed, 8 skipped** — unchanged from #316.
- `prek run --all-files` over every changed file: clean.
- The multi-rank gate, on a 2-rank job: unset → `ERROR:
monoprop_PARTITIONS is unset on a run of 2 ranks…` from both ranks, exit
4; set to 8 → runs, and `{"ranks": 2, "nodes": 1, "ranks_per_node": 2,
"partitions_env": "8", "monoprop_threads": "8"}` lands in
`results/<label>.json` and in the report's Configuration table. One rank
with the knob unset still runs.
- Each rejection exercised through the CLI: `--pauli-num-qubits=100` and
`--hubbard-observable-site=70` each fail with the message naming the
constraint.
- Every pytest flag in `LADDER.md` checked against `pytest benches
--help`; all present. Every `-k` run through `--collect-only`; each
selects exactly the intended node ids.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Signed-off-by: Aaron Miller <61472721+diagonal-hamiltonian@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
2 participants
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.
Reduces
benches/to what a performance review reads: execution time and peak memory, over four uniform operations. First of two; #311 adds the calibrated rung table on top.What goes
test_random_pare,test_random_inplacetest_modeltest_model_propagateunder a name that collapsed both models to one label in the reportresting-memorymeasurebmf.py, the report, and the workflow thresholdLeaves
build_graph,propagate,energy,gradientuniform across both bench modules, and exactly three tracked measures:latency,peak-memory,terms.Deletions, never renames. Benchmark names are Bencher's history key. Verified against
main:So no surviving series is orphaned or silently re-keyed.
What stays, deliberately
Every
opmem*/opbytesrecording inconftest.py. They render nowhere, but the out-of-tree A/B harness reads them —benches/results/README.mdnow says so, since "unrendered" read as "dead" to me on first pass.Also
bench.ymluploadsbmf.jsonandbenches/results/*.jsonas artifacts, so a CI benchmark run is inspectable after the fact._display_opfix: a non-picture parameter now names the model, so[hubbard]and[pauli]stop sharing a row label.Verification
pytest: 622 passed, 8 skipped — identical tomain, sincebenches/is not in the default testpaths.monoprop-bench-bmfemits exactlylatency,peak-memory,terms.prek run --from-ref origin/main --to-ref HEADclean.🤖 Generated with Claude Code