perf(evolution): ⚡ carry cross-rank queries as a width-adaptive record

[diagonal-hamiltonian]

🤖 AI text below 🤖

Carry cross-rank queries as a width-adaptive record

pr/query-wire-v3, on main 999caf18. The layer-build query path carried one dense word per 64
modes per query, at a fixed stride. This replaces it with a per-term-minimal record and stops the two
paths that never needed the wire from using it.

What changes

1. Cross-rank queries travel as a width-adaptive record. The fixed stride the old CompactQuery
   design assumed is gone; a record is gap-coded ascending positions, sized by its own popcount.
2. A rank resolves its own queries from positions, not through the cross-rank path.
3. Sink::kStride becomes incoming_form() / querier_form(), fixing a latent inconsistency:
   ContractSink::kStride was the fused width while on_response_block got the plain buffer and
   used the plain-stride default — correct only by accident. A wrong form reads a neighbouring
   record's phase, which is a coefficient sign flip, not a crash.
4. bulk_insert gains a hashed entry point, so a miss found by position reuses the hash the
   probe already folded instead of rebuilding a monomial to re-fold it.

1 and 2 ship together because they share the OperatorIndex position store.

Timings

Arms: main 999caf18 — this branch's own merge-base — against this branch, both
ENABLE_PROFILE=OFF. Extension md5s b201ec44 (main) and e39ec0db (branch).

port/main, so every row below 1.00 is this branch faster. 10 interleaved reps per cell in one
allocation, order flipped per rep; ratios divided per rep then median, never a ratio of medians —
which is why the ratio column does not equal the two median columns divided.
rung is lower_atol, with Trotter steps where they differ, and separates cells that are different
model sizes. Every p is a raw two-sided sign test.

Production layout (8 ranks x 16 partitions, 1 node)

operation	layout	rung	main (ms)	port (ms)	port/main	agree	p
build_graph[hubbard]	B_8x16	1e-04/2st	2752.8	1926.6	0.70x (1.43x faster)	10/10	0.00195

Single thread (1 rank x 1 partition)

operation	layout	rung	main (ms)	port (ms)	port/main	agree	p
build_graph[hubbard]	X_1x1	1e-3/2st	41102.6	31110.0	0.76x (1.32x faster)	10/10	0.00195
propagate[hubbard]	X_1x1	3e-5	48034.0	49811.2	1.04x	9/10	0.0215
build_graph[pauli]	X_1x1	1e-4	542.0	615.8	1.13x slower	10/10	0.00195
propagate[pauli]	X_1x1	1e-4	408.9	489.9	1.20x slower	10/10	0.00195

What these cells show, and where they do not

The production win is the point, and it is intact. build_graph[hubbard] at B_8x16 runs 1.43x
faster, 10 of 10 reps.

build_graph wins at one rank too, and not for the reason an earlier revision of this description
gave. That revision credited the X_1x1 gain to an inline (k, d) cutoff digest. This branch
deletes that digest path outright, and the cell got faster — 0.76x here against 0.899 there. So
the digest was not what made it win; it was the record, and plausibly the merge losing the d
bookkeeping from its inner loop. The digest bought ~0.8% of build_graph at 128 partitions, below
its own noise floor, and it is gone.

propagate at one rank still costs. Pauli is 1.13x / 1.20x, 10 of 10 each. The record's fixed
per-query cost needs something to amortise against, and at one partition there is no wire traffic to
save. This is not a narrow-system effect — the axis is amortisation, not width. It is also smaller
than it was: those cells were 1.43x and 1.54x before the follow-up commits and this cleanup.

A correction to RESULTS-qpos-stack.md. That file reads as though the follow-up commits removed
the X_1x1 regression. They reduce it by roughly a quarter, and it is still there. The stronger
reading came from composing a stack-vs-#263 ratio onto a #263-vs-main one; the two campaigns have
different baselines, so the product is not a measurement. The rows above are measured directly.

Scope. Four cells, chosen to answer two questions: does the production layout still win, and what
does one rank still cost. The A_1x128 rows, the 2-node rows, energy/gradient as an in-cell null
control, and peak RSS were measured on an earlier cut of this branch against 48cadcb — a baseline
the rebase moved off — and are not re-measured here. Treat that earlier grid as evidence about
the commit it measured, not about this one.

Gates

ctest on this branch: unit 271/272, serial 270/270. The single unit failure is
monoprop_link_export_probe, which fails identically on untouched main 999caf18 (243/244) —
it aborts with MPI_Comm_size() called before MPI_INIT. That is #313's own regression test failing
on main, not a defect of this branch, and it is worth a separate issue.

Python MPI suite: 600 passed at each of ranks x partitions 2x1, 2x16, 4x8 and 16x16 (world 2 to
256), plus the C++ MPI ctest 1/1.

