Add the Phase(gamma) unitary with exact naming, structural control, and hardware lowering

[ciaranra]

Stacked on #668 (the branch includes its commits; merge #668 first).

What

One new unitary, Phase(gamma), with one rule: on an operand set S of zero, one, or two qubits it multiplies the amplitude by exp(i gamma) on the subspace where every qubit in S is |1> and leaves everything else alone.

• S = {} is a global phase (the scalar exp(i gamma)).
• S = {q} is diag(1, exp(i gamma)), exactly U(0, 0, gamma).
• S = {c, t} is diag(1, 1, 1, exp(i gamma)).
• |S| > 2 is refused at construction.
• Controlling a Phase on S by a qubit c is structurally Phase on S + {c}.

This is layers 1 and 2 of the Phase(gamma) design (representation and hardware lowering). No GateType, byte-level command, simulator fast path, or ingress rewiring yet; those follow once the benchmark decides the command shape.

API (pecos-core)

• Unitary::Phase { gamma: Angle64, num_qubits: u8 } and UnitaryRep::phase_gate(gamma, qubits).
• UnitaryRep::control(self, c) -> Result<UnitaryRep, ControlError>: structural for Phase; refuses a control already in S, a third operand, a non-Phase rep, and a descriptor whose operand count disagrees with its qubit list.
• controlled_rotations::lower_phase(gamma_radians, &[QubitId]) -> Vec<Gate>: {} lowers to no gates (hardware cannot see a scalar; the caller keeps it), {q} to U(0, 0, gamma), {c, t} through lower_cphase.
• Dense conversion builds the diagonal from the rule directly (no half-angle, so negative angles need no representative choice).
• is_clifford reads the shared exact Clifford-angle table (try_simplify_rotation): one qubit at multiples of pi/2, two qubits at 0 and pi. to_named_gate returns a name only when the matrices are exactly equal: I, SZ, Z, SZdg, T, Tdg on one qubit, CZ on two. Adjoint, is_hermitian, to_pauli_string, is_pauli_equivalent/try_to_pauli, Unitary::is_pauli/try_to_pauli, to_clifford_rep, the hardware decomposition, the phase() accessor (a zero-operand Phase is the scalar itself), and circuit-diagram rendering handle Phase.

Fix found on the way

lower_cphase halved the source f64 before Angle64 reduction, so at lambda = 2pi (mod 4pi) the RZZ(-pi) and RZ(+pi) legs both stored HALF_TURN and the lowering executed as -I instead of I (QASM/PHIR cp(2pi) on dev). CPhase is 2π-periodic, so the angle is now reduced to (-pi, pi] first; every halved angle then lies in (-pi/2, pi/2] and the lowering is exact for every input. CRZ/CRX/CRY are 4π-periodic and cannot use that reduction; their documented ±1 at theta = 2pi (mod 4pi) is intrinsic to the two-gate form and is filed as #670 (the exact fix is a zero-operand Phase from the ingress, which this PR makes possible).

Tests

crates/pecos-simulators/tests/phase_gate_contract.rs is the specification: the matrix follows the rule at nine angles including ±2pi, pi, 3pi/2, for S = {}, {0}, {1}, {0,1}, {1,0}; single-qubit Phase equals U(0,0,gamma) entrywise; the lowering composed in the dense path and executed on StateVecSoA equals the rule entrywise (1e-12); control() composes and refuses as specified; Clifford membership and exact naming follow the angle table. Unit tests cover the control() error variants, lower_phase on {}, the T/Tdg names, the Pauli views, and the zero-operand phase().

Follow-ups filed

• to_named_gate names rotations up to global phase but Phase exactly; split or document the semantics #669: to_named_gate names rotations up to global phase (RZ(pi) -> Z) while Phase names exactly; split or document.
• CRZ/CRX/CRY lowering is -1 at theta = 2pi mod 4pi; emit a zero-operand Phase once the ingress has it #670: CRZ/CRX/CRY ±1 at theta = 2pi (mod 4pi).

Verification

cargo fmt --all -- --check; cargo clippy --workspace --all-targets -- -D warnings (default and --all-features lanes); pecos rust test --profile debug (full recipe); just lint check. Mutation: removing the lower_cphase reduction fails both lowering tests in the contract. Two independent reviews (fresh-context and cross-model) ran on the diff; every finding is fixed or refuted in this description.

[ciaranra]

Reviewed the parts that survive a rebase. I have not reviewed the diff itself, for the reason in the last section.

The lower_cphase bug is real, confirmed independently on current dev

Angle64::from_radians normalises through rem_euclid(TAU) into [0, 2pi) (crates/pecos-core/src/angle.rs:240-248), so from_radians(-pi) and from_radians(pi) are the same stored value. lower_cphase halves the f64 first (controlled_rotations.rs:62-63), so at lambda = 2pi:

• half_lambda = pi
• the RZZ leg asks for from_radians(-pi) and gets HALF_TURN
• the RZ leg asks for from_radians(pi) and gets HALF_TURN

The intended RZZ(-pi) therefore executes as RZZ(+pi). RZZ is 4pi-periodic, so those are not the same gate, and the composed lowering is off by -1. That matches the description exactly. Reducing lambda into (-pi, pi] before halving is the right fix, and the justification given for why CRZ/CRX/CRY cannot use the same reduction is correct: they are 4pi-periodic, so the reduction is not phase-preserving for them.

Why it survived, which matters for the test

controlled_phase_lowering_preserves_phase in controlled_rotations.rs already covers lambda = TAU. It cannot fail on this defect, because it asserts through assert_matrix_eq_up_to_one_global_phase, and the defect is a global phase.

So the existing coverage is not merely thin here; it is structurally incapable of detecting the bug it appears to cover. Worth stating explicitly in the PR, and worth making sure the new phase_gate_contract.rs assertions for this case compare entrywise rather than through that helper. The description says the contract test compares entrywise at +-2pi, which is right; the thing to avoid is anyone later "simplifying" it back onto the global-phase-tolerant helper.

This is also an argument for the change beyond the new gate: it fixes a live wrong-answer path reachable from QASM/PHIR cp(2pi) on dev today, independently of whether Phase(gamma) lands.

The design reads well

The single rule, phase exp(i gamma) on the all-ones subspace of S, with |S| = 0, 1, 2 and control as S + {c}, is a good shape: it makes controlling structural rather than a decomposition, and it gives the zero-operand case a home, which is what makes the #670 fix expressible at all. Refusing |S| > 2 at construction and refusing a control already in S are the right boundaries.

The one thing I would want stated plainly in the PR rather than inferred is the naming asymmetry you already filed as #669: to_named_gate names rotations up to global phase but names Phase exactly. Two callers with reasonable expectations will disagree about RZ(pi) versus Phase(pi) on one qubit. Since this PR is what introduces the exact-naming side, a sentence in the PR body pinning which behaviour is intended to win would help whoever resolves #669.

The diff needs a refresh before it is worth reviewing

The branch is 40 commits behind dev and currently conflicting, and 36 of the files it touches have also changed on dev since it forked. The interactions are not cosmetic:

• Enforce Gate angle arity at construction and DAG mutation #701 changed Unitary::Named(GateType) into Unitary::Named(NamedGate) with a private field and a checked constructor. This PR adds a new Unitary::Phase variant to that same enum, so the two designs meet directly.
• Build each term's GF(2) constraint rows once per measurement instead of once per pair #737 and Defer the upward reduction until a pair's constraint system is known consistent #738 substantially rewrote stab_vec.rs and stab_vec/ch_form.rs, which this PR also touches.
• angle.rs and unitary_rep.rs both moved.

Reviewing the current diff would produce findings the rebase invalidates, and would miss the interactions that only appear afterwards. I would rather review it once against current dev than twice against neither.

[ciaranra]

I attempted the dev refresh and stopped, because it needs a decision that is yours rather than a merge resolution. Separately, thinking the design through against what landed this session surfaced one thing worth deciding before the rebase, not after. The branch is untouched; the merge was aborted.

The refresh is blocked on a phase-convention collision, not on mechanics

44 commits behind, 12 conflicted files. Most are mechanical: this branch adds a Unitary::Phase variant while #701 sealed the sibling Unitary::Named(GateType) into Named(NamedGate) with a private field, so every Named pattern moved. The adaptation rule is clear from what dev already does: destructure Named(NamedGate(gt)) inside pecos-core, bind Named(named) and call named.gate_type() outside it.

Two conflicts are not mechanical:

• crates/pecos-simulators/src/clifford_gateable.rs:966 -- this branch preserves SXX = exp(-i pi XX / 4) through a global-phase correction. dev now uses exp(i pi/4) exp(-i pi XX/4) and carries tests requiring that canonical matrix. The two cannot both hold.
• python/selene-plugins/pecos-selene-general-noise/src/lib.rs:1945 -- the SZZdg residual phase, 0.0 against -pi/4, is the same disagreement.

dev's side of this is #668, "Halve the signed angle on the dense path and make the Clifford defaults phase-exact", which merged after this branch was written. So the branch predates the convention it now conflicts with. Resolving in dev's favour is very likely correct, but it means some of this PR's own assertions change, and that is an author decision rather than a merge decision. I did not make it.

A correction to something I said earlier

In my previous comment I described the broadened predicate as UnitaryRep::is_pauli_equivalent. That is wrong: the try_to_pauli().is_some() form is Unitary::is_pauli. UnitaryRep::is_pauli_equivalent on this branch matches explicitly and recognises one-qubit Phase(pi) while excluding Phase(0). Since Phase(0) on one qubit is exactly I, that exclusion looks like an oversight rather than intent, and it is worth settling deliberately while the surrounding code is being touched.

The design point: Phase's arity is documented, not enforced

Unitary::Phase { gamma: Angle64, num_qubits: u8 } has public fields, and the constraint lives in a doc comment: "The number of operands, which must be at most two." So Unitary::Phase { gamma, num_qubits: 200 } is constructible from any crate, bypassing phase_gate entirely.

This matters here specifically, rather than as a general objection. Most Unitary variants are open in exactly this way; Named is the sole sealed one, and #701 sealed it precisely because its invariant had a panicking consumer. Phase has that same property:

• UnitaryRep::phase_gate (unitary_rep.rs:1108) enforces the bound with assert!, so it panics on caller data rather than returning an error.
• controlled_rotations::lower_phase (controlled_rotations.rs:100) is pub, takes a plain &[QubitId], and panics outright on more than two qubits, with no enum involved.

Credit where due: the reading consumers all degrade safely. is_clifford, to_named_gate and try_to_pauli fall through to false/None for an out-of-range count rather than misbehaving, which is better than Named was before it was sealed. The exposure is the panicking paths, not silent wrong answers.

So the question worth answering before the rebase, since the answer changes what the rebase produces: should Phase follow Named and become a checked, sealed descriptor, with lower_phase returning Result instead of panicking? Doing it now costs a little; doing it after this lands means touching the same code twice, and the second PR will be the one that has to justify why a public API started returning Result.

I have no objection to the mathematics, which I reviewed earlier and still read as correct.

[ciaranra]

Refreshed against dev and applied the three decisions from my previous comment. Pushed to phase-gate-refresh rather than force-moving this branch, so the original is intact and the two can be compared; retarget or fast-forward phase-gate onto it when you are satisfied.

Decision 1: dev's phase conventions win

clifford_gateable.rs and the Selene general-noise plugin now match dev exactly, zero diff. This branch's SXX = exp(-i pi XX/4) correction and its 0.0 SZZdg residual were written before #668 made the Clifford defaults phase-exact; #668 is the repo's convention now, so the branch adopts it wholesale rather than meeting it halfway.

Phase's own matrix rule is untouched. That was the thing to protect: the rule is that Phase(gamma) multiplies the all-ones subspace of its operand set by exp(i gamma), and it is independent of how SXX is canonicalised. The only changes to phase_gate_contract.rs are three .unwrap() calls where lower_phase now returns Result. No assertion was relaxed and no case removed.

Decision 2: one-qubit Phase(0) is Pauli I

It was recognised as Z at Phase(pi) but excluded at Phase(0), which is diag(1, 1). Both directions are now consistent between is_pauli_equivalent and try_to_pauli, with a test pinning each.

The refresh also turned up something I had not anticipated: I(q) is represented as RZ(0, q), so identity recognition needed an extra case to stay consistent. That is in the report rather than silently folded in.

Decision 3: Phase is sealed and lowering is fallible

Unitary::Phase(PhaseGate) where PhaseGate is a newtype with a private field, gamma() and num_qubits() accessors, and checked construction. That is the shape #701 gave Unitary::Named(NamedGate), deliberately, so the enum has one sealing pattern rather than two.

lower_phase returns Result instead of panicking on more than two operands, and phase_gate has a checked form. The reading paths keep degrading safely: is_clifford, to_named_gate and try_to_pauli still return false/None for an out-of-range operand count rather than panicking.

The reason for doing this now rather than as a follow-up: both Unitary::Phase and lower_phase are new in this PR and exist nowhere on dev. Sealing a variant nobody matches on yet, and shipping a Result before anyone depends on it, costs nothing today. Either change a week later is a breaking change to API introduced a week earlier, which is exactly the position Named was in when #701 had to seal it retroactively.

Verification

Run independently rather than taken from the implementation report.

• Release 3,712 passed, debug 3,716 passed, zero failures.
• Workspace clippy with all targets and features, -D warnings: clean. cargo fmt --all -- --check: clean.
• The seal is load-bearing, not decorative: making PhaseGate's field public causes the compile_fail doctest to fail, because the forbidden construction then compiles. Restoring the seal passes.
• crates/pecos-simulators/tests/data/stab_vec_correctness_bits.txt appears as added because this branch predates Pin StabVec numerical behaviour against an oracle and exact reference bits #716; it is byte-identical to dev's copy, neither regenerated nor edited.

Left alone deliberately

Unitary::Phase (new) and UnitaryRep::Phase { phase, inner } (pre-existing) now share a name in the same module with different meanings. The compiler disambiguates and the hazard is human, so renaming either belongs in its own change rather than widening this one.

[ciaranra]

Applied the narrowing on phase-gate-refresh (e4a487357). This reverses part of what I did in the previous commit, deliberately.

What changed and why

A phase is a mathematical scalar. The two-operand limit is a property of the hardware we lower to, not of the operator: Phase(gamma) on three operands is diag(1, ..., 1, exp(i gamma)), perfectly well-defined and simply not directly lowerable. Encoding that limit in the representation conflated the two.

So:

• Unitary::Phase { gamma, num_qubits } is plain data again. PhaseGate, its checked constructor, and its compile_fail doctests are gone -- they pinned a rule that no longer exists.
• UnitaryRep::control now produces a third operand rather than refusing it. Controlling Phase on S by c is Phase on S + {c} for any |S|, which is the structural composition that made this design attractive in the first place; refusing at three was an artificial stop.
• lower_phase is the sole boundary. It reports "Phase direct hardware lowering supports at most two operands, got 3", which says the operator is fine and the lowering is not, rather than implying the caller built something invalid.

I sealed this variant in the previous commit on the argument that #701 had to seal Named for the same reason. That argument was wrong: Named's seal protects a real invariant, whereas once the arity bound is understood as a hardware constraint there is nothing left for a Phase seal to protect. The one genuine invariant, that a Gate's qubit list agrees with its descriptor's operand count, is a property of the pair and cannot be enforced by sealing the descriptor.

What deliberately did not change

NamedGate is untouched, and three of the four remaining compile_fail doctests in the file are still its. Its seal guards an invariant that does exist.

Unitary::num_qubits stays infallible and ToMatrix for Unitary still works, which is why the operand count remains in the descriptor even though the bound left it: every Unitary variant is self-describing in arity, and breaking that would ripple through every variant.

control still rejects a control already present in S (there S + {c} == S, so the control silently does nothing), a non-Phase rep, and a descriptor whose operand count disagrees with its qubit list. Those are real caller errors.

Verification, run independently

• Release 3,717 passed, debug 3,721 passed, zero failures. Workspace clippy with all targets and features and -D warnings: clean. Formatting: clean.
• crates/pecos-simulators/tests/data/: unchanged.
• The boundary is load-bearing, not asserted: making lower_phase accept three operands and emit a U fails lower_phase_rejects_unsupported_arity_without_panicking and phase_construction_accepts_arbitrary_arity_and_rejects_duplicates. Restoring passes.

Tests that pinned the removed rule were replaced rather than deleted: the arity-refusal cases now assert that larger arities are accepted and produce the expected larger diagonals, and control_is_structural asserts the two-to-three composition instead of its refusal. The contract file is now 12 tests.

Still open

Unitary::Phase and the pre-existing UnitaryRep::Phase { phase, inner } share a name in one module with different meanings. The compiler disambiguates; the hazard is human. Worth its own change rather than widening this one.