Sitelet https://github.com/PECOS-packages/PECOS/pull/977
Skip to content

Reject standalone PHIR-JSON execution that generates quantum commands and forward the boxed engine - #977

Merged
ciaranra merged 13 commits into
devfrom
phirjson-standalone-rejects-quantum
Oct 7, 2026
Merged

ciaranra merged 13 commits into
devfrom
phirjson-standalone-rejects-quantum

Conversation

@ciaranra

@ciaranra ciaranra commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Closes #883. Closes #974.

What was wrong

PhirJsonEngine is a classical-control engine: it generates quantum commands and consumes measurements supplied from outside. Run a program the intended way and it is correct. Call process(()) standalone and it was not:

How run Before
start(), a StateVecEngine, continue_processing() out = "001"
standalone from_program(...).process(()) out = "000"

The standalone path logged that it could not process quantum operations, said it was "falling back to manual direct execution for integration testing", discarded the command batch, walked the program executing only classical operations, and returned Ok(get_results()). The 0 was an export's untouched initial value, never a measurement. A caller received success for a program whose quantum half never ran, and the crate README demonstrated exactly that call.

The change

117 lines of manual replay are replaced by a loop over the existing execution cursor. An empty command batch advances; a non-empty one is an error:

PhirJsonEngine::process(()) cannot execute quantum commands; use start()/continue_processing() with a quantum engine, or the simulation builder.

Detection is dynamic, not static. A scan of the program for quantum operations would reject a program whose quantum branch is never taken; the cursor selects the branch actually executed. Dynamic detection is less restrictive and equally safe, and it also catches quantum work appearing only after an earlier empty yield. A test covers the untaken-branch case, and the mutation that swaps dynamic for static detection fails it.

Draining empty batches is required rather than incidental: Result operations and successful foreign calls both yield a batch with no quantum operation queued, so rejecting every NeedsProcessing would have broken eleven legitimate classical callers.

The boxed path, which is why a one-method fix would not have worked

Engine for Box<dyn ClassicalControlEngine> had its own process, looping and answering every NeedsProcessing with an empty ByteMessage until the engine reported completion. The public factory returns exactly that boxed type, and the box calls start/continue_processing, never the concrete process. Fixing only PhirJsonEngine::process would have left the publicly-returned path fabricating.

The box now carries the same policy as the concrete engine: start(), drain empty batches, reject a non-empty one, with its own error message so a caller can tell the two paths apart.

An earlier revision of this branch made the box forward to the concrete implementation instead, and that was a regression. An independent review executed every implementor through the boxed path and found that forwarding reintroduced, in five other engines, the same defect this change exists to remove:

Case Forwarding Corrected loop
PHIR classical export Ok({}), cursor at zero answer = 42, cursor 2, finished
QASM classical-only c = 1 c = 1
Pliron, outcomes seeded first retains stale c = 3 fresh c = 0, batch advanced
External, malformed [1,2,3] Ok({result: 0}) Err(Input("Message too small for batch header"))
quantum program, every engine fabricated or stale shot Err(Processing)

The cause is that the old boxed loop did two things: it drove the program to completion, which is necessary because several concrete process implementations return get_results() without advancing the cursor, and it answered quantum requests with nothing, which is the fabrication. Forwarding removed both. The corrected loop removes only the second, and keeping start() is what restores ExternalClassicalEngine's batch validation.

This still changes behaviour deliberately: a boxed engine whose concrete process discarded a non-empty batch now errors where it returned Ok. Discarding a batch is the defect.

Boxed coverage uses real engines in four crates — pecos-engines, pecos-phir, pecos-qasm and pecos-phir-pliron — rather than mocks, because the first version's two mock tests passed against an ExternalClassicalEngine-only mutation and so tested forwarding mechanics rather than cross-crate behaviour. The Pliron cases seed outcomes before processing, since a fresh zero result is otherwise indistinguishable from a retained stale one. Restoring the forwarding now fails three PHIR tests and two engines tests.

Removing the fallback also fixes a self-repairing test

tests/wasm_direct_test.rs detected that its result was wrong and inserted the expected value before asserting, so it could not fail — issue #974. There were two such repairs, not one. Both are removed, and real execution produces 10 and 11: the expected values were correct all along. The test had stopped reporting that the code was not.

Two analogous repairs in the WASM integration tests are removed on the same grounds.

Migrated callers

25 call sites were inventoried, 20 in registered Cargo test targets, plus the README.

Kept standalone — the classical-only callers, now requiring exact values rather than mere success: arithmetic 35, comparisons [1,1,1,1], bit operations [1,7,6,12], nested expression 60, bit access [1,0,1,5], WASM 12, 17, 10, 579, and the missing-function error test unchanged.

Migrated to a simulator — the callers that relied on fabricated success: the quantum-operation, angle-unit and machine-operation tests, and the README example. Permissive assertions are gone; one previously accepted either bit value and several tolerated a missing output entirely. Bell outcomes are now asserted as exactly {0, 3} over 32 shots, and the angle-unit and machine-operation tests assert the exact emitted commands. The machine-operation programs are also run to completion on a state-vector simulator, asserting the classical results after the machine operations (x = 1, a = 42), so a break in the classical path after a machine operation is caught.

Two fixtures changed H to X so their outcomes are deterministic. Because that thinned Hadamard coverage, a deterministic Hadamard test is added: H; Z; H is X, so it measures 1 every shot. The obvious alternative would have been worthless — H; H; Measure yields 0 even if H were the identity — whereas this fails for both the identity and X, verified by deleting the Hadamards and watching it fail.

The crate README's Rust examples are now compiled and run as doctests (#[cfg(doctest)] over include_str!("../README.md")). Doing so exposed four further stale examples: a nonexistent examples/bell.phir.json, a wrong DepolarizingNoiseModel path, a block importing a test-only helper through crate::common, and a converter example whose JSON was literally [...]. All are fixed and now run.

The five call sites under crates/pecos-phir/tests/engines/ and tests/circuits/ are in files Cargo does not register as targets, so they do not compile today. They are untouched; dead test files are tracked separately.

ByteMessage readers no longer drop data

The boxed process decides "quantum work or not" with ByteMessage::is_empty, which inherited the legacy v1 readers' leniency. Those readers returned Ok while dropping data:

  • a cut-off message header stopped the walk and silently dropped every later message;
  • an unknown message type was skipped without a bounds check;
  • a short Outcome/ReturnValue payload yielded nothing, and a long one had its surplus ignored;
  • bytes after the declared message count, and a wrong total_size, were ignored;
  • typed readers skipped other message types, so an Outcome-only batch counted as empty.

Two callers also swallowed parse errors: NoiseUtils::has_measurements mapped Err to false, and Python ByteMessage.is_empty mapped Err to true.

The readers are now strict:

  • One framing walker. A private MessageWalker replaces three duplicated helpers. It checks the batch header, total_size == byte_len, header and payload bounds, padding, and trailing bytes, using checked offset arithmetic. Every failure is an Input error naming the message (or Batch: for batch-wide faults). It bounds msg_count by what the bytes can hold before anything is reserved. Previously a 16-byte batch claiming u32::MAX messages made quantum_ops_into abort the process on allocation failure, and that path is reachable from the C ABI.
  • Typed readers require their type. quantum_ops accepts only Gate messages, outcomes only Outcome, return_value only ReturnValue (at most one). A zero-message batch reads as empty for every reader. Payload headers are read unaligned, so ReturnValueHeader's 8-byte alignment cannot panic on 4-aligned storage.
  • is_empty is a structural walk plus msg_count == 0. It no longer decodes every gate.
  • Builder. It can no longer build what the readers reject. A gate after a return value, and for_quantum_operations()/for_outcomes() after messages of another mode, now panic, as the existing mixing checks already did. Outcome/ReturnValue payloads must have their exact size. Sizes above u32::MAX panic instead of being clamped into a corrupt header with a warn!.
  • Swallowing callers. apply_measurement_reply parses outcomes once and propagates errors. has_measurements lost its only caller and is removed. Python is_empty() now raises.

No in-tree producer emits a batch the new rules reject; all engines, noise models, bridges, Selene plugins and the C ABI build single-type batches through the builder. The wire format and protocol version are unchanged. docs/development/foreign-plugins.md now states the reader contract and the C ABI's -1 / null / zero-length error outputs.

Evidence:

  • 32 new Rust tests and one Python test.
  • 41 mutations, each caught by its intended test. Three checks are unreachable by any test and are listed as such: the builder's u32::MAX panics, checked-offset overflow on 64-bit, and the Python is_empty error mapping, because Python has no raw-bytes constructor.
  • An independent review fuzzed every reader with 63.7 million random and mutated inputs: no panics.
  • On a release-build benchmark of a 1000-gate batch, parsing took 42-62 us against 64-120 us before (noisy machine), so no regression.

Simulator seed streams, and removing CuStabilizer (breaking)

Investigating QuantumSimulator.init's seed handling found two simulators giving wrong measurement statistics.

Run through HybridEngine, a noiseless Bell circuit should split roughly evenly between its two outcomes:

Backend Before, 200 shots After
StateVec, stabilizer, CuStateVec, CudaStateVec ~100 / ~100 unchanged
MPS one outcome on all 200 shots, with or without a seed 112 / 88
CudaStabilizer 000 on all 200 shots, and X; MZ returned 0 removed

MPS rebuilt pytket's MPSxGate on every reset() from the same seeded Config. pytket seeds a fresh RNG in that constructor, so every shot replayed the same draws. The wrapper now owns one random.Random(seed), created before the first reset. It sets a fresh Config.seed on each reset, so a fixed seed reproduces the whole run and every shot draws fresh randomness. Unseeded behaviour is unchanged.

CudaStabilizer / Rust CuStabilizer are removed. They wrapped cuStabilizer's Pauli-frame simulation, whose measurement table holds frame flips relative to a noiseless reference rather than outcomes, as if it were a gate-by-gate stabilizer simulator. The wrapper also started a fresh one-shot frame simulator at every measurement, losing all state. Yet it implemented CliffordGateable, StabilizerTableauSimulator, ForcedMeasurement and StabilizerSimulator, and was documented as a GPU stabilizer simulator. Its measurement trait is infallible, so it could not refuse without panicking.

This is a breaking change. Removed:

  • the Rust CuStabilizer type and its trait impls;
  • the Python CuStabilizer binding and stub;
  • pecos_rslib_cuda.is_custabilizer_usable, which now guarded nothing reachable from Python;
  • the pecos.simulators.CudaStabilizer package;
  • the "CudaStabilizer" backend name;
  • their benches, tests and example.

CuFrameSimulator, the raw frame-simulation API in the Rust crate, is kept. The docs now say no GPU stabilizer simulator is provided. #1068 tracks a correct GPU stabilizer sampler: frame samples XORed into a CPU tableau reference.

QuantumSimulator dispatch is an explicit table. Before:

  • self.backend in "state-vector" was a substring test, so 76 strings selected StateVec, including "" and "e".
  • The docstring promised a "custom backend object" that could never work, and listed Qulacs, which had no implementation.
  • A missing optional backend failed with 'NoneType' object is not callable.
  • seed was dropped by catching a TypeError and matching its message.

Now:

  • Exact names map to lazy loaders, and each records whether its constructor takes seed.
  • Unknown names and non-string objects raise ValueError at construction.
  • An unavailable backend raises an ImportError naming its dependency.
  • The TypeError retry is gone.

The seed retry itself never left a backend unseeded. SparseStabPy and CuStateVec draw from PECOS's global RNG, which HybridEngine seeds. The docstring now says so for direct callers.

