Conversation
robertodr
requested review from
adamglos92,
diagonal-hamiltonian,
fpietra and
ludmilaasb
as code owners
August 29, 2026 16:43
github-actions
Bot
added
documentation
5 tasks
|
Docs preview: https://pr-308.monoprop-docs.pages.dev |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## split/04-sparse-row-store #308 +/- ##
============================================================
Coverage ? 97.58%
============================================================
Files ? 14
Lines ? 746
Branches ? 98
============================================================
Hits ? 728
Misses ? 13
Partials ? 5
Flags with carried forward coverage won't be shown. Click here to find out more. |
9 tasks
A refactor that must not move a term is checked against a captured baseline rather than by reading the diff. capture-baseline.py propagates every msgpack fixture at a spread of cutoffs and cutoff types plus a few native qubit problems, and dumps each run's term count, its full (monomial indices, coefficient) set and its expectation value. Terms are dumped in the engine's own returned order and `just diff-baseline` is a byte-wise diff, because that order runs from SplitmixHash through probe order, MPI owner routing and insertion order into floating-point accumulation order -- a silent reorder is the regression this exists to catch. `--compare --tol` is the other mode, for a change that reorders on purpose: term set and count exact, coefficients and energies to a relative tolerance. Assisted-by: ClaudeCode:claude-opus-5
OperatorIndex was two things: a packed position-list row representation and a keyless open-addressing index over those rows. RowHashTable is the second half, lifted out whole -- power-of-2 slots, linear probing, load factor 0.7, a 32-bit folded hash per slot used only as an equality pre-filter. Keyless is what makes the split possible: the table never stores or compares a key, so the hash and the equality test arrive as callables and the row representation stays entirely on the caller's side. find_batch keeps its pipeline by taking the row prefetch as a third callable -- deferring confirmation past the probe is the whole reason that prefetch has somewhere to go. The table's slot layout fixes for_each_slot's iteration order, which is the order of a propagator's evolved-term list and therefore its floating-point accumulation order, so this had to come out inert. Checked byte-wise against the previous implementation over 4000 random monomials: identical iteration order, find and find_batch results, clone order and memory_bytes. Assisted-by: ClaudeCode:claude-opus-5
SparseRowStore is a third row representation: each row is a fixed-width list of the modes it occupies plus one word carrying two bits per occupied mode. CodesAlgebra.h is the structural algebra over that word -- one function per dense counterpart, reading the codes word instead of looping over storage words, plus sparse_toggle, the product M (+) G as one merge over two ascending lane arrays. Not yet selected by anything. MPOperator still holds a single OperatorIndex, and neither RowAccess nor the layer-build scan knows this store exists; monoprop_SPARSE_ROW_MIN_MODES is defined and preferred_for_modes() reads it, but no caller asks. Wiring it in is the next change. Two pieces of shared machinery move to RowHashTable.h beside the index both stores key rows through: geometric_row_capacity, the 1.5x growth rule, and spilled_rows_bytes, the side-map accounting. A single number in two stores would otherwise be correctable in one and not the other, which would skew operator_memory_breakdown() for one backend only. splitmix_finalize comes out of SplitmixHash<Bitset<N>>::mix, whose body never depended on the width: the row hash accumulates over (mode, code) slots and so needs the mix with no Bitset in hand. That value routes MPI ownership, so having one definition rather than two matters more than the tidiness. Verified out of tree against the compile-time-width engine: 3000 random monomials round-trip through row(), popcount(), find(), find_batch(), for_each_position(), clone() and resized(); and over 5000 monomials the codes forms of cutoff_sums, is_paired, pauli_y_count, pauli_anticommutes and pauli_rotation_sign agree with the dense ones exactly. OperatorIndex comes out inert after the two helpers move -- identical iteration order, find results, clone order and memory_bytes. Assisted-by: ClaudeCode:claude-opus-5
Wires SparseRowStore and CodesAlgebra into the engine: a propagator now picks a row backend once at construction and everything from the scan to the inserts is templated on it. MPOperator holds one pointer per backend with exactly one non-null and binds the live one via with_store -- once per layer, inside build_layer, never per term, since the scan asks the store for a row per anticommuting term. LayerBuildEngine, fused_find_and_collect and probe_incoming_queries take the store as a template parameter; off that path the forwarding accessors pay one well-predicted branch. There is no accessor handing out a store, because there is no one type to hand out, so MonomialPropagator::indexing() is replaced by for_each_term() and num_local_terms(). The choice comes from the mode count against monoprop_SPARSE_ROW_MIN_MODES; monoprop_ROW_STORE=dense|sparse forces it process-wide, and an unrecognized value throws rather than falling back to auto -- the point of setting it is to know which backend ran. Every C++ case is registered a second time with the sparse backend forced (the sparse-rows ctest label, 541 cases where there were 270): every fixture is below the crossover, so without that the support-form backend would ship untested. row_store_selection_tests.cpp is what fails if the variable stops reaching the propagator. Gates: the dense capture is byte-identical to main's golden baseline, and the sparse capture agrees with it as term sets plus rtol 1e-10 (just diff-baseline-sparse). Assisted-by: ClaudeCode:claude-opus-5
The engine no longer carries the mode count as a template parameter. A propagator takes its logical width as a constructor argument and sizes its monomial storage from it at runtime, so there is one compiled MonomialPropagator whatever the mode count and no compile-time ceiling on it. Bitset *is* the monomial: the width is data, so a width exists per value rather than per type. Per-word loops go through detail::with_nwords, which dispatches a runtime word count to a compile-time-unrolled arm in place of the if constexpr branches a compile-time width allowed. The binding generators go with it -- tools/generate-binders.py and tools/generate-dispatch.py emitted one instantiation per mode tier, and bindings.cpp is now a real file rather than a configured template. Dense-row terms and energies are byte-identical to the compile-time-width engine across every baseline case, and the support-form backend still agrees with them as term sets and to rtol 1e-10. The per-gate kernel seam and the support-form query record that recover the lost per-term specialization are not here; they follow separately. Assisted-by: ClaudeCode:claude-opus-5
robertodr
force-pushed
the
split/05-drop-nttp
branch
from
August 31, 2026 08:57
e89a79e to
e2cd6ff
Compare
|
6 tasks
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
The engine no longer carries the mode count as a template parameter. A propagator takes its logical width as a constructor argument and sizes its monomial storage from it at runtime, so there is one compiled
MonomialPropagatorwhatever the mode count and no compile-time ceiling on it.Bitsetis the monomial: the width is data, not a template parameter, so a width exists per value rather than per type. Construct with one (Bitset(num_bits)), take a mode count frommono.size() / 2, recover a width with an instance call. Per-word loops go throughdetail::with_nwords, which dispatches a runtime word count to a compile-time-unrolled arm, in place of theif constexprbranches a compile-time width allowed.That removes the binding generators with it.
tools/generate-binders.pyandtools/generate-dispatch.pyexisted to emit one instantiation per mode tier;bindings.cppis a real file now rather than a configured template, so the nanobind version the module reports arrives asmonoprop_NANOBIND_VERSIONfrom CMake.Stacked on #305.
Changes
Bitset/Monomial: runtime width,detail::with_nwordsdispatch,heap_bytes()for the spilled regime. Only words[0, num_words())hold a value — the inline tail above them is left indeterminate so a copy costs the operand's own width.MonomialPropagator:num_modesis a constructor argument;detail::storage_modes_for()derives the storage width from it, rounded up to a whole 32-mode block.with_algebrabinds a runtimeBasiswithout a width; the algebra policies lose theirNumModesparameter.MonomialPropagator.inlbecomesMonomialPropagator.cpp— there is nothing left to instantiate per width.tools/generate-binders.py,tools/generate-dispatch.py,tools/_binding_layout.py,bindings.cpp.in,tests/test_binding_layout.py.ModeEmbedding/WIDE_EMBEDDING(tests/cases.py) andtest_utils::embed_caserelabel a fixture into 260 logical / 288 storage modes — nine words per monomial, pastBitset's eight inline ones. The map is monotone, so the fixture's exact energy still applies and the wide run owes the narrow run's evolved operator term for term.detail/partition/StagedCollect.h: one result per partition written from the partitions' own masters. The staging vector is there for theboolcase only —std::vector<bool>is bit-packed, so concurrent writes to disjoint logical elements can tear the same word.Verification
capture-baselinevs main's golden, dense rows--compare --tol 1e-10, sparse vs dense (incl. the new wide case)ctestsparse-rows)pytest,autoandmonoprop_ROW_STORE=sparseprekover the changed filesThe byte-wise dense gate is the load-bearing one: the width stops being a type, and no term, coefficient, iteration order or energy moves.
Two pre-existing
monoprop-bench-toolstest_memory.pyfailures are unrelated — they fail identically on a pristinemaincheckout and pass in isolation.Not in this PR
The per-gate kernel seam (
TermProduct.h,WordKernel<W>,with_kernel_width) and the support-form query record follow in #309. Both are written against the runtime width introduced here, so they cannot precede it. Until they land the per-term instruction count runs above the compile-time-width engine — about a fifth of which reaches cycles.Checklist
docs/,CONTRIBUTING.md) if neededCHANGELOG/ release notes updated if applicableAI/LLM disclosure