Repository navigation
Conversation
_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>
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. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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
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.
| 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)| ) -> 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".
Stacked on #222. Review that one first; this PR's diff is against its branch.
Summary
These are the maintainability follow-ups from the statevector review. Every commit leaves the samples bit-identical. The first commit adds a test, and the other three are behaviour-preserving refactors.
_Relations. This is the GF(2) elimination that turns classical spiders into output parities. Until now it was only covered through whole-circuit distributions. The new test builds 300 random small systems, sums the sign over the eliminated variables by enumeration, and checks the reduced rows. It also checks that the reduction raises exactly when no such rows exist. It catches each of three one-line mutations of_substituteandeliminate.lane_programunpacked before packing them into uint64 lane words. The tape now keeps a boolean column per symbol, sounpack_symbols, the uint32 packer andStatevectorComponent.num_wordsare gone.run_lanes. A pruning now writes the dying lanes' shots' outputs right away and drops those shots from the live list. Before, they waited in a grave buffer until the next random measurement or the end. This removesgraves,grave_at,dead_shot,dead_grave, the per-entry hidden masks and the-1 - gravecodes. The per-shot work lives in_finish_shot, which takes only the arrays it reads rather than theLaneProgram, for the same reason as_lane_key.lane_bytesnow counts two rows of lane symbols instead of four.CompiledMeasurementSampler.sampleandCompiledDetectorSampler.sampleboth call_sample_outputs(shots, batch_size, columns, bit_packed, mask, referenced). It picks the shot-word or stabilizer-rank path once. Each path handles its own reference sample and mask screening, and the inline reference and mask logic inCompiledDetectorSampler.sampleis gone. The stabilizer-rank batching methods stay where they were, since tests patch them.Verification
bit_packedand observable layout. The hash is identical before and after.🤖 Generated with Claude Code