Uh oh!
There was an error while loading. Please reload this page.
Initial support for leakage - #195
Conversation
|
There was a problem hiding this comment.
🟡 Changes recommended
A compile-breaking invalid import was introduced (use std::debug_assert;) and should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds initial leakage support to the GeneralizedTableau simulation stack by introducing a per-qubit status enum (live/leaked/lost), extending the Stim parser/executor to handle I_ERROR[leakage](p0, p1), and exposing leakage state through the Python bindings.
Changes:
- Introduces
QubitStatusto representLive/Leaked/Lost, and updates gate/noise behavior to skip non-live qubits while preserving distinct measurement behavior for leakage vs loss. - Extends
stim-parser+ppvm-stimto parse/lower/print/execute theleakagetag onI_ERROR. - Exposes
is_leaked()/leakage_values()through Python bindings and adds Python/Rust tests for leakage behavior.
File summaries
| File | Description |
|---|---|
| ppvm-python/test/generalized_tableau/test_stim.py | Adds Stim-driven leakage execution tests |
| ppvm-python/test/generalized_tableau/test_loss.py | Adds Python API tests for leakage state helpers |
| ppvm-python/src/ppvm/generalized_tableau.py | Adds Python-facing is_leaked / leakage_values |
| ppvm-python/src/ppvm/_core.pyi | Updates core stub with leakage APIs |
| crates/stim-parser/tests/tags.rs | Tests parsing of leakage tag args |
| crates/stim-parser/tests/roundtrip.rs | Adds leakage to roundtrip corpus |
| crates/stim-parser/tests/proptest_parse.rs | Adds [leakage] token to proptests |
| crates/stim-parser/tests/proptest_ast.rs | Generates ExtendedInstruction::Leakage in proptests |
| crates/stim-parser/tests/extended.rs | Tests lowering/tag validation for leakage |
| crates/stim-parser/src/print/mod.rs | Prints leakage instruction canonically |
| crates/stim-parser/src/pipeline/lower.rs | Lowers I_ERROR[leakage] into extended AST |
| crates/stim-parser/src/ast/extended.rs | Adds ExtendedInstruction::Leakage variant |
| crates/ppvm-traits/src/traits/noise.rs | Adds leakage-related noise traits |
| crates/ppvm-traits/src/traits/mod.rs | Re-exports leakage traits from traits module |
| crates/ppvm-tableau/tests/gates.rs | Updates tests for qubit_status replacing is_lost |
| crates/ppvm-tableau/src/qubit_status.rs | New enum + helpers for live/leaked/lost handling |
| crates/ppvm-tableau/src/noise.rs | Implements leakage channel + updates loss logic for status |
| crates/ppvm-tableau/src/measure.rs | Updates lost-measurement check for status |
| crates/ppvm-tableau/src/measure_all.rs | Updates measure-all lost checks for status |
| crates/ppvm-tableau/src/lib.rs | Exposes qubit_status module + prelude export |
| crates/ppvm-tableau/src/gates/tgate.rs | Skips T gates on inactive (lost/leaked) qubits |
| crates/ppvm-tableau/src/gates/rot2.rs | Updates rot2 fallback/skip logic for inactive qubits |
| crates/ppvm-tableau/src/gates/rot1.rs | Skips rot1 on inactive qubits |
| crates/ppvm-tableau/src/gates/reset.rs | Prevents reset from re-zeroing leaked qubits |
| crates/ppvm-tableau/src/gates/clifford.rs | Skips Clifford ops on inactive qubits; batch filtering updates |
| crates/ppvm-tableau/src/display.rs | Updates display output to show qubit_status |
| crates/ppvm-tableau/src/data.rs | Replaces is_lost storage with qubit_status |
| crates/ppvm-tableau-sum/src/storage/vec.rs | Adapts storage sizing to qubit_status |
| crates/ppvm-tableau-sum/src/storage/mod.rs | Adapts hashing/equality to qubit_status |
| crates/ppvm-tableau-sum/src/storage/map.rs | Adapts mask sizing to qubit_status |
| crates/ppvm-tableau-sum/src/noise.rs | Updates loss branching to write QubitStatus::Lost |
| crates/ppvm-tableau-sum/src/measure.rs | Updates lost checks to use is_lost() method |
| crates/ppvm-tableau-sum/src/data.rs | Updates tests for qubit_status instead of is_lost |
| crates/ppvm-tableau-sum/benches/parallelization_test.rs | Updates bench to use is_lost() |
| crates/ppvm-stim/tests/executor.rs | Adds executor tests for leakage behavior |
| crates/ppvm-stim/src/validate.rs | Allows leakage instruction through validation |
| crates/ppvm-stim/src/executor.rs | Executes leakage instructions via leakage_channel |
| crates/ppvm-stim/benches/stim-circuits.rs | Counts leakage targets for required-qubits computation |
| crates/ppvm-python-native/src/interface_tableau.rs | Exposes leakage state through Python native interface |
| crates/ppvm-python-native/ppvm_python_native.pyi | Updates native stub with leakage APIs |
Review details
- Files reviewed: 40/40 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core simulator state semantics across multiple crates/languages (tableau, stim parsing/execution, Python bindings), warranting final human review despite strong test additions.
Review details
- Files reviewed: 40/40 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core simulator semantics across multiple Rust crates and the Python binding surface, so a final human review is needed to confirm correctness of the leakage model end-to-end.
Review details
Suppressed comments (1)
crates/ppvm-tableau/src/data.rs:789
- Same as above:
any_lostnow includes leaked qubits viais_inactive, so the name is inaccurate and can cause confusion when reasoning about leakage behavior.
let any_lost = (0..count).any(|i| {
let c = word_c * bits_per_word + base_bit_c + i;
let t = word_t * bits_per_word + base_bit_t + i;
self.is_inactive(c) || self.is_inactive(t)
});
- Files reviewed: 40/40 changed files
- Comments generated: 2
- Review effort level: Lite
| // Check if any qubit in the range is lost or leaked | ||
| let any_lost = | ||
| (0..count).any(|i| self.is_lost[base + i] || self.is_lost[base + offset + i]); | ||
| (0..count).any(|i| self.is_inactive(base + i) || self.is_inactive(base + offset + i)); | ||
| if !any_lost { | ||
| self.tableau.cz_block_pairs(base, offset, count); |
| // record entry it pushed (mirrors `loss_channel`). | ||
| let m = self | ||
| .measure(addr0) | ||
| .expect("Loss was checked before, this should be unreachable"); |
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🟢 Approval recommended
The leakage feature is implemented end-to-end (parser → executor → tableau → Python bindings) with targeted tests covering the new behavior and updated loss/leakage interactions.
Review details
- Files reviewed: 40/40 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The change spans core simulator semantics, Stim parsing/execution, and Python bindings, so it warrants final human review despite strong test coverage.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
crates/ppvm-tableau-sum/src/storage/mod.rs:160
- Doc/comments for
phase_loss_hashstill refer tois_lost, but the code now usesqubit_status. This is misleading (andqubit_statuscan represent more than just loss). Update the wording to reflect that this hash tracks the loss state viaqubit_statusand fix the inlinelen()comment accordingly.
This issue also appears on line 317 of the same file.
crates/ppvm-tableau-sum/src/storage/mod.rs:319
- The
BranchMutation::Lossdoc comment still mentions the removedis_lostfield; it should describe the newqubit_statusrepresentation so future readers don't look for a non-existent boolean vector.
BranchMutation::Loss { q } => {
tab.qubit_status[q] = QubitStatus::Lost;
}
- Files reviewed: 41/41 changed files
- Comments generated: 0 new
- Review effort level: Lite
Adds support for leakage on
GeneralizedTableauby replacing theis_lost: Vec<bool>by an enum. Leaked qubits are fixed to either 0 or 1 depending on the given probabilities and are detected as such.Note, that
GeneralizedTableauSumdoes not support leakage yet.