chore: update the benchmark scripts and documentation - #148
Open
ludmilaasb wants to merge 99 commits into
Open
Conversation
…untime Re-homes the performance work from perf/align-with-paper onto the current public interface (#40). The diff vs main is performance-only; the propagator Python/C++ interface is unchanged. What lands: - Rebuilt evolution/layer-build engine (detail/evolution/layer_build/*, CosineRecompute, LayerBuilder) aligning the build path with the paper, plus the operator store rework (detail/operator/{MPOperator,OperatorIndex, InvertedIndex}) and cosine-recompute graph encoding, replacing the old masked_execution_plan / MPGraphEncodingCompression / EvolutionMajorana. - Operator sharding runtime (detail/shard/{CpuTopology,ShardGroup}, detail/mpi/*): one single-threaded shard per physical core is the default parallelism (shards=0 => auto), composing MPI ranks and shards into one flat hybrid world; query (two-pass) is the sole cross-rank exchange protocol. - In-repo std::thread pool (run_static) replacing oneTBB; drop the oneTBB dependency from CMake, packaging, and the Config export. - Additive-only public surface: basis/shards ctor kwargs (defaulted), evolved_operator_terms, __deepcopy__ — main's interface otherwise untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Migrate the C++ unit suite to the ported engine internals (OperatorIndex / MPOperator / cosine-recompute, the shard runtime, the align mpi:: transport). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two engine-behavior adaptations, no interface changes: - Sharding is now the default parallelism, so white-box tests that inspect raw per-partition internals (graph_data()) opt out via a new @pytest.mark.unsharded marker (autouse fixture sets monoprop_SHARDS=off before construction). - upper_atol in the rebuilt build path RESCUES over-cutoff partner terms (it can only add terms, never drop). At cutoff 6 the LiH molecule has no over-cutoff partners, so exercise the rescue at cutoff 2 where it observably changes the size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oops Every parallel_for_*/parallel_reduce_* wrapper and the layer-build chunk helpers (for_each_chunk / append_*_in_order) took a serial-fallback branch on the default shard runtime — each shard master sets gate_serial_override, so effective_parallelism() reports 1 and the pool threads never start. Only the non-default SHARDS=off path ever engaged them. Rewrite all ~46 call sites (13 files) as the plain serial loop each wrapper's fallback already ran, so results are bit-identical on the default path. The in-repo thread pool (Threading.h/.cpp) and Parallel.h still exist after this commit; they lose their last callers and are deleted next. Bit-exactness: reductions were already chunk-order-deterministic, so the serial folds reproduce them exactly. FP association differs ONLY on the demoted paths (SHARDS=off, explicit shards=1 off a master, pure-MPI unbound ranks): inner_product, accumulate_cos_*, and the endpoint-contrib folds get serial association there; those paths' tests are tolerance-based. Also collapses the now-single-table OperatorIndex bulk_insert (deletes the counting-sort branch), the InvertedIndex row-block-parallel fill + its persistent staging members, and folds the single cosine block set in the layer-build scan directly into the result. Validated: MPI-OFF C++ unit tests 102/102 "No errors detected". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ue tests The rescue invariant is independent of the shard/thread count (each shard runs its partition serially), so the thread-mode axis is meaningless. Drop the ScopedParallelismCap guard and the thread_mode_values() axis: 4 tests per (fixture, comm) instead of 8. Removes this suite's dependency on Threading.h ahead of deleting the pool. snapshot_invariance.cpp: reword the comment off the now-gone parallel_reduce_indices. Validated: MPI-OFF C++ unit tests 98/98 "No errors detected" (was 102). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e only parallelism
The pool primitive (run_static), the parallel_for_*/parallel_reduce_* wrappers,
the gate_serial_override TLS, ScopedParallelismCap, and the layer-build chunk
helpers now have no callers (all rewritten to serial loops). Delete them:
- src/monoprop/Threading.{h,cpp}
- src/monoprop/detail/evolution/layer_build/Parallel.h
and every #include of them, plus the two dead detail:: wrappers in
EvolutionHelpers.h, the gate_serial_override write in ShardGroup, the
init_from_env() calls in MPICompat and the nanobind module init, and
Threading.cpp from the library sources.
monoprop_NUM_THREADS is unaffected: resolve_shard_count_ reads
config::get().num_threads directly, never the (now-gone) pool. std::thread is
still used by ShardGroup and the ShmComm/HybridComm barriers, so Threads::Threads
stays linked. Breaking (!) because installed public headers/symbols (Threading.h,
run_static_impl, init_from_env, ScopedParallelismCap, current_max_parallelism)
are removed.
Validated: MPI-OFF C++ unit tests 98/98 "No errors detected".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
busy_ns/tasks and the capture()/TaskScope machinery existed to attribute parallel-task time to a dispatch region across pool worker threads. With the pool gone and each shard running its region serially, they have no producers. Drop capture(), TaskScope, profiling_current(), RegionAcc::busy_ns/tasks, and ScopedRegion's prev_ save/restore. The monoprop_PHASE dump is now wall_ms + calls per region. Validated: MPI-OFF C++ unit tests 98/98 "No errors detected". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y shim
CpuTopology.h is now the ONE platform-specific file: its whole body (the /sys
parse, L3-domain interleave placement, pthread_setaffinity_np) sits under
`#if defined(__linux__)` with a `using CpuSet = cpu_set_t;`, and a portable
`#else` provides an empty CpuSet plus no-op enumerate/shard_cpusets/pin — so no
Linux type or header leaks out (ShardGroup now holds std::vector<CpuSet>). The
portable shard-count fallback (hardware_concurrency()/2) is unchanged; macOS
additionally reports its physical-core count via sysctl for accurate auto
sharding even though it cannot pin.
Two correctness/portability fixes a topology package (hwloc) would have given —
done in ~15 lines instead of a bundled C dependency:
- enumerate_physical_cores() intersects with sched_getaffinity(): a
cgroup-restricted / partial Slurm allocation now reports and pins only ITS
cores, instead of enumerating all 112 and oversubscribing / pinning outside
the mask (previously masked only because the sbatch scripts set NUM_THREADS).
- monoprop_SHARD_PINNING moves into EnvConfig (config::get().shard_pinning),
parsed once with the shared parse_flag; semantics identical.
hwloc was evaluated and rejected: it replaces only the ~75-line /sys parse and
the 3-line pin call (the ~80-line placement policy survives on top of it),
cannot pin threads on macOS regardless, and would re-add a bundled autotools C
dependency right after TBB/quill were dropped.
Validated: MPI-OFF C++ unit tests 98/98 "No errors detected".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TBB is neither used nor a build dependency. Remove the leftover mentions the engine's TBB removal never swept: the libtbb-dev / brew tbb installs in CI (test.yml, docpages.yml, copilot-setup-steps.yml), the README and AGENTS.md dependency lines, and a stale benches comment. No build/behaviour change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parallelism.mdx claimed monoprop used oneTBB with a monoprop_NUM_THREADS worker count. Rewrite it around the real model: one serial shard per physical core by default, composing with MPI into an R×S world. Document all five runtime env vars (NUM_THREADS, SHARDS, SHARD_PINNING, PHASE_TIMERS, RECOMPUTE_CACHE_MAX_MB), the measured 6-9x (Pauli) / ~2x (Hubbard) win over the former pool, the non-Linux/partial-allocation fallbacks, and why there is no hwloc dependency. Fix the "oneTBB worker count" line in benchmarks.mdx and the sharding-controls link blurb in building.mdx. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a `topology` field to KickedIsingConfig with a `_topology_edges` helper: "heavy-hex" (default, byte-identical to today's 127q map) or "chain" (1D nearest-neighbour), so the qubit count can be swept for scaling studies. Record for fixed-model runs the same non-timing quantities the random benches capture (term count, operator/graph storage, resting PSS) keyed by model name, plus `membase` (resting PSS before the build) so the operator's persistent footprint can be isolated as memrest - membase. Fall back to serial when mpi4py is present but cannot dlopen libmpi (the ABI wheel on a serial node), not only when it is absent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
operator_memory_bytes()/graph_memory_bytes() require an unsharded propagator and raise once the operator shards under multi-thread parallelism, which failed the fixed-model point (and tripped the sweep's ratchet). The byte count is thread-count-independent, so the serial run captures it; skip it when sharded rather than fail, keeping time/PSS/term-count recording intact for every point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-width cap Expose a per-component operator memory breakdown plus popcount / support-window / diagonal-count histograms and the inline width via MonomialPropagator and the Python bindings, to attribute per-term byte cost in the scaling study. Split OperatorIndex's single inline-width constant into kDefaultInlinePositions=11 (byte-identical default for Schrödinger/opaque-cutoff rows) and kMaxInlinePositions=32, and derive packed_inline_width_ from the cutoff (2*cutoff for a Pauli support bound) so cutoff-bounded terms stay inline instead of spilling to the mutex-guarded overflow arena. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…local-symplectic Fold the sweepable pauli-chain bench topology (per-model stats, sharded storage accounting) and the operator memory/popcount diagnostics onto the JW-free native-Pauli fix branch so it can drive its own memory+speed A/B. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…frame
Route PauliPropagator onto the engine's native Basis::Pauli path instead of the
Jordan-Wigner Majorana image. JW packed a weight-1 X_q as a Majorana monomial of
popcount 2q+1 = O(N), so operator memory grew with the qubit count at fixed cutoff.
The local packing (X_q->slot 2q, Y_q->slot 2q+1, Z_q->{2q,2q+1}) gives popcount
<= 2*weight, independent of N (verified: X on the top qubit is popcount 1 at every N).
- dispatch generator + _init_simulator: thread a `basis` argument through to the core
(the binding already accepted it); PauliPropagator passes basis="pauli".
- conversion_utils: _pauli_to_local_slots / _local_slots_to_pauli (encode/decode,
mirroring slots_of_string / letter_from_bitset in the C++ tests).
- PauliOperator.get_local_operator(): pack terms into local slots with real coeffs.
- circuit._gate_layers: native branch emits (local slots, real g) directly -- no
jw_coeff, no antihermitian normalization (the Pauli rotation kernel handles phase).
- PauliPropagator: ingest via get_local_operator, cutoff_type="support", no basis_change;
evolved_operator() now decodes to a PauliOperator (was a raw index dict).
No C++ engine changes: the native path is already implemented and validated
(tests/cpp/pauli_build_layer_tests.cpp T6/T7). Full Python suite green (365 passed);
the qiskit-reference evolution test matches at 1e-6.
BREAKING CHANGE: PauliPropagator.evolved_operator() returns a PauliOperator (Pauli-keyed)
instead of a dict keyed by Jordan-Wigner Majorana index tuples.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route PauliPropagator onto the engine's native local-symplectic (Basis::Pauli) path instead of Jordan-Wigner Majorana storage. Fixes the O(N) memory growth (JW popcount artifact): operator store is now N-flat (~54.7 B/term, 0 overflow) with popcount O(cutoff). Arch-native compute A/B (job 49844575) vs a same-build JW shim: energies bit-identical, term counts identical, build_graph 1.98x-7.30x faster single-thread. C++ arbiter tests (pauli_build_layer_* T6/T7, pauli_algebra_*, shard_equivalence_*) green. Python-only change; no C++ algebra touched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The per-layer fold cache retained a (sum of mask_words * 8B) buffer per functional and was gated by monoprop_RECOMPUTE_CACHE_MAX_MB. A functional-path A/B (2026-07-20) showed it bought <=5% per eval -- and LOST for large operators, its buffer going cold faster than the L1-blocked recompute rebuilds it -- while costing 0.3-1.0 GB per 1.7M-term operator and creating a silent mid-sweep regime change in scaling benchmarks. Time-only break-even was 22-70 evals: trading GB of memory for a shrinking (then negative) speedup is backwards for a scaling-focused library. build_cos_callbacks now always takes the recompute (LazyFold) path. make_fold_cache / scale_cos_cached / fold_to_cos_mask survive only as the one-shot pare materializer and the C++ recompute-equivalence-test oracle (no retained buffer, no scale cost). Validated: C++ suite 106/106 cases, 1,413,155 assertions pass (incl. *cache_equals_recompute*); Python results bit-identical to pre-change; the cache-vs- recompute peak-RSS gap collapses from +600-950 MB to +/-1 MB; pytest functional/exact slice 33 passed, 1 skipped. BREAKING CHANGE: the monoprop_RECOMPUTE_CACHE_MAX_MB environment variable is removed; cosine folds are always recomputed on the fly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Matteo AC Rossi <9745862+matteoacrossi@users.noreply.github.com>
Attacks the anticommutation fold, measured at ~60-70% of per-gate cost, using
the bimodal gate-participation structure (most gates anticommute with almost
nothing yet the fold streamed all K/64 operator words regardless).
- Zero-postings early-out: a generator whose inverted-index fold columns hold
no postings provably anticommutes with nothing (even |G|; Pauli is always
even in this path), so pass 1 + pass 2 are skipped entirely. A dense column
always holds >=1 posting, so the emptiness test is O(|G|) sparse-size reads.
Fires on ~70% (hubbard) / ~64% (pauli) of per-shard gate scans, removing that
share of fold busy-CPU at high shard counts. The g_odd guard is load-bearing:
an odd Majorana generator anticommutes with the odd-weight terms it is
disjoint from, so zero overlap does not imply commutation there.
- Lazy sparse-pivot expansion: a sparse pivot column is scatter-expanded only
for fold blocks that produced a nonzero overlap word; empty blocks (the common
case on low-participation gates) no longer pay the expansion stream. Dense
pivots stay folded inline (deferring them regressed hubbard +7.6%).
- combine_columns_block seeds its scratch from the first dense column by memcpy
(XOR-commutative) instead of memset + XOR-all, saving one pass over the block.
New monoprop_FOLD_STATS env knob (default OFF, zero-overhead when disabled,
RegionProfiler atexit dump): per-gate {all-sparse?, sum postings, K/64, n_anti,
structural rejects} + a log2 postings/(K/64) histogram. The sizing data it
produced ruled out two further proposals (candidate-merge discovery and a
bit-sliced structural prefilter) as sub-single-digit on the real workloads.
All changes bit-identical (verified: full C++ suite + real-MPI R=2 green;
AB probe byte-for-byte vs baseline, hex-float 0x1.2a7a10767e528p+0 preserved).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove ~150 lines of copy-pasted Pauli oracle code duplicated between pauli_algebra_tests.cpp and pauli_build_layer_tests.cpp into a shared PauliTestOracle.h (native/JW encoding, dense Pauli-matrix brute force, string helpers). Factor the ShmComm/HybridComm thread-spawn harness into ThreadHarness.h (run_comm_threads), and hoist the duplicated near() float comparison and LihFixture into TestUtilities.h; slots_of_string, the near() constant, and RandomExactFixture now come from the shared headers across the equivalence suites. Merge the two single-case files into their neighbours: utilities.cpp (even/odd-bit helpers) -> mpfunctions.cpp, snapshot_invariance.cpp -> the recompute-equivalence file that exercises the same machinery. Rewrite the stale tests/cpp/README.md (wrong binary name, deleted-file list). Behaviour-preserving: C++ 94/94 serial + MPI green, no test cases removed beyond the two folded ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove validators with zero callers anywhere in the codebase (verified by repo-wide grep, incl. bindings and Python): determine_evolution_mode, validate_evolution_parameters, validate_graph_state_for_mode, validate_params, validate_propagation_params, validate_propagation_contraction, validate_param_map_gen_coeffs_majoranas_match, the EvolutionMode enum, and the private has_complete_evolution_parameters helper. The live surface (validate_coefficient_lengths, validate_gate_indices, validate_parameters_length, validate_functional_call, validate_expected_graph_layers, validate_equal_sizes) is untouched. No behaviour change: the deleted symbols had no callers, so nothing in the hot path is affected. Library relinks clean; C++ + Python suites green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add direct unit coverage for structures that were only exercised indirectly through the engine end-to-end: - bitset_tests.cpp: Bitset ctor top-mask, cross-word count_and/parity_and, multi-word shift and find_first/find_next, position-sensitive hash, plus a randomized differential fuzz against std::bitset. - majorana_cutoff_tests.cpp: length/support cutoff (incl. logical_num_modes masking on single- and multi-word paths), CutoffEvaluator dispatch + popcount fast path + max_positions_bound, interleave_phase vs its masked- parity form, encode/decode_coeff round-trip and non-Hermitian throw. - validation_tests.cpp: the live parameter validators (accept + reject paths). - mpi_utils_tests.cpp: find_rank determinism/range/hash-mod and the Majorana word (de)serialization round-trip. - evolution_detail_tests.cpp: MatchedEpochSet O(1)-clear / tail-grow / u32 wrap and the CutoffContext atol/upper-atol gating predicates. - row_accessor_tests.cpp: dense-vector vs OperatorIndex backend differential for materialize_row/assign_row/row_popcount/for_each_row_position. - ctor_validation_tests.cpp: MonomialPropagator ctor guards (logical_num_modes, Pauli+cutoff/basis_change, atol ordering, operator index range), propagate()-on-nonempty-graph, and MPGraph::get_layer bounds. 40 new cases; full suite 134 serial + MPI green. Validation.cpp coverage 28% -> 95%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The monoprop_WIDE_TERM_INDEX=ON configuration (64-bit TermIndex) was never built anywhere, so the wide `#if defined(monoprop_WIDE_TERM_INDEX)` branches in operator_index_tests / large_cosine_storage_tests were dead in CI. Add: - CMakePresets: release-gcc-wide configure/build/test presets. - justfile: `test-cpp-wide` (build + serial ctest under wide) and a `coverage-cpp` convenience recipe encoding the gcovr invocation. - CI: a `cpp-wide` job (gcc-14, Release + WIDE_TERM_INDEX=ON, serial ctest) so the wide branches are compiled and run on every push. Verified locally: wide build is green (serial 128/128) and the wide-only term-index-width case executes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… backbone Make the code's names and types state the essence: one monomial container, two algebra models, and a propagation backbone that is generic over the algebra. Clean break — old names are removed outright, no back-compat aliases. Bit-exact and perf-neutral: each policy method wraps the existing kernel, and the runtime Basis is bound to a compile-time model exactly once via with_algebra, replacing the compile-time IsPauli flag and every scattered `if (basis == Basis::Pauli)`. - core/Monomial.h: split the monomial vocabulary out of TypeAliases.h. MajoranaSet<N> -> Monomial<N> (Bitset<2N>: a Majorana product, or its Jordan-Wigner-image Pauli string); MajoranaVector -> MonomialList; MajoranaOperator (C++ map alias) -> MonomialMap; MPHash/MPEqual/majorana_hash -> MonomialHash/MonomialEqual/monomial_hash. The Python class MajoranaOperator is unchanged. - algebra/: MajoranaAlgebra.h and PauliAlgebra.h move here as true siblings over a new AlgebraCommon.h (shared structural primitives); PauliAlgebra no longer includes MajoranaAlgebra. - algebra/Algebra.h: the Algebra concept, the MajoranaAlgebra/PauliAlgebra policy models, the with_algebra(Basis, fn) dispatcher, and point-dispatch helpers for the cold sites that carry a runtime Basis. - backbone: fused_find_and_collect and the cosine fold are templated on the algebra policy; every basis branch outside the policy is gone, including the last two in MPOperator (HF scoring via algebra_score_hf, coeff codec via algebra_encode_coeff). - tests: reference-only helpers moved to tests/cpp/AlgebraReference.h. - docs: AGENTS.md core-abstractions section + an essence doc on core/Monomial.h. BREAKING CHANGE: the C++ core type names change (MajoranaSet -> Monomial and friends) and the two algebra headers move under algebra/. Out-of-tree subclasses such as MonomialPropagatorExtra must update to the renamed types and paths. Verified on host bebe (gcc-14): C++ 127/127, MPI 135/135 (+ 4-rank clean), wide-index 128/128, Python 447 passed / 8 skipped; exact-value suites bit-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Matteo AC Rossi <9745862+matteoacrossi@users.noreply.github.com>
Qiskit evolves by exp(-i t H); ExpGate applies exp(+i theta H). from_qiskit_circuit carried the coefficients through unchanged, so every converted circuit was the conjugate rotation. Verified against Statevector: rx(0.7, 0) with observable IY gave +0.644217687 instead of -0.644217687, an exact sign flip on every observable that does not vanish. The sign goes on the generator rather than the angle, so a converted circuit's parameters stay numerically equal to the qiskit evolution times and a gradient with respect to a monoprop parameter is a gradient with respect to the qiskit angle. to_qiskit_circuit negates the same way, which is why the roundtrip test never caught this; test_rotation_sign_matches_qiskit compares against qiskit's own simulator using observables that anticommute with the generator, so the expectation value is odd in the angle and a missing negation cannot hide. The docs asserted cos(2*theta) / -sin(2*theta) while their prose claimed exp(-i theta M / 2); the prose was wrong on both the sign and the factor of two. BREAKING CHANGE: expectation values and gradients of circuits built via from_qiskit_circuit change sign. Circuits built directly from ExpGate are unaffected. Assisted-by: ClaudeCode:claude-opus-5
- ExpGate keeps its atol so _with_index (and therefore every Circuit.__add__)
re-truncates at the tolerance the gate was built with. Concatenating any circuit
silently deleted sub-1e-8 terms an author had explicitly kept with atol=0.0.
- A gate whose every term falls below atol now expands to an identity layer (the empty
monomial with a zero generator coefficient) instead of nothing. Emitting nothing left a
hole in gate_indices, which the engine rejects outright ("gate_indices must start at 0;
got 1"), and orphaned the gate's slot on the parameter axis.
- Circuit.initial_state distinguishes unspecified (None) from the explicit vacuum (()).
The empty tuple read as "unset", so a vacuum-authored circuit skipped the
reference-state check and was silently evolved against a half-filled reference.
Circuit.__add__ had the same falsy-empty defaulting.
- Pauli rejects negative qubit indices, as Majorana already did. Pauli("Z", -1) on four
qubits resolved to qubit 3 through Python list indexing, and get_local_operator emitted
slots (-2, -1) into the engine's size_t.
_gate_layers' num_qubits argument was a bare presence check; it now bounds the Pauli
slots, catching a generator authored for a wider system than the propagator.
Assisted-by: ClaudeCode:claude-opus-5
… the setters indices_to_bitset computes 2*NumModes-1-index and Bitset::set is noexcept and unchecked, so an out-of-range index is an out-of-bounds heap write, not an exception. Only one of its five untrusted call sites was guarded. indices_to_bitset_checked bounds each index against the LOGICAL width (2*logical_num_modes, not the storage width) and now covers the initial operator, update_initial_operator, the basis-change rows, and -- the gap that was reachable straight from Python -- the gate generator in build_evolve_result_, where nothing between build_graph/propagate and the bitset constrained the indices. update_cutoff_type and update_basis_change wrote straight through to regenerate_cutoff_fn_ with no validation while nanobind exposed both as writable properties, so a Pauli propagator could be given a Length cutoff or a basis change its constructor rejects, and a short basis_change made regenerate_cutoff_fn_ index [0, 2*logical_num_modes) past the end of the vector. The constructor's checks move into validate_cutoff_config_, shared by all three, plus the row-count check that previously existed only in Python -- on a path the front-ends no longer reach, since they hard-code basis_change=None. tests/test_basis.py, deleted in this PR, is reinstated against the current API and now also pins the guards. Assisted-by: ClaudeCode:claude-opus-5
…y buffer FoldMask cached a raw pointer into InvertedIndex::row_parity_, which append_rows resizes and an index rebuild frees. A LazyFold is RETAINED -- build_cos_callbacks keeps one per graph layer inside the functional's closure -- so expectation_value_functional(pare_threshold=...) followed by another build_graph left the fold reading freed memory. optional::emplace keeps the index's address stable, so the consumer could not detect it, and the pare path's expected_layers guard can never fire because pare preserves the layer count. The pointer is now fetched at use time: scale_cos_lazy / accumulate_cos_lazy already receive the index, so they hoist it once per layer per evaluation and pass it down. The per-fold-word inner loop reads a register-resident local instead of a field through FoldMask, so this is if anything cheaper. FoldCache keeps its copy -- it is built and consumed within one call. lazy_fold_survives_operator_growth pins both halves: that the buffer really moves under growth, and that a fold built before it still matches a FoldCache built after. It fails on the previous code with a genuinely different fold, not merely latent UB. While here: LazyFold sizes its column list to |G| (typically 2-4) instead of embedding EvenParityGeneratorColumns' std::array<size_t, 2*NumModes>. The fixed array is right for the build scan, but a retained LazyFold spent 4 KB per layer per functional per shard at NumModes=256 to hold a handful of indices. Also restores the trailing return types this file and MPFunctions.cpp/Scan.h had regressed. Assisted-by: ClaudeCode:claude-opus-5
…on failure Ticket abandoned a live MPI_Ialltoallv: ~Ticket was defaulted and move-assign overwrote request_ without waiting. A posted collective keeps writing into the caller's recv buffer, so a throw between the post in begin_cross_rank_derivative_exchange and the wait left MPI writing into a thread_local buffer that the next exchange reallocates. The handle now completes what it owns in both places. resolve_recv never checked that the layout's width matches the live comm. alltoall_counts moves comm_size ints each way regardless of `n`, and the layouts live on a shared_ptr<const LayerCore> that survives propagator copies and pare rebuilds, so a graph replayed on a differently-sized comm read and wrote out of bounds -- and, on a cache hit, silently reused the stale layout while its peers waited in a count round it never joined. Displacement prefix sums accumulated in plain int in three places (resolve_recv, MPICompat's vector-of-vectors path, HybridComm::size_staging_impl_) while only the per-rank counts were range-checked. Each is now a wide accumulator narrowed through a checked cast: signed overflow was UB, and the wrapped value then sized a staging buffer or became a negative displacement. The comment claiming size_staging_ was already overflow-guarded is corrected. ShardGroup started its master threads before building the shards on them (for first-touch locality), so a throwing factory -- every MonomialPropagator ctor validation, including the checks added earlier in this branch, and any allocation failure -- unwound past joinable threads without ~ShardGroup setting stop_. Both constructors now join through the shared stop_and_join_, which also poisons first so a master parked in a barrier is released instead of joined on forever. Reproduced: the new test hangs the suite on the previous code. enumerate_physical_cores broke at the first CPU id whose thread_siblings_list is unreadable, assuming online ids are contiguous. One offlined CPU truncated the core list to whatever preceded the hole, and that list picks the AUTO shard count -- 3 shards instead of 56 on a 112-core host with cpu6 offline. It now scans the whole allowed id range and skips holes. Assisted-by: ClaudeCode:claude-opus-5
…ctive ShardBarrier::poison is rank-local, but shard 0 is its rank's only participant in the collectives on parent_. One rank's shard throwing (a peer shard's bad_alloc, or the count overflow guarded in the previous commit) made that rank's shard 0 throw ShmCommPoisoned before entering MPI_Alltoallv, leaving every peer blocked inside MPI forever with no timeout and MPI_Abort the only exit. hybrid_comm_poison_releases_waiters covers only the symmetric case where shard 0 poisons BEFORE entering a collective, which is safe and still raises cleanly. guard_shard0_ wraps all five verbs: once shard 0 is inside one, any rank-local failure aborts the job with the underlying error text instead of unwinding. Nothing is added to ShardBarrier::sync (the documented top hotspot at S=112) or to any pack/scatter loop -- a try/catch costs nothing until it throws. This also aborts when every rank fails identically, which used to raise cleanly on each; telling the two apart needs a collective, and that is exactly the per-layer cost this must not add. Single-rank sharded runs use ShmComm rather than HybridComm and keep their exceptions. Verified against the MPI build the previous cmake fix unblocked: 187/187 pass with monoprop_ENABLE_MPI=ON, including the two-rank suite. Assisted-by: ClaudeCode:claude-opus-5
LayerTraversal::num_cos_inds() returns 0 unless the layer carries a STORED (pruned) cosine set, which no normally-built layer does -- MPGraph::append leaves pruned_cos_ empty. Reading it was therefore structurally zero for every non-pared graph, so graph_size()[0], documented as "the number of cosine indices", was always 0. tests/test_coeff_trunc.py asserted == 0 in two places and passed vacuously. The count cannot be computed inside MPGraph: it needs the operator's inverted index, which the graph has no access to. So MPGraph keeps only total_cycles() and MonomialPropagator::graph_size() recomputes the fold per layer, exactly as graph_data() already did, taking the stored count instead for a pared layer. fold_popcount does it without materialising the index list. This is a diagnostic off every evaluation path. The count is legitimately zero when nothing is truncated -- every anticommuting term's sine partner survives and is a rotation endpoint, so cosine-ONLY is empty. The new tests pin both sides: a cutoff tight enough to drop the partners must give a positive count, and an exact cutoff must give zero. Verified on random_exact at cutoff=8: (349, 911) where it used to read (0, 911). Assisted-by: ClaudeCode:claude-opus-5
…te the sums max_slots_per_cutoff_unit hung the position-bound scale factor off the ALGEBRA, but the factor depends on the cutoff TYPE: a length cutoff counts set bits directly, a support cutoff counts modes and each spans two slots. Majorana + Support is a legal combination that got factor 1, so packed_inline_width_ under-sized the packed row and roughly half those terms spilled to the overflow map. The bound moves onto CutoffEvaluator::max_slot_bound, which already knows which functor it holds, and both algebras lose a constant the Algebra concept never required. length_cutoff and support_cutoff were four near-identical blocks (single-word and multi-word, x2) differing only in popcount vs or_sum -- the drift risk the two width paths already invite. They now share one always_inline cutoff_sums helper and read the field they care about. Measured, since this is per-monomial: a focused microbenchmark over 16k monomials x 400 reps, at both widths, with and without a logical prefix, is unchanged to within noise -- best-of-3 minima 0.340 vs 0.341 ns single-word, 7.183 vs 7.181 ns and 10.738 vs 10.740 ns multi-word. The unused sum folds away as expected. Also drops a guaranteed self-assignment in evolve_mode_contract_immediately_: both sides of the picture choice are the same member vector, so only the lazy-materialization side effect mattered. (The benches/ harness could not gate this -- its per-test medians shuffle by orders of magnitude in both directions between runs of identical code, so it was measured directly instead.) Assisted-by: ClaudeCode:claude-opus-5
`graph_size()` and `evolved_operator()` are rank-LOCAL (tests/conftest.py). Asserting on them over COMM_WORLD makes the outcome depend on which rank owns a term's hash partition, so both tests failed under `mpiexec` for reasons unrelated to what they cover. Assisted-by: ClaudeCode:claude-opus-5
The pare sweep ran two blocking `MPI_Alltoallv` rounds per layer to reach a foregone conclusion. `mark_replayed_d_targets` had already forced every cross-rank D target kept (the cos pass scales all of them), so the D pass's `keep_src || keep_tgt` was unconditionally true: round 1's received source-keep flags could not change any outcome, round 2 therefore always carried all-ones, and the B pass kept every remote source unconditionally. Decide both sides at the reachability level instead -- each rank reaches the same conclusion about its own endpoints without agreement -- and drop the exchange layout/pack/execute machinery it needed. 282 lines out, two collectives per layer out. Verified equivalent, not just plausible: `expectation_value_functional(pare_threshold=...)` values and `graph_size()` are byte-identical before/after across both pictures x 3 seeds x 4 thresholds at 1, 2 and 4 ranks. Assisted-by: ClaudeCode:claude-opus-5
…agnostically
The engine propagates both Majorana monomials and native Pauli strings, but the
notation below the Python API still spoke fermionic quantum chemistry: the
reference state was `slater_determinant` with everything derived from it prefixed
`hf_` (including `pauli_hf_phase`, a "Hartree-Fock phase" over a Pauli string),
the operand of nearly every generic and Pauli-specific function was named `maj`
(`pauli_rotation_sign` took two Pauli strings called `maj`/`new_maj`), and the
operator-dictionary type was `FermiOperatorMap`. A reader had to know those names
were historical, not semantic.
Renames:
slater_determinant(_bytes) -> initial_state(_bytes)
get_hf_mask / hf_mask -> initial_state_mask / state_mask
hf_phase (Majorana free fn) -> majorana_state_phase
pauli_hf_phase -> pauli_state_phase
A::hf_phase -> A::state_phase
algebra_hf_phase -> algebra_state_phase
algebra_score_hf -> algebra_score_state
hf_rows_/hf_vals_/ -> state_rows_/state_vals_/
hf_scored_rows_ state_scored_rows_
maj, new_maj, maj_pop, ... -> mono, new_mono, mono_pop, ...
append/read_majorana_words -> append/read_monomial_words
FermiOperatorMap -> OperatorDict
`maj` is deliberately kept inside MajoranaAlgebra.h and for `jw_majs` (a genuine
JW Majorana image), where it is accurate. Doc comments and the API-facing docs now
present a general product reference state, with Hartree-Fock as the chemistry
special case; the chemistry notebook is untouched. The `tests/data/*.msgpack`
fixtures keep their frozen `hartree_fock` key, read into a field named
`initial_state` (documented in tests/data/README.md).
BREAKING CHANGE: the `monoprop._core` constructor keyword `slater_determinant` is
now `initial_state`, and `operator_memory_breakdown()` reports
`initial_state_bytes` instead of `slater_determinant_bytes`. The public Python
front-ends (`MajoranaPropagator`, `PauliPropagator`) already said `initial_state`
and are unaffected.
Verified as a pure rename: same-commit, same-flags A/B over five cases (Majorana
and Pauli, Heisenberg and Schrodinger, non-empty initial states) is bit-identical
in every expectation value and gradient entry compared as hex floats, with the
renamed dict key the only difference. 492 passed / 8 skipped (pytest) and 188/188
ctest including the world=2 MPI variant.
Assisted-by: ClaudeCode:claude-opus-5
…adient 339c133 made the Heisenberg reference state sparse on the operator, but the evaluation functional threw that away: make_functional_ scattered a full dense VecD for BOTH functionals. The energy path touches the state at exactly one site (an inner product), and the gradient path copied the vector a second time into its scratch -- so two dense buffers were live per functional to serve one that needs none and one that needs a single shared one. Introduce EvalState as the operand of ev/ev_and_grad: sparse (owned ascending rows + unit +-1 phases) in the Heisenberg picture, dense in Schrodinger, where the state is the live evolved vector. It offers exactly the three things any consumer needs -- dot it (energy), scatter it into a mutable dense buffer (the gradient's in-place back-evolution), or ask which rows clear a paring threshold. The dense state now exists ONLY in the thread-local EvalScratch inside ev_and_grad_impl. So make_functional_ needs no energy-vs-gradient branch at all, energy-only runs never build one, and N functionals on a thread share one buffer instead of each owning one plus making a copy. On the 120-qubit Hubbard model that is 8.9 MiB per functional per shard off the energy path. Two simplifications fall out: - get_pared_graph is DELETED. It was only a threshold wrapper over the already-public pare_graph, and its `state` argument was entirely unused in the Schrodinger picture. The one caller now uses a free indices_above(VecD, double) plus pare_graph directly. - materialize_state() is off every library path (it stays as the tests' dense oracle; MPOperator must not depend on MPFunctions). BREAKING CHANGE: ev/ev_and_grad take an EvalState rather than a dense VecD state, and get_pared_graph is removed in favour of indices_above + pare_graph. Values are unchanged. The sparse dot omits terms that contribute an exact 0.0 and ascending rows preserve the dense summation order, so it is bit-identical for a finite operator; the gradient is bit-identical by construction (same dense buffer). Pinned end-to-end by sparse_energy_matches_the_dense_gradient_value_bit_exactly: the gradient path still takes its value from the dense inner_product while energy takes it from the sparse dot, over the same forward-evolved operator, so the two must agree exactly -- checked in both pictures, on the exact and the pared graph. (An operator holding +-Inf at an unscored row would have yielded NaN before and a finite value now. That is a robustness improvement, not a value change.) Two hazards handled explicitly: - scatter_into ASSIGNS rather than resizing. Its buffer is thread-local scratch reused across calls and across propagators, arriving with a previous back-evolution in it; resize+scatter silently corrupts every repeated gradient call. Two tests cover it. - A negative pare_threshold reaches this unvalidated from Python, and |0.0| > threshold holds there, so the dense scan keeps every index. EvalState::indices_above returns the full iota below zero to match, and keeps the per-entry check so the equivalence does not rest on the +-1 invariant. Validated: 193/193 serial ctest, 194/194 including world=2 on a DCGP compute node, 438 Python tests. Assisted-by: ClaudeCode:claude-opus-5
The prose below the public API had drifted from the code, and the drift was
concentrated in the parts hardest to reason about. Four classes of problem:
- A parallelism narrative with no runtime behind it. There is no tbb, no
OpenMP, no std::execution and no thread pool; the only threads are
ShardGroup's per-shard masters, each running the engine serially. ~25
comments across 13 files still described plain serial loops as "race-free",
"atomics-free", "chunked into tasks" or "cache-blocked into L1 sub-blocks".
A reader preserving a concurrency invariant that does not exist makes bad
changes. Deleted, keeping the real invariant where one was wrapped inside
the concurrency framing.
- Comments naming symbols that no longer exist: assemble_partners (the code
is GraphSink::finalize) and the types PrunedLayer / FoldLayer (there is one
Layer; the distinction is whether pruned_cos_ is engaged).
- Five comment styles serving a Doxygen build that does not exist -- no
Doxyfile, nothing in CMake, the justfile or CI. Every @brief and @param
produced zero output. Settled on bare /// one-liners in include/monoprop
(the installed API) and plain // everywhere else, and wrote that into
AGENTS.md so it does not regress.
- Duplication and narration: the B/D layout invariant restated 9x, "no
trailing barrier" 4x, the sparse-vs-dense state story 5x in one file, plus
comments narrating fixed bugs that git already records. Each invariant now
has one canonical home; other sites point at it or say nothing.
Load-bearing comments were tightened, not dropped: MPI ordering and deadlock
contracts, lifetime and aliasing rules, sign and bit conventions, and the
"why not the obvious thing" notes that carry a measured number.
Also drops the eight in-code paper citations and adds nanobind docstrings to
the bound methods that had none, so help() on the extension is no longer bare.
Attribution files (CITATION.cff, README, docs/) are deliberately untouched.
Comment lines 4609 -> 2997. Verified comment-only: the comment-stripped token
stream of every .h/.cpp is byte-identical to HEAD, and every .py has an
identical AST with docstring statements removed. binder.h is the sole exception
and holds only added docstring literals. 180/180 and 188/188 ctest (incl.
world=2), 492 pytest + 500 under MPI, 516 docstrings parse clean under griffe.
Assisted-by: ClaudeCode:claude-opus-5
…rmatted `clang-format --dry-run --Werror` failed on 17 sites that predate this commit: MonomialPropagatorImpl.h (2), TestUtilities.h (2) and mpfunctions.cpp (13). All three files were touched by 6c91972 / 4954507 and went in unformatted, so the pre-commit hook and CI would reject the next change to any of them. Purely `clang-format -i` output. Verified whitespace-only: the comment-stripped, whitespace-stripped token stream of each file is byte-identical to its previous state. 186/186 and 194/194 ctest. Assisted-by: ClaudeCode:claude-opus-5
Member
Author
|
Ok, the merging got messed up. Fixing it |
|
robertodr
reviewed
Jul 29, 2026
Co-authored-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com> Signed-off-by: Ludmila Botelho <39570941+ludmilaasb@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
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.
By opening this PR I confirm that I have read CONTRIBUTING.md and I agree to the terms of the Contributor License Agreement.
Summary
This PR updates the 3rd party benchmark scripts. The SPECS are the same, the number of quibits is increased for Pauli Propagation and the scripts for comparing with MajoranaPropagation.jl were improved and simplified.
Changes
Checklist
docs/,CONTRIBUTING.md) if neededCHANGELOG/ release notes updated if applicableAI/LLM disclosure