Not reproducible from this diff

Deucalion, 2x AMD EPYC 7742 / 128 cores / SMT off / NPS4 / 242 GiB per node; a private harness that
never ships; two prebuilt venvs at the md5s above.

[github-actions]

Docs preview: https://pr-263.monoprop-docs.pages.dev

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.70%. Comparing base (6c9d6c7) to head (6c606f1).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #263   +/-   ##
=======================================
  Coverage   97.70%   97.70%           
=======================================
  Files          14       14           
  Lines         742      742           
  Branches       98       98           
=======================================
  Hits          725      725           
  Misses         12       12           
  Partials        5        5           

Flag	Coverage Δ
cpp	97.70% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[robertodr]

I think this duplicates what Claude&I have been doing in #226 🤔

[robertodr]

you did indeed tell me to stack my stuff on top of this one

[diagonal-hamiltonian]

Rebased onto current main (was 13 commits behind, at cb9a033a). The five commits are unchanged in
content; the pre-rebase tip is kept at the tag archive/query-wire-v3-pre-rebase.

The commit that mattered is #270 (cf84009e), the exchange-layout / HybridComm rewrite — it
derives the exchange layouts and stores only occupied world slots, and deletes RecvLayout.h. This
branch predated it, so it could not be benchmarked against anything current. Two conflicts, both
trivial: AlgebraCommon.h (keep both includes) and cpp/tests/fused_query_codec_tests.cpp (this
stack deletes it; main had only added a TypeAliases include, so the delete stands).

Verified after the rebase: 270/270 ctest, and clean under real MPI at world 2 and 4.

#296 stacks GF(2)-linear rank routing on top of this and is now based on this branch.

[diagonal-hamiltonian]

@robertodr your back is scratched. Chop chop

[robertodr]

I'm chopping wood like never before

[robertodr]

I would ask two things:

1. It is not straightforward to get how the encoding for QueryWire works in practice. Can you add an explicit example using a Doxygen docstring?
2. Could you maybe try to tone down the C-isms? I'm not sure all of them are warranted, even in performance-sensitive contexts.

Also a concern, which might be me overthinking: there are a lot of asserts but these do nothing in Release so I don't know if we'll ever catch anything with them... Raising exceptions/returning std::expected seems also a bad idea though 🤷

[diagonal-hamiltonian]

🤖 AI text below 🤖

Review pass: 6c606f18

All three asks addressed, plus the two open threads and the Writer question you self-resolved
without an answer. No wire-format change and no change to what any of this computes.

1. An explicit example, in a docstring

QueryWire gets a Doxygen block naming each field with its bit range and working one term end to
end — {3, 7, 8, 40} at 128 modes packs into 34 bits, one word, against the four a 256-bit dense
stride spends. The bracketed field list is gone. AGENTS.md:28 already asked for Doxygen on
header declarations, which the file was not following, so this header now uses /*! @brief */
and //!< throughout. That does raise comment density here, against the previous round's
direction — deliberate, and only in this file.

2. C-isms

Pointer+length pairs and reference out-params become std::span / ranges and returned
aggregates. This is not new ground: std::span is already the idiom in MPFunctions.h:47,
detail/mpi/Exchange.h:29, InvertedIndex.h:242 and Scan.h:88,280.

was	is
push(buf, const PosU *pos, size_t k, int phase)	push(buf, pos, phase)
read_query(buf, form, off, OutT *out, int &phase_out)	read_query(buf, form, off, out) -> Decoded{next, phase}
merge_partner_positions(a, ka, b, kb, out, size_t &overlap_out)	merge_partner_positions(a, b, out) -> MergedPartner{count, overlap}
find_batch_positions(pos_flat, pos_off, k_of, n, out, hash_out = nullptr)	five spans; n is pos_off.size(), the optional tail is an empty span
RowPositions{const PosT *pos; size_t count;}	holds a std::span
pos_flat.data() + pos_off[g], at three sites	IncomingProbe::positions_at(g)

gap_width, pair_count, read_positions, set_positions, row_eq_positions,
find_positions_, fold_hash_positions and SelfQueryStage::push go the same way. The element
type stays deduced (a contiguous_range) rather than fixed, because OperatorIndex::PosT is
uint8_t/uint16_t/uint32_t by NumModes while the wire's is always uint16_t — that
genericity is real, not decoration. Every call site got shorter; they all passed
.data(), .size().

Two things I kept, with reasons rather than by default: Writer/Reader (answered on the
thread — a variable-width bit-field packer, which bitset_to_indices is not and the repo has no
other instance of) and [[gnu::always_inline]], which has seven prior uses on main.
Resolve.h's std::vector<double> *lv is still a nullable-pointer-as-optional; I left it, as
it is an owning container rather than a view, but say the word.

3. The asserts — you were right, and it was worse than it looked

