Skip to content

Translation symmetry: TranslationGroup + orbit-representative Pauli-sum merging - #180

Open
AlexSchuckert wants to merge 5 commits into
mainfrom
split/1-translation-symmetry
Open

Translation symmetry: TranslationGroup + orbit-representative Pauli-sum merging#180
AlexSchuckert wants to merge 5 commits into
mainfrom
split/1-translation-symmetry

Conversation

@AlexSchuckert

Copy link
Copy Markdown
Collaborator

PR 1 of 4 splitting #178 into reviewable chunks (this → CTPP core → symmetry-merged evolution → ledgers). Full development history: branch continuous-time-pauli-propagation.

Adds ppvm_pauli_sum::symmetry (pure Rust, no new dependencies):

  • TranslationGroup: finite abelian permutation groups — 1D chains, 2D/3D tori, multi-leg ladders, or arbitrary generator lists — with lex-min orbit canonicalization, shift counters, and momentum-sector characters χ_k(g).
  • k = 0 merging: canonicalize_pauli_sum (Vec-pair form used by the upcoming adaptive evolution) and symmetry_merge_pauli_sum (PauliSum form). Preserves all G-invariant expectation values for G-commuting dynamics (Theorem 1 of Teng, Chang, Rudolph & Holmes, arXiv:2512.12094).
  • k ≠ 0: canonicalize_pauli_sum_complex (character-weighted projection with 1/|G| normalization) and check_momentum_sector to validate inputs before projecting (silent projection discards physics).

Tests: canonicalization/orbit properties on chains, tori, ladders; merge correctness; momentum-eigenstate round trips; a Trotter end-to-end check that per-step merging matches merge-at-end in the dt → 0 limit.

🤖 Generated with Claude Code

…erging

TranslationGroup (finite abelian permutation groups: 1D chains, 2D/3D
tori, multi-leg ladders, arbitrary generators) with lex-min orbit
canonicalization, shift counters, and momentum-sector characters.
Pauli-sum merging in both sectors: canonicalize_pauli_sum /
symmetry_merge_pauli_sum (k=0, real coefficients) and
canonicalize_pauli_sum_complex (k≠0, character-weighted projection,
1/|G| normalization), plus check_momentum_sector to validate an input
before projecting. Following Teng, Chang, Rudolph & Holmes
(arXiv:2512.12094).

Split 1/4 of the CTPP work (see PR body for the stack); full
development history on branch continuous-time-pauli-propagation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://QuEraComputing.github.io/ppvm/pr-preview/pr-180/

Built to branch gh-pages at 2026-09-01 14:58 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Rust-only symmetry layer to ppvm-pauli-sum for translation-invariant (and momentum-resolved) Pauli-sum canonicalization/merging, intended to support upcoming continuous-time Pauli propagation work (#178 split).

Changes:

  • Introduces ppvm_pauli_sum::symmetry with TranslationGroup plus orbit-canonicalization utilities.
  • Adds real (k=0) and complex/momentum-sector merging/canonicalization helpers, along with momentum-sector validation.
  • Exposes the new module from ppvm-pauli-sum’s public API and adds a comprehensive test suite for canonicalization/merge behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
crates/ppvm-pauli-sum/src/symmetry.rs New symmetry module: translation group model, orbit canonicalization, (momentum) merging, and tests.
crates/ppvm-pauli-sum/src/lib.rs Exposes the new symmetry module publicly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 16, 2026 12:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
@david-pl

david-pl commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

@AlexSchuckert I think this is a pretty good boiled down PR here. I just saw that the single file here was pretty large.

codex suggested some semantic split, see below.

Codex output:

symmetry.rs is approaching 1,000 lines, and I think there is a worthwhile semantic split here beyond simply moving code around. I would suggest preserving the existing ppvm_pauli_sum::symmetry::* API through re-exports, but organizing the implementation as:

symmetry/
├── mod.rs       # module docs and public re-exports
├── group.rs     # TranslationGroup, constructors, orbit/canonicalization
├── merge.rs     # k=0 Vec/PauliSum orbit merging
├── momentum.rs  # characters, complex projection, sector validation/error
└── tests.rs     # current tests

The group versus reduction boundary is already quite clear in the current file. I think momentum also merits its own module because canonicalize_pauli_sum_complex is a normalized sector projection, not merely the complex-coefficient equivalent of the plain merge; colocating it with character, check_momentum_sector, and SectorCheckError would make that semantic distinction easier to maintain.

I would not split the individual lattice constructors into separate files—the 1D/2D/3D/ladder constructors are cohesive parts of TranslationGroup. Also, about a third of the current file is tests, so moving those out alone would make the production code substantially easier to scan.

Copilot AI review requested due to automatic review settings July 20, 2026 10:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (6)

crates/ppvm-pauli-sum/src/symmetry.rs:101

  • from_generators accepts orders that can be zero. That leads to division/modulo by zero later (e.g. character divides by orders[g], and mixed-radix decoding uses % o). Add an upfront order > 0 validation per generator.
    pub fn from_generators(n_qubits: usize, perms: Vec<Vec<u32>>, orders: Vec<u32>) -> Self {
        assert_eq!(perms.len(), orders.len(), "perms and orders must match");
        for (g, perm) in perms.iter().enumerate() {

crates/ppvm-pauli-sum/src/symmetry.rs:219

  • TranslationGroup::order() multiplies generator orders with product(), which can silently overflow usize in release builds. That can make order() wrong and can even feed 1.0 / order in momentum merging. Use checked multiplication and fail fast on overflow.
    /// Total group order: `Π orders[g]`.
    pub fn order(&self) -> usize {
        self.orders.iter().map(|&o| o as usize).product()
    }

crates/ppvm-pauli-sum/src/symmetry.rs:133

  • The convenience constructors can panic on zero dimensions (e.g. chain_1d(0) does (q + 1) % n). Add explicit assert!(...) guards so invalid sizes fail with a clear message.
    /// 1D chain of `n` sites with periodic boundary conditions.
    /// Single generator: cyclic shift by one site.
    pub fn chain_1d(n: usize) -> Self {
        let perm: Vec<u32> = (0..n).map(|q| ((q + 1) % n) as u32).collect();
        Self::from_generators(n, vec![perm], vec![n as u32])
    }

crates/ppvm-pauli-sum/src/symmetry.rs:152

  • torus_2d(0, ly) / torus_2d(lx, 0) will panic due to modulo-by-zero in the permutation builders. Add explicit dimension assertions so the API fails fast with a clear message.
    /// 2D `lx × ly` torus, qubit at `(i, j)` indexed as `j*lx + i`.
    /// Two generators: x-shift (i → i+1 mod lx) and y-shift (j → j+1 mod ly).
    pub fn torus_2d(lx: usize, ly: usize) -> Self {
        let n = lx * ly;
        let perm_x: Vec<u32> = (0..n)
            .map(|q| {
                let (i, j) = (q % lx, q / lx);
                (j * lx + (i + 1) % lx) as u32
            })
            .collect();
        let perm_y: Vec<u32> = (0..n)
            .map(|q| {
                let (i, j) = (q % lx, q / lx);
                (((j + 1) % ly) * lx + i) as u32
            })
            .collect();
        Self::from_generators(n, vec![perm_x, perm_y], vec![lx as u32, ly as u32])
    }

crates/ppvm-pauli-sum/src/symmetry.rs:187

  • torus_3d has the same modulo-by-zero hazard as torus_2d when any dimension is zero. Add explicit assertions for lx, ly, and lz > 0.
    /// 3D `lx × ly × lz` torus, qubit at `(i, j, k)` indexed as
    /// `k*lx*ly + j*lx + i`.
    pub fn torus_3d(lx: usize, ly: usize, lz: usize) -> Self {
        let n = lx * ly * lz;
        let perm_x: Vec<u32> = (0..n)
            .map(|q| {
                let i = q % lx;
                let j = (q / lx) % ly;
                let k = q / (lx * ly);
                (k * lx * ly + j * lx + (i + 1) % lx) as u32
            })
            .collect();
        let perm_y: Vec<u32> = (0..n)
            .map(|q| {
                let i = q % lx;
                let j = (q / lx) % ly;
                let k = q / (lx * ly);
                (k * lx * ly + ((j + 1) % ly) * lx + i) as u32
            })
            .collect();
        let perm_z: Vec<u32> = (0..n)
            .map(|q| {
                let i = q % lx;
                let j = (q / lx) % ly;
                let k = q / (lx * ly);
                (((k + 1) % lz) * lx * ly + j * lx + i) as u32
            })
            .collect();
        Self::from_generators(
            n,
            vec![perm_x, perm_y, perm_z],
            vec![lx as u32, ly as u32, lz as u32],
        )
    }

crates/ppvm-pauli-sum/src/symmetry.rs:204

  • ladder(l, n_legs) can panic when l == 0 due to (j + 1) % l, and n_legs == 0 produces a degenerate 0-qubit group. Add input validation for clarity and to avoid modulo-by-zero.
    /// Multi-leg ladder: `l` sites along the chain × `n_legs` legs.
    /// Single generator: cyclic shift along the chain direction (all
    /// legs simultaneously). Qubit at `(leg, j)` indexed as
    /// `leg * l + j`. No translation along the leg axis (legs are
    /// distinguished).
    pub fn ladder(l: usize, n_legs: usize) -> Self {
        let n = l * n_legs;
        let perm: Vec<u32> = (0..n)
            .map(|q| {
                let leg = q / l;
                let j = q % l;
                (leg * l + (j + 1) % l) as u32
            })
            .collect();
        Self::from_generators(n, vec![perm], vec![l as u32])
    }

Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
Comment thread crates/ppvm-pauli-sum/src/symmetry.rs Outdated
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

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings September 1, 2026 14:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Review details
  • Files reviewed: 14/14 changed files
  • Comments generated: 1
  • Review effort level: Lite

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:?}"
),
Keep main's #214 clippy fixes (arg structs, type annotations) over the

Co-authored-by: Cursor <cursoragent@cursor.com>
#193 #[allow] workarounds on the five overlapping tableau files.
Copilot AI review requested due to automatic review settings September 1, 2026 14:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

crates/ppvm-pauli-sum/src/symmetry/momentum.rs:325

  • SectorCheckError::CoefficientMismatch's Display message is misleading: it says the orbit representative "expected c=…" but expected is the expected coefficient for the offending orbit member (after applying the sector relation). This makes debugging confusing.
            Self::CoefficientMismatch {
                rep,
                offending_pauli,
                expected,
                actual,
                shift,
            } => write!(
                f,
                "input not in target momentum sector: orbit rep {rep} expected c={expected:?}, \
                 but orbit member {offending_pauli} (shift {shift:?}) has c={actual:?}"
            ),
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +158 to +176
/// Returns `Ok(())` on pass; `Err(SectorCheckError)` on fail with the
/// offending orbit-rep, expected coefficient, and actual coefficient.
///
/// Use this on a user-supplied initial state before feeding it to a
/// phase-aware merging pipeline — silently projecting a wrongly-typed
/// input throws away meaningful physics.
pub fn check_momentum_sector<A, S, const R: bool>(
basis: &[PauliWord<A, S, R>],
coeffs: &[Complex<f64>],
group: &TranslationGroup,
k_modes: &[i32],
tol: f64,
) -> Result<(), SectorCheckError<A, S, R>>
where
A: PauliStorage,
S: BuildHasher + Clone + Default + HashFinalize,
{
assert_eq!(basis.len(), coeffs.len());
assert_eq!(k_modes.len(), group.n_generators());
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants