Skip to content

feat(stim-parser): native R_X/R_Y/R_Z/U3 rotation gates + require *pi in tags#159

Open
david-pl wants to merge 9 commits into
mainfrom
david/parse-bare-rotations
Open

feat(stim-parser): native R_X/R_Y/R_Z/U3 rotation gates + require *pi in tags#159
david-pl wants to merge 9 commits into
mainfrom
david/parse-bare-rotations

Conversation

@david-pl

Copy link
Copy Markdown
Collaborator

Summary

Lets stim-parser / ppvm-stim natively ingest the clifft/tsim rotation
vocabulary, so circuits using R_X/R_Y/R_Z/U3 no longer need tsim's
shorthand_to_stim rewrite before ppvm can run them.

Two commits:

1. feat(stim-parser): parse bare R_X/R_Y/R_Z/U3 rotation gates

  • Register R_X/R_Y/R_Z (Exact(1)) and U3 (Exact(3)) in the
    instruction table; lower them to the existing Rotation/U3 extended
    variants. No executor/validator changes needed downstream.
  • The bare argument is in half-turns (clifft convention:
    R_Z(a) = exp(-i*a*pi/2*Z), gate_data.h/frontend.cc), so lowering
    multiplies by π — making R_Z(a) identical to I[R_Z(theta=a*pi)].
    Verified empirically: clifft and ppvm rotation statistics match to
    sampling noise across several angles.
  • ppvm-stim's exhaustive GateName matches gain the new variants; they
    always lower away before execution, so they join T/T_DAG as
    Ok-in-validate / unreachable!-in-executor.

2. feat(stim-parser)!: require *pi in rotation/U3 tag angles

  • Mirror tsim's parametric-tag convention: tag angles must be written as
    <n>*pi (I[R_Z(theta=0.5*pi)]); a bare number (theta=0.5) is now
    rejected with invalid-tag. tsim's tag parser requires the *pi literal
    and reads the coefficient as half-turns; ppvm refuses the ambiguous bare
    form rather than silently treating it as radians.
  • TagParam::Named gains a had_pi flag (new pi_expr_flagged grammar
    rule); exact_named_params enforces it; the printer re-emits *pi
    (theta={theta/pi}*pi) so print→parse stays a fixpoint.
  • The bare gate form R_Z(0.5) is unchanged — only the bracketed tag is
    tightened, matching tsim (shorthand R_Z(a) vs canonical theta=a*pi).

Background

clifft uses half-turn units; tsim's canonical tag is theta=a*pi (it
stores the coefficient as an exact Fraction). ppvm's tag stored radians
and was the outlier — it coincided with the ecosystem only on the *pi
form. These commits make the bare mnemonic clifft-faithful and tighten the
tag to the *pi convention.

Test plan

  • cargo test -p stim-parser -p ppvm-stim — all 25 test binaries pass.
  • TDD throughout (each test watched failing first); clippy/fmt/license hooks pass.
  • cargo build --workspace clean.

Notes

  • One ppvm-python fixpoint test was updated to the *pi form. It only
    runs against the compiled wheel, which still ships old ppvm 0.1.0 — the
    feature reaches Python only after ppvm-python-native is rebuilt with
    maturin (not done here).
  • SPP[T]/SPP_DAG[T] (Pauli-product rotation, from TPP) remain
    unsupported and out of scope.

🤖 Generated with Claude Code

david-pl added 2 commits June 24, 2026 11:46
clifft/tsim circuits use bare rotation mnemonics (`R_Z(0.02) 0`,
`U3(a,b,c) 0`) that the parser previously rejected as unknown
instructions; only the tagged `I[R_Z(theta=..*pi)]` / `I[U3(...)]`
forms were accepted (via tsim's shorthand_to_stim rewrite).

Register R_X/R_Y/R_Z (Exact(1)) and U3 (Exact(3)) in the instruction
table and lower them to the existing Rotation/U3 extended variants.
The bare argument is in half-turns (clifft convention: clifft's
`R_Z(a) = exp(-i*a*pi/2*Z)`, gate_data.h half-turn units), so the
lowering multiplies by pi to obtain the radians theta the Rotation
variant carries — making `R_Z(a)` identical to `I[R_Z(theta=a*pi)]`.
Verified empirically: clifft and ppvm rotation statistics match to
sampling noise across several angles.

Tags are intentionally untouched here (a follow-up tightens them).

ppvm-stim's exhaustive GateName matches gain the new variants: they
are always lowered away before execution, so they join T/T_DAG as
Ok-in-validate / unreachable-in-executor.
Mirror tsim's parametric-tag convention: rotation and U3 tag angles
must be written in half-turns as `<n>*pi` (e.g. `I[R_Z(theta=0.5*pi)]`),
and a bare number (`I[R_Z(theta=0.5)]`) is now rejected with an
`invalid-tag` diagnostic. tsim's tag parser requires the `*pi` literal
(`core/parse.py`, regex `^\w+=<float>\*pi$`) and treats the coefficient
as half-turns; ppvm now refuses the ambiguous bare form rather than
silently reading it as radians.

Mechanics:
- `TagParam::Named` gains a `had_pi` flag, captured by a new
  `pi_expr_flagged` grammar rule (pi_expr is now defined in terms of it).
- `exact_named_params` rejects any rotation/U3 angle written without `*pi`.
- The printer re-emits the half-turn form (`theta={theta/pi}*pi`); since
  every parser-produced rotation angle is a `<n>*pi` multiple, theta/pi
  recovers the coefficient exactly (verified for the relevant float set),
  so print -> parse stays a fixpoint.

The bare *gate* form `R_Z(0.5)` (clifft half-turn shorthand) is
unchanged — only the bracketed tag form is tightened, matching tsim
(shorthand `R_Z(a)` vs canonical tag `theta=a*pi`).

Tests across extended/roundtrip/proptest suites move to the `*pi` form;
the proptest AST generator now produces `coeff*pi` angles (the only
parser-producible shape). The ppvm-python fixpoint test is updated too.
Copilot AI review requested due to automatic review settings June 24, 2026 12:42
@david-pl

Copy link
Copy Markdown
Collaborator Author

FYI @rafaelha @Roger-luo: adds some additional non-Clifford instructions that tsim and clifft support. With this, we support everything but TPP and SPP, I think.

@github-actions

github-actions Bot commented Jun 24, 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-159/

Built to branch gh-pages at 2026-06-24 20:10 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the stim-parserppvm-stim pipeline to natively accept clifft/tsim-style parameterized rotation mnemonics (R_X, R_Y, R_Z, U3) and tightens the extended-tag syntax so rotation/U3 tag angles must explicitly carry a pi unit (preventing ambiguity between half-turns and radians).

Changes:

  • Adds R_X/R_Y/R_Z/U3 to the instruction table and lowers their arguments (half-turns) into existing extended Rotation/U3 instructions (radians).
  • Updates the tag grammar/AST to track whether pi appeared, and rejects rotation/U3 tags written without *pi-style units.
  • Updates printing and test corpora/property tests to use the canonical ...*pi form.

Reviewed changes

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

Show a summary per file
File Description
ppvm-python/test/test_stim_api.py Updates Python-side Stim API test input to the new canonical ...*pi tag form.
crates/stim-parser/tests/tags.rs Adjusts tag parsing tests for the new had_pi field.
crates/stim-parser/tests/roundtrip.rs Updates roundtrip corpus and expected printed forms to include *pi.
crates/stim-parser/tests/proptest_roundtrip.rs Updates generated extended-tag fragments to use *pi.
crates/stim-parser/tests/proptest_ast.rs Adjusts AST generators so Rotation/U3 angles originate from coefficient * PI.
crates/stim-parser/tests/extended.rs Updates extended parsing/lowering tests to assert radians (coefficient * PI).
crates/stim-parser/src/syntax/grammar.rs Introduces pi_expr_flagged and records had_pi on named tag params.
crates/stim-parser/src/print/mod.rs Re-emits rotation/U3 tags in ...*pi form and prints named params respecting had_pi.
crates/stim-parser/src/pipeline/validate.rs Updates validation test scaffolding for the had_pi field.
crates/stim-parser/src/pipeline/lower.rs Lowers bare R_*/U3 mnemonics in half-turns; enforces had_pi for rotation/U3 tags.
crates/stim-parser/src/instructions/mod.rs Adds new GateName variants and table entries for R_* and U3.
crates/stim-parser/src/ast/shared.rs Extends TagParam::Named to include had_pi.
crates/ppvm-stim/tests/executor.rs Updates executor test circuit to use *pi for all U3 parameters.
crates/ppvm-stim/src/validate.rs Treats new gate names as valid-at-validate (they should be lowered before execution).
crates/ppvm-stim/src/executor.rs Marks new gate names as unreachable!() in the executor since they are expected to be lowered away.

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

Comment thread crates/stim-parser/src/pipeline/lower.rs
Comment thread crates/stim-parser/src/pipeline/lower.rs
The bare-rotation lowering arms bound `tags` but never used them, so a
tag on a bare rotation mnemonic (e.g. `R_Z[foo](0.5)`) parsed and was
silently dropped — lossy on parse->lower->print and able to mask a typo'd
tag. A tag has no meaning on the bare mnemonics, so reject a non-empty tag
list with an `invalid-tag` diagnostic pointing at the argument form.

(The pre-existing T/T_DAG arms drop tags the same way; left untouched here
as out of scope for this change.)
Copilot AI review requested due to automatic review settings June 24, 2026 12:59
Follow-up to the bare R_X/R_Y/R_Z/U3 tag fix: the native T / T_DAG arms
dropped any tag the same way (e.g. `T[foo] 0` parsed and silently lost
`[foo]`). Reject a non-empty tag list with an `invalid-tag` diagnostic
that points at the canonical tagged spelling, S[T] / S_DAG[T].

Copilot AI left a comment

Copy link
Copy Markdown

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 15 out of 15 changed files in this pull request and generated 2 comments.

Comment thread crates/stim-parser/tests/proptest_ast.rs Outdated
Comment on lines +72 to +77
TagParam::Named { key, value, had_pi } => {
if *had_pi {
write!(out, "{key}={}*pi", FloatLit(*value / std::f64::consts::PI))?;
} else {
write!(out, "{key}={}", FloatLit(*value))?;
}
@Roger-luo Roger-luo enabled auto-merge (squash) June 24, 2026 17:03
Copilot AI review requested due to automatic review settings June 24, 2026 17:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Roger-luo Roger-luo disabled auto-merge June 24, 2026 17:03
…ouble-scale)

Bare rotation/U3 args are in half-turns and get multiplied by pi when
lowering (R_Z(0.5) == I[R_Z(theta=0.5*pi)]). But args flow through the
generic pi_expr parser, so R_Z(0.5*pi) / R_Z(pi) were silently scaled by
pi twice (0.5*pi*pi instead of 0.5*pi) with no diagnostic — the inverse
of the ambiguity the *pi tag requirement was added to prevent.

args_block now carries a had_pi flag (via the existing pi_expr_flagged);
the validator rejects a *pi argument on the bare R_X/R_Y/R_Z/U3 mnemonics
with a clear half-turn-arg error. pi-expressions remain valid in args for
every other instruction (stim.rs compatibility, e.g. X_ERROR(0.5*pi)).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Roger-luo

Copy link
Copy Markdown
Collaborator

Review: edge cases + a pushed fix

Reviewed the PR for unhandled edge cases. Found one confirmed correctness bug and pushed a fix to this branch (a97599c3); two other items are flagged below for your call.

🔴 Fixed in a97599c3 — bare rotation/U3 args silently double-applied π

Bare rotation/U3 args are in half-turns and get × π at lowering (R_Z(0.5) == I[R_Z(theta=0.5*pi)]). But args flow through the generic pi_expr parser, so the *pi/pi form was accepted and then multiplied by π again:

R_Z(0.5*pi) 0   ->  theta = 4.9348…  (= 0.5·π², not 0.5·π)
R_Z(pi) 0       ->  theta = π²

No diagnostic — the inverse of the ambiguity commit #2's *pi tag requirement was added to prevent. A user copying the *pi coefficient from the canonical tag form into the bare gate gets a silently wrong angle.

Fix (targeted, not global): args_block now records a had_pi flag (reusing the existing pi_expr_flagged), and the validator rejects a *pi arg only on the bare R_X/R_Y/R_Z/U3 mnemonics with a clear half-turn-arg error. Crucially, pi-expressions stay valid in args for every other instruction — noise.rs documents that as an intentional stim.rs-compat feature (X_ERROR(0.5*pi)), so a global removal would have been wrong.

R_Z(0.5) 0         -> OK
R_Z(0.5*pi) 0      -> ERROR 'R_Z' takes angle arguments in half-turns; remove the '*pi' suffix …
U3(0.5*pi,0,0) 0   -> ERROR (same)
X_ERROR(0.5*pi) 0  -> OK  (compat preserved)

Added 5 regression tests (grammar / validate-unit / lower-e2e). cargo test -p stim-parser -p ppvm-stim green, cargo build --workspace clean, all pre-commit hooks pass.

🟡 For your call — U3 phi/lambda half-turn convention

Commit #1 multiplies all three U3 angles by π, but the empirical verification cited in the PR covers the single-axis rotations; the only U3 execution test uses theta=1.0*pi with phi=lambda=0, so the half-turn scaling of phi/lambda is never exercised with nonzero values. Worth either confirming clifft's U3 uses half-turns for all three angles, or adding a U3 test with nonzero phi/lambda. (Didn't touch this — I can't verify against clifft, and a test here would just encode a guess.)

🟢 Confirm intent — bare T/T_DAG now reject tags

Commit #1 adds tag rejection to the bare T/T_DAG arm (previously tags were silently dropped). Correct and consistent with the new rotation behavior, just slightly broader than the "rotation vocabulary" scope in the summary — flagging so it's a deliberate choice.

✅ Checked and fine (no action)

  • Printer fixpoint: the new divide-on-print / multiply-on-parse *pi cycle is stable — simulated the full pipeline over 300k random angles, settles on the first print; the AST proptest's exact-eq property holds for all float_lit() values.
  • Arity soundness: the unreachable!("arg count is fixed…") in lowering is sound — Exact(1)/Exact(3) is enforced in validate before lowering.
  • Record targets / positional tag params / empty tag blocks on the new gates are all handled.

🤖 Generated with Claude Code

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 24, 2026 19:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ail)

The rotation/U3 printer emitted `theta/PI` directly, so the division's
rounding leaked into the output: `theta=0.76*pi` printed as
`theta=0.7599999999999999*pi`. Roughly 10% of ordinary decimal angles
were affected.

`pi_coeff` now returns the shortest decimal `c` whose `c * PI` recovers
the stored radians bit-for-bit, falling back to `value / PI` when no exact
short form exists. Because acceptance requires exact equality, this both
prints cleanly and keeps `parse → print` lossless and the printer a
byte-for-byte fixpoint — verified by a new property test over arbitrary
decimal coefficients (exact theta recovery + fixpoint + no rounding tail)
plus a unit test covering tags, bare gates, and U3. Addresses Copilot's
print/mod.rs review note.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Roger-luo

Copy link
Copy Markdown
Collaborator

Addressed Copilot's printer note (print/mod.rs) — b7e282fe

The rotation/U3 printer emitted theta/PI directly, so the division's rounding tail leaked into the output — e.g. theta=0.76*pi printed as theta=0.7599999999999999*pi. ~10% of ordinary decimal angles were affected.

Fix: a new pi_coeff(value) returns the shortest decimal c whose c * PI recovers the stored radians bit-for-bit, falling back to value / PI when no exact short form exists. Because acceptance requires exact equality, the output is both clean and round-trip-lossless:

I[R_Z(theta=0.76*pi)] 0   ->  I[R_Z(theta=0.76*pi)] 0     (was 0.7599999999999999*pi)
R_Z(0.19) 0               ->  I[R_Z(theta=0.19*pi)] 0
I[U3(theta=0.34*pi, …)]   ->  unchanged, no tail

Round-trip safety (the key constraint): since c is only accepted when c*PI == theta exactly, parse → print stays lossless and print → parse → print remains a byte-for-byte fixpoint. Covered by:

  • a new property test (rotation_pi_coeff_round_trips) over arbitrary decimal coefficients asserting exact theta recovery + fixpoint + no rounding tail (stress-run at 20k cases),
  • a unit test covering tags, bare gates, and U3,
  • and the existing fixpoint / AST-roundtrip proptests, all still green.

cargo test -p stim-parser -p ppvm-stim, cargo clippy -- -D warnings, and fmt all pass.

🤖 Generated with Claude Code

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