The seed-determinism tests compared results to "11"/"00", but results are 3-character strings ("000", "011"). So those counts were always zero, and the "noise produces errors" check passed vacuously. They now compare int(x, 2). The different-seeds test also covers MPS, CuStateVec and CudaStateVec.

The MPS file's diff is whole-file because it had mixed CRLF/LF line endings. They are normalised to LF.

Evidence:

  • MPS stream. A stubbed unit test runs on CPU, so CI exercises it: 200 resets give 200 distinct seeds, and the same seed reproduces them. A GPU statistics test runs locally only.
  • Dispatch. CPU tests check that every name resolves correctly, that rejected names never trigger a CUDA import, and that each seed flag matches the real constructor signature.
  • Mutations. Each of these fails its tests: restoring per-reset reseeding, seeding after the first reset, substring dispatch, the TypeError retry, dropping the unavailable-backend error, marking MPS as not taking a seed, disabling the construction-time name check, and flipping CuStateVec's seed flag.
  • Review. An independent review found no remaining references to the removed API. Pickling still works on the multiprocessing path.

Out of scope, deliberately

The cause is that ClassicalEngine requires Engine<Input = (), Output = Shot>, obliging eight controllers to advertise a capability none has. Five fabricate a shot in five different ways — the box's empty-message loop, PHIR-JSON's manual replay, QASM's "best-effort results", QIS's "empty measurements", and the Python bridge's all-zero outcomes. That is #975 and is an API break. This change fixes the reachable path; QASM, QIS, PHIR, Pliron, the Monte Carlo controller and the Python bridge keep their own fabrications until #975 lands.

Coverage

crates/pecos-phir-json/tests/standalone_execution.rs is new. It pins rejection for a plain gate, a measurement, a gate in a taken branch, a machine operation emitting a command, and quantum work after an earlier export or foreign call; success for an untaken quantum branch; classical success across multiple yields; and that a foreign call executes exactly once — the removed fallback restarted its walk, so a stateful foreign call could previously have run twice, and restoring the replay changes that counter to 2.

31 mutations were applied and every one failed its intended tests before the code was restored.

Verification

On head ed281025e (dev merged in at 72dccd7d3, bringing #1050):

  • just rstest dev: exit 0; 14,077 passed, 0 failed.
  • just python-ci-lint (rustfmt, clippy with all features, pre-commit, cargo check): exit 0.
  • pecos-cuquantum tests on a local RTX 4090: exit 0, 41 passed.
  • The touched Python tests, the generated cuda_setup doc tests and the import-laziness test: 95 passed.
  • just pytest-ci-core locally: 8,918 passed. Of the 28 failures under the lane's parallel workers, 26 are CuStateVec/MPS/CudaStateVec tests hitting CUDA out-of-memory on the shared local GPU, and all 26 pass when re-run serially. The other 2 are the known test_simulators doc blocks 9 and 10, which fail with Unrecognized quantum engine builder type; that error predates this change. CI has no GPU, so the GPU tests skip there.
  • The PHIR-JSON README doctests and the machine-operation, standalone and boxed tests were each mutation-checked: breaking the asserted value or path fails them.

Noted, not changed

The box routes Engine::reset through ClassicalEngine::reset, whose QIS implementation is a no-op, so a boxed engine reset aborts no dynamic worker where a control reset aborts one. That predates this change. #1050 (now merged) adds the ClassicalEngine::reset override, and #1057 (#1044) cancels a running shot and reclaims the worker-owned interface, so a QIS shot abandoned by the boxed process becomes recoverable once those merge.

Tracing that path found two QIS defects that no existing issue covered:

They are fixed in their own stacked PRs, #1064 and #1066, not here. #1050 has merged and is included via the dev merge. The Python bridge's reset clears program, so its process already fails; unchanged here.

@ciaranra
ciaranra force-pushed the phirjson-standalone-rejects-quantum branch from 1d9a03d to 597b5a0 Compare October 2, 2026 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant