Uh oh!
There was an error while loading. Please reload this page.
Codex/pr180 symmetry design - #193
Merged
david-pl merged 14 commits intoSep 1, 2026
Merged
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the ppvm-pauli-sum translation-symmetry implementation into a dedicated symmetry/ module split (group / merge / momentum / tests), while also tightening validation logic for momentum-sector handling and applying a handful of small cleanup tweaks in examples/benches/tests across the workspace.
Changes:
- Split
ppvm_pauli_sum::symmetryinto focused submodules (group,merge,momentum) and update the public re-exports. - Add/expand symmetry test coverage, including a new public API “imports remain available” regression test.
- Minor micro-optimizations / clarity tweaks in examples, benches, and existing tests (e.g.,
hash_one,to_vec(), iterator usage).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/ppvm-tableau/src/data.rs | Simplifies test assertions/collection for coefficient vectors. |
| crates/ppvm-tableau/examples/profile_measure_all.rs | Avoids indexed mutation by iterating per_qubit_runs mutably with enumerate(). |
| crates/ppvm-tableau/examples/profile_measure_all_flame.rs | Small doc-comment formatting tweak. |
| crates/ppvm-tableau-sum/examples/truncation-scaling.rs | Minor readability + Clippy suppression for a multi-arg helper. |
| crates/ppvm-tableau-sum/examples/msd-noisy-compare.rs | Adds Clippy suppression for a multi-arg helper. |
| crates/ppvm-pauli-sum/tests/symmetry_api.rs | New regression test ensuring symmetry APIs remain publicly importable/usable. |
| crates/ppvm-pauli-sum/src/symmetry/mod.rs | New symmetry module root with docs + public re-exports. |
| crates/ppvm-pauli-sum/src/symmetry/group.rs | Implements TranslationGroup and orbit/canonicalization traversal infrastructure. |
| crates/ppvm-pauli-sum/src/symmetry/merge.rs | Implements real-coefficient canonicalization and PauliSum symmetry merge. |
| crates/ppvm-pauli-sum/src/symmetry/momentum.rs | Implements momentum characters, complex projection canonicalization, and sector validation errors. |
| crates/ppvm-pauli-sum/src/symmetry/tests.rs | New comprehensive test suite for group/orbit/momentum/merge behavior. |
| crates/ppvm-pauli-sum/src/symmetry.rs | Removes the prior monolithic symmetry implementation (replaced by the split module). |
| crates/ppvm-pauli-sum/examples/hash_quality.rs | Uses BuildHasher::hash_one for simpler hash extraction in the analysis example. |
| crates/ppvm-pauli-sum/benches/truncation-weight.rs | Avoids cloning Word where it is Copy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+321
to
+325
| } => write!( | ||
| f, | ||
| "input not in target momentum sector: orbit rep {rep} expected c={expected:?}, \ | ||
| but orbit member {offending_pauli} (shift {shift:?}) has c={actual:?}" | ||
| ), |
4 tasks
Uh oh!
There was an error while loading. Please reload this page.
david-pl added a commit
that referenced
this pull request
Sep 2, 2026
## Summary - Same kind of cleanup as #193 did for #180: split the 1300-line `ppvm-lindblad` `lib.rs` / `LindbladSpec` impl into `word`, `algebra`, `spec`, `basis`, `step`, and `tests`, keeping the public `ppvm_lindblad::*` API. - Collapses the timed/untimed `pc_step` copy, demotes `ppvm-pauli-sum` to a dev-dependency, drops unused `approx`, strips 182-forward `orbit_rep` doc links, and forwards `admit_basis` / `tau_add` on the Python string `pc_step`. - **Base is #181 (`split/2-ctpp-core`) on purpose.** #193 replaces #180 and is not an ancestor of #181; retargeting this at #193 now would mix the whole CTPP crate into the diff. After #193 merges and #181 is rebased onto it, this PR rebases with #181. ## Test plan - [x] `cargo test -p ppvm-lindblad` (6 passed) - [x] `cargo clippy -p ppvm-lindblad --all-targets -- -D warnings` - [x] `cargo check -p ppvm-python-native` - [ ] Rebase onto #181 after #181 is retargeted onto #193 Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.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 freeto 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.
This implements the split suggested in #180 (comment) and fixes some issues that surfaced during the split or from copilot findings.
Should be merged before #181
cc @AlexSchuckert