Skip to content

feat: Add RXYXY2Q gate - #788

Open
jake-arkinstall wants to merge 3 commits into
PECOS-packages:devfrom
jake-arkinstall:feat/rpp-gate
Open

jake-arkinstall wants to merge 3 commits into
PECOS-packages:devfrom
jake-arkinstall:feat/rpp-gate

Conversation

@jake-arkinstall

@jake-arkinstall jake-arkinstall commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Adds RXYXY2Q as a GateType, performing the 2-qubit operation:

$$ \text{RXYXY2Q}(\theta, \phi) = e^{-i \frac\theta2 \left(P\left(\phi\right) \otimes P\left(\phi\right)\right)}, $$ where $$ P\left(\phi\right) = \cos\left(\phi\right)X + \sin\left(\phi\right)Y, $$ similar to how $$ \text{RXY1Q}(\theta, \phi) = e^{-i \frac\theta2 P\left(\phi\right)}$$

Copilot AI lite review requested due to automatic review settings September 17, 2026 13:31

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.

🟡 Changes recommended

Batched RPP commands receive noise only on their first pair, and the new decomposition paths lack targeted tests.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds RPP as a parameterized two-qubit gate with arbitrary-angle and Clifford simulator support.

Changes:

  • Adds RPP metadata, constructors, serialization, display, and dispatch.
  • Implements RPP through an RZ–RXX–RZ decomposition.
  • Integrates RPP with legacy depolarizing noise models and explicitly rejects it in QASM.
File summaries
File Description
clifford_rotation.rs Adds Clifford RPP execution.
circuit_executor.rs Dispatches RPP gates.
arbitrary_rotation_gateable.rs Adds arbitrary-angle decomposition.
unitary_matrix.rs Classifies RPP as parameterized.
circuit_display.rs Adds RPP rendering.
engine.rs Rejects unsupported QASM RPP gates.
quantum.rs Adds engine dispatch paths.
depolarizing.rs Applies two-qubit noise classification.
biased_depolarizing.rs Applies biased two-qubit noise classification.
builder.rs Adds byte-message RPP construction.
gates.rs Adds the RPP gate constructor.
gate_type.rs Defines RPP and its arities.
Review details
  • Files reviewed: 12/12 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

| GateType::SZZ
| GateType::SZZdg
| GateType::SWAP
| GateType::RPP

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This isn't related to this PR - if the review is correct, it'd be the same in all other 2Q gates too and would need addressing separately.

| GateType::SZZ
| GateType::SZZdg
| GateType::SWAP
| GateType::RPP

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

/// rotations the same sign. Each ion contributes a minus sign, so the
/// two-qubit global phase still cancels.
#[inline]
fn rpp(&mut self, theta: Angle64, phi: Angle64, pairs: &[(QubitId, QubitId)]) -> &mut Self {
///
/// # Errors
/// Returns an error before changing the state if an angle is unsupported.
fn try_rpp(
@ciaranra

Copy link
Copy Markdown
Member

RPP is not a great name, .e.g.:

  • p(θ) = phase(θ) = u1(θ) = diag(1, exp(iθ)), the phase gate.
  • the P Prepare prefix (PX, PZ).
  • generic-Pauli P in RP(θ) = exp(-i θ P/2).

I think a better naming derivation is:

XY_φ = cos(φ)X + sin(φ)Y

RXY1Q(θ, φ) = exp(-i θ XY_φ / 2)
RXYXY2Q(θ, φ) = exp(-i θ (XY_φ ⊗ XY_φ) / 2)

Therefore, to add to confusion, in PECOS I would go with the RXYXY2Q. Ugly... but less confusing.

@jake-arkinstall jake-arkinstall changed the title feat: Add RPP gate feat: Add RXYXY2Q gate Sep 17, 2026
@jake-arkinstall

Copy link
Copy Markdown
Contributor Author

I have updated the gate name, and I have also added some hooks into Selene where they were previously bailing due to rpp (a.k.a. rxyxy2q) being unsupported.

@ciaranra ciaranra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How this review was produced

This is an AI-generated review, posted at my request, of commit 8c87d492a. It has not been hand-checked line by line by me, so please push back on anything that looks wrong.

  • Primary reviewer: Claude Fable 5.1 (claude-fable-5-1) running in Claude Code. It read the full diff, swept the repository for gate lists the diff does not touch, compared the gate against Selene's own rpp definition, and wrote and ran two throwaway test files in a local worktree of this branch (since deleted, nothing was pushed).
  • Independent second reviewer: OpenAI gpt-6-astra via Codex CLI 0.154.0, high reasoning effort, read-only sandbox. It was given the diff and a list of questions, with no access to the first reviewer's conclusions.
  • Fusion: every Codex finding listed below was re-checked against the code by Claude before being included. Where a claim rests on something that was executed rather than read, it says so.

Summary

The gate itself is correct. The requests are at the edges: one inconsistency this PR introduces in the QASM layer, and two Python paths that silently drop the gate or its noise.

Verified correct

  • Definition and Selene convention. Rz(-phi); RXX(theta); Rz(phi) gives exp(-i theta/2 P(phi) (x) P(phi)). The resulting matrix was compared entry by entry, real and imaginary parts, against the matrix in Selene's QuEST simulator (selene-ext/simulators/quest, rpp). They agree.
  • Phase exactness beyond the f32 backend. The PR's matrix test uses StateVecSoA32 at 3e-6. A scratch probe ran the same four-column matrix check on the f64 dense state vector, the sparse state vector and StabVec, over a 7x7 grid of angles including phi = pi and negative angles. All agree to 1e-10, so the claim that the two sign flips at phi = pi cancel holds on those backends. (Executed.)
  • Clifford path atomicity. The precheck in try_rxyxy2q uses the same snapping as try_rz and try_rxx, and the Clifford angle grid is closed under negation, so a passing precheck cannot be followed by a failure after the first rz has mutated state.
  • General noise model scales its two-qubit rate by angles[0], which is theta. That is the right angle.
  • DEM builder and pecos-neo adapter reject the new gate with an error rather than skipping it.
  • Test oracles. The matrix tests derive the expected matrix independently of the implementation; a flipped sign on phi or swapped theta/phi would fail them.

Requested changes

  1. QASM accepts the gate at parse time and rejects it at run time. parse_native_gate (crates/pecos-qasm/src/parser/native_gates.rs:36) now returns RXYXY2Q, while crates/pecos-qasm/src/engine.rs:692 returns "not yet supported in the QASM engine". The is_qasm_native_gate test helper in the same file classifies the gate as non-native, so the round-trip test skips it and does not notice. Reproduced: RXYXY2Q(pi, 0) q[0], q[1]; parses Ok and qasm_sim(...).run(1) returns that error. RXX and RYY are not QASM-native either, so the smallest consistent fix is to remove the parser line.

  2. TickCircuit.with_noise applies no two-qubit noise to the gate. python/pecos-rslib/src/dag_circuit_bindings.rs:2952 lists the two-qubit gates explicitly and ends in _ => {}. The gate is already reachable from Python through add_gate("RXYXY2Q", ...) because GateType::from_str accepts the name. Adding the variant to that arm fixes it.

  3. Stim export drops the gate without an error. _gate_to_stim in python/quantum-pecos/src/pecos/qec/surface/circuit_builder.py:3304 falls through to return [], None. Codex executed the exporter on RXYXY2Q(pi, 0) with p2 = 1 and got an empty circuit. Please either lower the Clifford angles the way the RXY1Q branch above it does, or raise for this gate.

  4. _qis_trace_replay.py cannot replay the new operation (both the raw path near line 193 and the runtime-lowered path near line 440). This fails loudly, so it could be a follow-up, but note that traces containing RPP now reach this code where previously the runtime refused them.

  5. Clifford edge cases (minor).

    • try_rxyxy2q compares theta == Angle64::ZERO exactly, whereas try_rxx snaps. A theta within snapping tolerance of zero combined with a non-Clifford phi is therefore rejected although the gate is the identity. Snapping theta first, as try_simplify_rxy1q does, removes the inconsistency. The Selene stabilizer plugin already does this on its side.
    • theta = pi with phi an odd multiple of pi/4 is Clifford ((X+Y)/sqrt(2) swaps X and Y and negates Z), and Selene's Stim simulator accepts it. This PR rejects it. Existing RXY1Q has the same limitation, so I do not consider this blocking.
  6. docs/user-guide/gates.md has no entry for the new gate.

Pre-existing, not caused by this PR

These affect every batched two-qubit gate and should be tracked in separate issues rather than fixed here.

  • Copilot's batched-noise comment is correct, and so is the reply that it is not specific to this PR. apply_tq_faults in crates/pecos-engines/src/noise/depolarizing.rs:328 and biased_depolarizing.rs:355 samples once and applies the fault to qubits[0] and qubits[1] only, so a command carrying several pairs gets noise on the first pair alone.
  • apply_tq_faults in crates/pecos-engines/src/noise/general.rs:1203 evaluates has_leakage over the whole command inside the per-pair loop, so one leaked qubit removes every pair in the batch, including healthy ones.
  • Items 2 and 3 above exist because of wildcard arms that silently ignore unknown gate types, the same class as #575.

@ciaranra

Copy link
Copy Markdown
Member

Follow-up to the review above: the two pre-existing batched-noise defects are now tracked separately and do not need to be addressed in this PR.

Both were reproduced by execution with plain CX, so they are independent of the new gate.

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