main carries 9 runtime asserts, all in detail/. This stack had added 24 on top. And
pyproject.toml:131 pins cmake.build-type = "Release", CMakeLists.txt:41 defaults to it, and
every cmake/compiler_flags/*.cmake puts -DNDEBUG in RELEASE and RELWITHDEBINFO — while
test.yml:239,248 and the justfile only ever run ctest --test-dir build/editable/Release. So
none of them executed anywhere, including CI. Only the coverage job keeps them live. clang-tidy
also runs against the Release compile_commands.json, so their bodies are not even linted.

Rule applied: keep only what prevents undefined behaviour or a silently wrong encode at an
entry point; anything restating the code's own arithmetic or walking a stream becomes a test.
24 added asserts → 7; the branch total goes 33 → 16 against main's 9, and main's own 9 are
untouched. The 26 static_asserts all stay — compile-time, always on, and they are what pin the
header layout.

The one that mattered: check_header existed only to be called from an assert inside
read_positions, so it had never run. It moves to cpp/tests/dense_query_reference.h as
wire_header_is_consistent and is asserted from the differential — which also lets it check k
against the positions handed back, where the library version took k from the header it was
validating. 13 dead asserts became one check that runs in CI. The popcount identity dropped from
the emit site is already asserted in partner_merge_tests.cpp:74.

Gates

ctest -L serial on a build made from this source: 100% tests passed, 0 tests failed out of 270, 538 s. clang-format --dry-run --Werror clean on every touched file.

monoprop_link_export_probe remains the one red unit case, and it is red on a24143fb itself
(#313's own regression test, MPI_Comm_size before MPI_Init) — unchanged by this branch.

Performance

You asked whether this slowed anything down, so I measured it rather than asserting it from
"semantically identical". It does not.

Interleaved A/B in one allocation, arms alternating with the order flipped every rep, 10 reps,
ratio taken as the median of per-rep paired ratios. main = 8697f6d9 (the branch as you
reviewed it), port = 6c606f18. The arms are provably different binaries: _core.so
e39ec0db… vs c27cb883…, and .text 7,144,041 → 7,147,113 bytes, so this is not two copies
of one build being compared.

layout	operation	main med	port med	port/main	raw p	Holm p
X_1x1	propagate[hubbard]	2440.9 ms	2415.2 ms	0.99x	.0020	.0117	resolved, faster
X_1x1	propagate[pauli]	500.4 ms	495.0 ms	0.99x	.0020	.0117	resolved, faster
X_1x1	build_graph[pauli]	627.0 ms	618.8 ms	0.99x	.0215	.086	unresolved
B_8x16	build_graph[pauli]	789.4 ms	763.8 ms	0.98x	.34	.69	unresolved
B_8x16	propagate[hubbard]	1949.1 ms	1962.6 ms	1.01x	.11	.33	unresolved
B_8x16	propagate[pauli]	699.9 ms	700.4 ms	1.01x	.75	.75	unresolved

Holm across the family of six. No cell resolves as slower. Two resolve as ~1% faster, and I
am not going to claim that as a win: I cannot name the line that would cause it, the two B_8x16
propagate cells point the other way while unresolved, and a 3 KB shift in .text moves inlining
decisions on its own. The honest reading is "no detectable cost, with a hint of a small
improvement I can't attribute".

Peak RSS, kernel truth from /usr/bin/time -v around every rank: 1.00x at B_8x16 and 1.01x at
X_1x1, both flat.

Provenance held on both cells, including the placement gate, which is a property of the layout and
was read off the baseline arm rather than declared: B_8x16 pins 16 NUMA-aligned threads per rank on
both arms, X_1x1 pins nothing on either.

Two limits worth stating rather than burying. The rung is small — hubbard 1.17M terms, pauli
223K, ~250 MB/rank — so this resolves a regression of a few percent, not a fraction of one; and
build_graph[hubbard] is not covered, because the suite skips it by its own guard (29
successive calls retain 29 layer-sets, over 242 GiB) unless monoprop_BENCH_ALLOW_BIG_GRAPH=1,
which at the default config would OOM the node.

One for someone else

harness/build.sh broke on #314: passing monoprop_ENABLE_MPI as a config-setting gets the cmake
define but not the build.requires injection, because a [[tool.scikit-build.overrides]] if.
clause matches the environment only. Configure then dies in src/monoprop/bindings/CMakeLists.txt
while mpi4py sits in the project venv — the missing copy is the one in the isolated build
environment. Fixed on my side by exporting the variable; flagging it in case other tooling drives
the build the same way. The new FATAL_ERROR message is what made it a five-minute diagnosis
rather than an afternoon, so that part of #314 earned its keep.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
42.0% Coverage on New Code
1.3% Duplication on New Code

See analysis details on SonarQube Cloud