Repository navigation
Simplify the statevector executor and the sampler's backend dispatch - #223
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the statevector simplification and shared sampler dispatch refactor.
No open issues 🎉
What changed in this PR
This PR builds on #222’s postselection fixes to simplify statevector execution and centralize sampler backend dispatch, while preserving sampling behavior.
Changes:
- Adds 300 randomized brute-force checks for
_Relationselimination and reduction. - Replaces packed tape symbols with boolean columns and finalizes pruned shots immediately, removing grave buffers.
- Routes both public samplers through a shared
_sample_outputsentry point.
| File | Description |
|---|---|
test/unit/statevector/test_shots.py |
Updates symbol handling and lane-memory expectations. |
test/unit/statevector/test_program.py |
Verifies boolean symbol columns instead of packed words. |
test/unit/statevector/test_pbc.py |
Adds brute-force validation of relation reduction. |
test/helpers/statevector.py |
Reads boolean output symbols directly. |
src/tsim/statevector/shots.py |
Builds shot layouts from boolean symbol columns. |
src/tsim/statevector/shared.py |
Removes symbol unpacking and grave buffers; finalizes pruned shots immediately. |
src/tsim/statevector/program.py |
Lowers plans into boolean-column tapes. |
src/tsim/sampler.py |
Centralizes backend dispatch, reference handling, and mask screening. |
CHANGELOG.md |
Documents the behavior-preserving internal refactors. |
Review effort: Balanced
☂️ Code Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
rafaelha
left a comment
There was a problem hiding this comment.
I checked that samples are bit-identical, ran timings, and tried mutations on the new test.
Bit-identity holds. I wrote my own harness and ran it against this branch and the base (fix/statevector-discard-consistency) with the same seeds. Both trees produced identical hashes in every case:
active_statevector: 74,304 cases over the 8 helper circuits plus 40 random circuits. The circuits have T gates, mid-circuitM/MR/MX, X and depolarizing noise, and several components. Each circuit ran with sharing on and off, 2 seeds, and 4 masks (none, all, random, empty). Each mask ran with all 4 reference-flag combinations,bit_packedon and off, 4 observable layouts and batch sizes None/1/37. The harness also checks the measurement sampler,shots=0, and repeated calls on one sampler.- Lane statistics (
groups,peak lanes,lanes,dead) were also identical on 60 more random circuits and 2 hand-written ones. That run covers pruning before the first action, every lane dying, and a pruning right after a split (498 programs). cat5: 4,788 cases were identical. On theDIRECT_FLIPScircuit, every one of the 8 masks with every reference combination chose the same path (screened or_sample_batches_with_postselection) and gave the same samples.
I also read the new _prune for the alive update, coins for pruned shots, hidden at action -1, and code_row remapping across splits. I found no behaviour change. The relevant unit tests pass (612).
Two things are worth fixing. Details are inline.
- The non-inlined per-shot
_finish_shotcall makes the shared, unpruned path about 1.7x slower on one thread. - The
_Relationstest does not check the echelon form, so tworeducemutations pass it.
Review by a Claude subagent.
| hidden = np.zeros(len(out_words), dtype=np.uint64) | ||
| for i in range(n): | ||
| shot = live[i] | ||
| _finish_shot( |
There was a problem hiding this comment.
Per-shot slowdown on the sharing path. This loop now makes one out-of-line call to the cached _finish_shot per shot. It also builds a pattern[...] row view per shot and runs an extra &= ~hidden pass even though hidden is all zeros. Before, all of this was inline.
I timed it with NUMBA_NUM_THREADS=1, best of 5, on a width-7 component with 12 repeated-measurement detectors and 2M shots, sharing on:
- No mask: base 0.085 s, this PR 0.145 s.
- Every second detector masked: base 0.105 s, this PR 0.14 s.
As an experiment I inlined only this final loop: e = code_row[row[i]], XOR pattern[e, w], then the coin loop, with no hidden pass. That brought the no-mask case back to 0.088 s. With 24 threads the difference is within noise at this size, and the PR's width-7, 400k-shot benchmark doesn't show it.
Suggested fix: inline the loop here and keep _finish_shot for _prune only. If you keep the helper, pass the 2-D pattern plus the row index, and skip the hidden pass when nothing is hidden.
There was a problem hiding this comment.
Fixed in ad8d042. The end-of-run loop in run_lanes is inline again and has no hidden pass; _finish_shot now serves only pruned shots. On one thread, a width-7 component with 12 detectors, 2M shots and sharing on now takes 0.031 s, against 0.033 s on #222 and 0.045 s before this fix. Sample hashes are identical across all three. Claude
| except ValueError: | ||
| assert not all(representable(total) for total in weights) | ||
| return | ||
| for f, total in enumerate(weights): |
There was a problem hiding this comment.
The test checks the solution set of rows but not its form. It never checks that rows is in reduced echelon form with pivots taken in order. That form is what reduce promises and what callers rely on. Two one-line mutations of reduce pass all 300 cases, and each is caught only by test_ops_match_stabilizer_rank[True-midcircuit_feedforward]:
- Dropping the back-substitution loop (
for bit in _bits(mask & below): ...). - Reversing pivot priority (
position = {v: i ...}andvariable = list(order)).
Adding these assertions before this loop catches both mutants (10 and 18 failures). Unmutated, all 300 cases pass:
for p, mask, _ in rows:
assert mask >> p & 1
assert min((v for v in kept if mask >> v & 1), key=kept.index) == p
assert not any(m >> p & 1 for q, m, _ in rows if q != p)There was a problem hiding this comment.
Added in ad8d042, then tightened in 04b7ae3. The test now checks that pivots are unique and in kept order, that every row term is in kept with the pivot as its first term in kept order, and that no other row contains a pivot. Dropping the back-substitution fails 18 of the 300 cases, and reversing the pivot priority fails 10. Claude
| ) -> list[np.ndarray]: | ||
| """Return the output bits ``columns`` of ``shots`` shots (``ShotSampler.sample``).""" | ||
| assert self._shots is not None | ||
| """Return the output bits ``columns`` of ``shots`` shots, one array per list. |
There was a problem hiding this comment.
Nit: "one array per list" should probably read "one array per entry of columns".
There was a problem hiding this comment.
Reworded in ad8d042: "an array per entry of columns". Claude
rafaelha
left a comment
There was a problem hiding this comment.
Do we need this PR? It isn't required: no fix or feature depends on it. I still think it's worth merging once the open items below are fixed, after #222.
- The
_Relationstest is cheap and covers the part of the reader that is hardest to get right. Keep it, with the echelon-form assertions from the earlier comment. - The single symbol encoding removes a pack/unpack round trip for little cost. The tape's symbol rows grow to 1.3 MB on d=5 cultivation, from about 0.16 MB packed.
- The single
_sample_outputsis the most useful commit. It removes the second copy of mask screening and reference handling fromCompiledDetectorSampler.sample. #219'ssample_postselectedbuildsreferenced/flipthe same way again, so whichever PR lands second should reuse it. - Dropping the graves is the weakest trade.
shared.pygrows by about 60 lines, and_prunenow takes 16 arguments. In exchange it drops the negative codes, the deferred dead-shot lists and half of the lane-symbol memory. That's worth it only if the end-of-run loop is inlined again (see the earlier comment). Without that fix I'd leave this commit out.
Extra checks on c221fea. I timed cultivation with every detector postselected, 2M shots, best of 5. Pruning now scans the live shots each time a lane dies, and I wanted to see whether that costs anything. It doesn't, and the samples are identical:
CX_d1=5_cult |
Base | This PR |
|---|---|---|
| 1 thread, sharing on | 1.38 s | 1.31 s |
| 1 thread, sharing off | 1.87 s | 1.82 s |
| All threads, sharing on | 0.18 s | 0.19 s |
| All threads, sharing off | 0.25 s | 0.26 s |
CX_d1=3_cult with 200k shots also had identical hashes and times.
Before merge:
- Fix the two earlier comments: inline the final shot loop, and add the echelon-form assertions.
- Rebase onto
mainonce #222 lands.CHANGELOG.mdandtest_pbc.pyconflict with #224.
I updated the PR description to match.
Review by Claude.
c221fea to
97c95ca
Compare
_Relations turns a component's classical spiders into output parities and was only covered through whole-circuit distributions. The test sums the sign over the eliminated variables of random small systems and checks the reduced rows, and that the reduction raises exactly when no such rows exist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The plan's symbol sets were packed into uint32 words on the tape, which the lane program unpacked again before packing them into uint64 lane words. The tape now holds a column per symbol, so lane_program packs once and unpack_symbols and the uint32 packer are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
run_lanes kept dying lanes in a grave buffer and found their shots at the next random measurement or at the end, with codes that pointed into the graves, per-shot grave lists and a hidden mask per entry. A pruning now writes the dying lanes' shots' outputs right away and drops them from the live list, so the graves and their bookkeeping are gone. The samples and lane counts are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CompiledMeasurementSampler.sample and CompiledDetectorSampler.sample each branched on the shot-word and stabilizer-rank backends, and the detector sampler prepared masks and reference samples for both inline. _sample_outputs now takes the columns, the mask and the referenced outputs, and picks the backend once; the stabilizer-rank path keeps its own mask screening and reference handling. The samples are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d rows run_lanes finished every live shot through an out-of-line _finish_shot call with an all-zero hidden mask, which made the shared, unpruned path about 1.4x slower on one thread. The end-of-run loop is inline again, and _finish_shot serves only pruned shots. The _Relations brute-force test now checks that the rows are in reduced echelon form with pivots in kept order, which catches dropping the back-substitution or reversing the pivot priority. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
04b7ae3 to
2912950
Compare
…es (#259) ## Summary Fixes the postselection slowdown that #252, #222 and #223 brought to `strategy="active_statevector"` before 1.0.0. Samples are bit-for-bit those of `main`. #252 made `CompiledDetectorSampler.sample(postselection_mask=...)` return only the shots that pass. The statevector path found them with one NumPy pass over every shot's words after sampling, on one thread. It also still cleared (#222) and finished (#223) the discarded shots, though nothing reads them any more. Against `main` at 5af2731 (before these PRs), on B850 (Ryzen 9 9900X), the postselected cells fell to: | cell, postselected | 1 thread | 10 threads | | --- | --- | --- | | `surface_d7_r7` | 0.74x | **0.25x** | | `cultivation_d5` | 0.80x | **0.53x** | | `coherent_d3_r1` | **0.17x** | **0.19x** | | `coherent_d5_r1` | **0.31x** | **0.23x** | | `coherent_d3_r3` | 0.85x | | ## Changes - **Survivors from `alive`.** Each chunk's `_run_chunk` already knows which shots passed: the screen of postselected direct detectors, then each component's pruning. It now hands that back, compacts its survivors in place on its own thread (`_compact`), and a numba `_join` puts the chunks together. `_shrink` gives the rest of the buffer back, so a view of a few survivors doesn't keep a whole call's words alive. `ShotPlan.rejected` is now a bool. - **Programs without components** (surface, coherent r=1). These postselect direct detectors only, all of them in the screen. `ChannelSampler.sample_screened_words` screens each noise block inside the noise kernel, while the block is still in cache. A parallel `_copy_kept` then copies the survivors out of scratch words that the sampler reuses (at most 32 MiB). Fresh scratch memory each call cost about 0.4 ms of page faults per 2 MB on these 0.2 ms calls. Both loops have branchless paths for 1-word rows and for blocks that keep most of their shots. - **Discarded shots are no longer finished.** `_prune` drops the dying lanes' shots from `live` and `alive`, without `_finish_shot`'s pattern XOR, coin draws and hidden clearing. `_screen`'s `&= kept` and the `pruned_kept` clearing are gone too. This is the follow-up #252 listed. #252 kept the pattern XOR because its filter read the words; this filter reads `alive`. ## Benchmarks `near-clifford-benchmarks` harness on B850, sharing on, bit-packed. Shot data is the simulator's call alone. "Counting included" adds the harness's counting, which for 5af2731 includes dropping the rejected shots itself. | cell, postselected | threads | shot data vs 5af2731 | counting included | | --- | --- | --- | --- | | `surface_d7_r7` | 1 / 10 | 0.97x / 0.94x | 1.11x / 1.10x | | `distillation_d3` | 1 / 10 | 1.04x / 1.44x | 1.11x / 1.50x | | `distillation_d5` | 1 / 10 | 1.04x / 1.35x | 1.06x / 1.38x | | `cultivation_d3` | 1 / 10 | 1.06x / 1.37x | 1.16x / 1.44x | | `cultivation_d5` | 1 / 10 | 1.04x / 1.19x | 1.09x / 1.22x | | `cultivation_d5_end2end` | 1 / 10 | 1.01x / 1.05x | 1.01x / 1.05x | | `cultivation_d7` | 1 / 10 | 1.09x / 1.14x | 1.11x / 1.16x | | `coherent_d3_r3` | 1 / 10 | 0.99x / 1.69x | 1.06x / 1.78x | | `coherent_d5_r5` | 1 / 10 | 1.03x / 1.00x | 1.03x / 1.00x | | `coherent_d3_r1` | 1 / 10 | 0.50x / 0.57x | 1.05x / 0.74x | | `coherent_d5_r1` | 1 / 10 | 0.69x / 0.61x | 1.34x / 0.86x | On the two K=0 coherent r=1 circuits, a call is 0.1–0.2 ms of noise sampling. Returning only the survivors costs one extra read and write of the call's words there. 5af2731 skipped that work and left it to the caller, so the shot-data rate can't match. Counting included, these cells are at 0.74–1.34x of 5af2731. Unmasked calls take the same paths as before. ## Testing - **Bit-for-bit equality with `main` (aa9d6a2):** 880 configurations, at 1 and 10 threads, and again with the part size forced to 4096 (the multi-part path). - 11 circuits: the benchmark circuits, a constant-detector circuit and a small T component. - Masks: all, none, half, one, zero. - Bool and bit-packed output, four call sizes including 0 and 1, `batch_size`, the reference sample, and appended and separate observables. - A sampler reused across calls of shrinking and growing sizes (scratch reuse). - **Updated tests:** two `test_shared.py` tests read discards off discarded rows' contents. - `test_pruning_discards_exactly_the_postselected_failures` now reads `alive`. - `test_pruned_shots_report_false_after_their_pruning_action` is replaced by `test_pruning_keeps_exactly_the_shots_an_unpruned_run_accepts`, an exact check: with the same seed, pruning keeps exactly the shots that an unpruned run accepts, with identical rows. - **New unit tests** for `_copy_kept` (every branch, serial and parallel), `_join`, `_compact`, scratch reuse across shrinking and growing calls, and pickling a sampler after a screened call (as on `main`; the scratch lock is left out). - **Suite:** `uv run pytest`: 2465 passed. `test_kernels.py::test_the_calling_thread_keeps_parallel_kernels_after_run_tasks` fails on this machine on unmodified `main` too, and was deselected. `pre-commit run --all-files`, including pyright, is clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Stacked on #222 (stack #245). This PR's diff is against that branch.
Summary
This PR changes no behaviour. It is cleanup from the statevector review, made of one new test and three refactors. Samples are bit-identical after every commit. No feature or fix depends on it, so it can wait behind #222 and #219.
_Relations(test_pbc.py)._Relationsis the GF(2) elimination that turns classical spiders into output parities. Until now it was only covered through whole-circuit distributions. The test builds 300 random small systems and sums the sign over the eliminated variables by enumeration. It then checks the reduced rows, and that the reduction raises exactly when no such rows exist.program.py,shared.py,shots.py; 13 lines added, 32 removed). The tape used to pack symbol sets into uint32 words, whichlane_programunpacked again before packing them into uint64 lane words. The tape now keeps a boolean column per symbol._pack,unpack_symbolsandStatevectorComponent.num_wordsare gone, andnum_symbolsreplacesnum_words. The tape grows slightly: its symbol rows take 1.3 MB on d=5 cultivation, up from about 0.16 MB.run_lanes(shared.py; 161 lines added, 100 removed). A pruning now finishes the dying lanes' shots right away, with their outputs, coins and hidden bits, and drops them fromlive. Before, they waited in a grave buffer until the next random measurement or the end of the run. This removesgraves,grave_at,dead_shot,dead_grave, the per-entry hidden masks and the-1 - gravecodes. The per-shot work moves into_finish_shot, andlane_bytescounts two rows of lane symbols per lane instead of four.sampler.py).CompiledMeasurementSampler.sampleandCompiledDetectorSampler.sampleboth call_sample_outputs(shots, batch_size, columns, bit_packed, mask, referenced). That method picks the shot-word or stabilizer-rank path once. The stabilizer-rank path's mask screening and reference XOR move into_sample_stabilizer_rank_outputs, soCompiledDetectorSampler.sampleno longer duplicates them.Open before merge
ad8d042: the end-of-run loop is inline again and has nohiddenpass, and_finish_shotserves only pruned shots. On one thread, a width-7 component with 12 detectors, 2M shots and sharing on takes 0.031 s, against 0.033 s on Make postselection discards consistent across programs and components #222 and 0.045 s before this fix. Sample hashes are identical._Relationstest checks the reduced echelon form (ad8d042,04b7ae3): pivots are unique and inkeptorder, every row term is inkept, and no other row contains a pivot. Dropping the back-substitution and reversing the pivot priority both fail it now._sample_outputs, fixed inad8d042.mainat6ee9e1b(Keep PauliString signs for every Pauli-product gate in Circuit.append #240), and the stack merges them together.sample_postselectedbuildsreferenced/flipthe same way_sample_outputsnow does, so whichever PR lands second can reuse that code.Verification
After the rebase onto
mainat6ee9e1b, the full suite passes locally (2367 passed) and pyright is clean.CX_d1=5_cult, with every detector postselected, gives the same hash as Make postselection discards consistent across programs and components #222 with sharing on and off. Before the rebase, CI was green onc221fea.Executor commits. A snapshot hashed exact samples and lane statistics for 48 circuits, with sharing on and off and three masks, plus two prune-heavy workloads. The hash is identical before and after.
Sampler commit. A second snapshot covered both strategies and every combination of mask, both reference flags,
bit_packedand observable layout. The hash is identical before and after.Review harness. 74,304
active_statevectorcases and 4,788cat5cases produced identical hashes against the base branch. So did lane statistics on 62 more circuits.Timing. The table below uses cultivation, with every detector postselected, best of 5. Sample hashes are identical.
CX_d1=5_cult, 2M shots, 1 thread, sharing onCX_d1=5_cult, 2M shots, 1 thread, sharing offCX_d1=5_cult, 2M shots, all threads, sharing onCX_d1=5_cult, 2M shots, all threads, sharing offPruning now scans the live shots each time a lane dies, instead of waiting for the next random measurement. That cost doesn't show up even on this prune-heavy workload. The only slowdown measured was the single-thread case without pruning, now fixed.
🤖 Generated with Claude Code