Skip to content

Enforce Gate angle arity at construction and DAG mutation - #701

Open
ciaranra wants to merge 7 commits into
devfrom
gate-angle-invariant
Open

Enforce Gate angle arity at construction and DAG mutation#701
ciaranra wants to merge 7 commits into
devfrom
gate-angle-invariant

Conversation

@ciaranra

@ciaranra ciaranra commented Sep 4, 2026

Copy link
Copy Markdown
Member

Native gates with missing or surplus angles can otherwise reach consumers that discard data or panic. Gate constructors check native angle arity, and DAG insertion and transactional mutation validate their payloads before storing them. Rejected DAG updates preserve the original gate and its wiring and measurement identities.

Scope: Gate::gate_type and Gate::angles remain public. Struct literals and mutation of owned gates can still create malformed values. This is enforcement at constructors and circuit boundaries, not a type invariant that makes invalid states unrepresentable. Encapsulating both fields and providing atomic replacement is separate work.

An adversarial review of this PR produced twelve findings, four of them high severity, including panics on input that dev accepted. All twelve are addressed here:

  • Keep CommandQueue::push, builders and FromIterator infallible. Reuse command validation at fallible execution and conversion boundaries. Add fallible shot methods to the program runner, quantum-engine adapter and importance runner; existing convenience methods delegate to them.
  • Separate rotation-angle arity from Idle's integer-duration payload. Preserve durations and signals during snapping, classify Idle independently of its duration, and reject inexact f64 duration conversion only when a core representation is required.
  • Seal Unitary::Named with a checked payload whose gate-type field is private. Unitary::try_named and UnitaryRep::try_gate share the fixed-unitary and angle checks. Direct invalid construction and payload mutation are compile errors.
  • Check zero as well as positive angle arities in the extensible validators, and apply the shared injected-noise arity guard in the importance runner. The angle-dependent noise-rate selector remains unchanged.
  • Route the three production Gate struct-literal sites through checked constructors. Preserve defensive tests using malformed literals, including measurement-resolution and sampler duplicate-ID defenses.
  • Reject surplus QASM parameters at user-defined gate argument binding, including qelib wrappers and nested calls. Remove the unreachable native-construction error wrapper.
  • Keep rejecting dynamically angled HUGR Rz, with an actionable explanation of the representation limitation and TODO(dynamic-angles) documentation.
  • Reject angle-bearing Custom gates at DAG insertion and mutation because a DAG has no gate registry to validate their arity. Unparameterized Custom gates remain supported; batched-qubit re-partnering remains unchanged.
  • Update the removed gate_mut and Rust sim_neo(circuit) documentation and restore command_queue_to_gates's must-use annotation. Rust Tick/DAG inputs require explicit fallible conversion to a command queue.

Verification (using eval "$(uv run --frozen pecos env)"):

  • Stable crates (pecos-core, pecos-quantum, pecos-simulators, pecos-engines, pecos-qasm, pecos-qec): release 5,520 passed / 9 ignored; debug 5,524 passed / 9 ignored. The difference is four existing debug-only simulator contract tests.
  • cargo test -p pecos-neo --release: 1,374 passed, none ignored.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings: passed.
  • cargo fmt --all -- --check: passed.
  • All 43 targeted mutants were killed by test failures, including constructor routes, validation and error propagation, sealed-payload privacy, and measurement defenses. Source restorations received fresh mtimes.

The execution regressions demonstrate infallible duplicate-CX queue construction followed by a structured execution/conversion error; lossless integer Idle storage with explicit rejection of inexact core conversion; Idle duration preservation under every snapping policy; duration-independent Clifford classification; and checked/compile-time rejection of invalid Named payloads.

@ciaranra
ciaranra force-pushed the gate-angle-invariant branch from 4bff01b to da077f0 Compare September 4, 2026 08:15
@ciaranra

ciaranra commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Rebased onto current dev, which has moved 14 commits since this PR's checks last ran -- including six merges touching pecos-core, pecos-quantum and pecos-simulators, the exact crates this PR changes (#714, #716, #717, #719, #722, #724).

That mattered: an earlier PR in this same convention arc (#681) looked ready at 77 green checks and turned out to have twelve stale pinned constants, because its ticks predated a convention change. Re-verifying rather than trusting the existing green.

Merged cleanly, no conflicts. Verified on the new base:

  • cargo test -p pecos-core -p pecos-quantum -p pecos-simulators green in both debug and release
  • cold cargo clippy --all-targets --all-features -- -D warnings exit 0
  • cargo fmt --all -- --check clean

No interaction with the new phase-exactness conformance tests or the pinned correctness harness.

@ciaranra

ciaranra commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Root cause of the Idle findings

The two Idle-related review findings (duration snapping to zero, and every idle-noise circuit misclassifying as non-Clifford) both trace to a single change in this PR, at exp/pecos-neo/src/command.rs:

 pub fn angle_arity(self) -> usize {
-    pecos_core::gate_type::GateType::from(self).angle_arity()
+    if self == Self::Idle {
+        1
+    } else {
+        pecos_core::gate_type::GateType::from(self).angle_arity()
+    }
 }

pecos-core already models this correctly and does not need changing: GateType::Idle.angle_arity() returns 0 (crates/pecos-core/src/gate_type.rs:755, falling through to _ => 0), and Gate::idle constructs with GateAngles::new() and the duration in params (crates/pecos-core/src/gates.rs:962-968).

The divergence is in pecos-neo. GateCommand has no params field, only angles, so GateCommand::idle stores the duration in the angle slot by reinterpreting Angle64's u64 storage (exp/pecos-neo/src/command.rs:405-425, where the comment states this outright). The special case above makes that storage pass the PR's new arity validation, and in doing so tells every angle consumer that an idle duration is a rotation angle. is_clifford_circuit then applies is_clifford_angle to it, and snap_command_queue snaps it.

The IdleDurationNotRepresentable error introduced here is a consequence of the same conflation: a duration is a u64 and needs no angle representability check.

Consequence for sequencing

The special case cannot simply be deleted here. Removing it makes Idle arity 0, which then fails validation for a GateCommand that still carries a duration in angles. The storage has to move first.

Proposed order:

  1. Idle stores its duration in the rotation-angle slot #727 lands on its own and moves the duration out of angles in GateCommand. Scope is 13 files, almost all within pecos-neo. No wire-format change is required: ByteMessage already derives angle count from gate type and writes non-angle parameters separately, and GateCommand has no serde implementation.
  2. This PR then rebases and deletes the Idle special case above along with IdleDurationNotRepresentable, so pecos-neo agrees with pecos-core that an Idle gate has zero angles.

That removes both findings at the source rather than handling Idle specially in each angle consumer, and keeps each PR to one idea.

Note that Gate.params in core is SmallVec<[f64; 2]>, so an idle duration above 2^53 is not exactly representable there. That is a separate, pre-existing issue and is entangled with the f64-based wire format, so it is out of scope for both PRs.

…ebase

# Conflicts:
#	exp/pecos-neo/src/adapter.rs
#	exp/pecos-neo/src/circuit.rs
#	exp/pecos-neo/src/command.rs
#	exp/pecos-neo/src/engines.rs
#	exp/pecos-neo/src/extensible/queue_validation.rs
#	exp/pecos-neo/src/lib.rs
#	exp/pecos-neo/src/runner.rs
#	python/pecos-rslib-exp/src/sim_neo_bindings.rs
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.

1 participant