Skip to content

feat: support Selene 0.3, Guppy v1, and Python 3.12+ - #480

Open
qartik wants to merge 31 commits into
devfrom
codex-upgrade-selene-guppy
Open

feat: support Selene 0.3, Guppy v1, and Python 3.12+#480
qartik wants to merge 31 commits into
devfrom
codex-upgrade-selene-guppy

Conversation

@qartik

@qartik qartik commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Upgrade selene-sim and all Selene plugin runtime dependencies to ~=0.3.0.
  • Upgrade Guppy to ~=1.0.1, with compatible HUGR and tket constraints, and retain Guppy v1 semantics throughout generated programs: output(), explicit Measurement.read(), and collect_measurements() for measurement arrays.
  • Support Python 3.12+ by dropping Python 3.10 and 3.11. The wheel ABI, cibuildwheel targets, release smoke test, package metadata, classifiers, and consistency guard now agree on 3.12.
  • Adapt PECOS Selene plugins, the shared adapter, and runtime loading to the Selene Core 0.3 batched-operation and descriptor ABI.
  • Extend HUGR-to-AST compatibility for Guppy v1 function/CFG layouts while explicitly rejecting sequential or nested conditionals until recursive lowering is implemented.
  • Migrate experimental Zlup/Zluppy packages to Python 3.12+ and preserve Zlup HUGR Measure results as direct booleans.

Why

Guppy 0.21 constrained selene-sim<0.3. Guppy 1.0.1 is the compatible stable release and requires Python 3.12 or newer.

Known limitations

  • The deterministic Z-basis gate-local canonical-frame test is a strict expected failure. Raw gate-local records still demask correctly through PauliFrameLookup; only canonical runtime emission diverges. The X-basis regression remains required, and an unexpected pass of the Z case fails CI.
  • PECOS currently rejects output(tag, array(m0, m1, m2)) when its complete runtime result-ID list is not in certified source order. This preserves correctness rather than silently misbinding detector and observable provenance; the root-cause investigation is tracked in bug: determine Guppy v1 array result provenance ordering #489.
  • SZZ traced-QIS DEM construction now fails loudly when final-readout provenance is reversed, rather than producing a potentially misbound DEM. The affected tests are strict expected failures pending the root-cause fix in SZZ surface path binds final data readout to reversed measurement IDs #498.

Validation

  • just python-ci-core debug (xdist): 4,816 passed, 71 skipped, 23 xfailed
  • Focused HUGR/Guppy/SZZ provenance and canonical-frame regression tests (xdist).
  • RUSTUP_TOOLCHAIN=stable just python-ci-lint, cargo fmt --check, and uv run --frozen pre-commit run --all-files.
  • uv lock --check and uv run --frozen python scripts/check_python_workspace.py.

@qartik
qartik marked this pull request as ready for review August 11, 2026 15:12
@qartik
qartik requested a review from ciaranra as a code owner August 11, 2026 15:12
@qartik
qartik marked this pull request as draft August 11, 2026 15:32
@qartik
qartik marked this pull request as ready for review August 11, 2026 15:35
@qartik
qartik marked this pull request as draft August 11, 2026 15:48
@qartik
qartik marked this pull request as ready for review August 11, 2026 15:49
@qartik
qartik marked this pull request as draft August 11, 2026 16:10
@qartik
qartik marked this pull request as ready for review August 11, 2026 16:22
@qartik qartik changed the title chore(deps): upgrade Selene and Guppy feat: support Selene 0.3 and Guppy v1 Aug 11, 2026
@qartik
qartik marked this pull request as draft August 11, 2026 16:57
@qartik
qartik marked this pull request as ready for review August 11, 2026 17:31
@qartik
qartik marked this pull request as draft August 11, 2026 18:44
@qartik qartik self-assigned this Aug 11, 2026
@qartik
qartik marked this pull request as ready for review August 12, 2026 13:19
[
pytest.param(
"Z",
marks=pytest.mark.xfail(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

here's the expected failure

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

Upgrades PECOS to Selene 0.3 and Guppy 1.0, adapting simulator plug-ins, HUGR processing, documentation, and Python support.

Changes:

  • Migrates Selene plug-ins and runtime integration to batched operations and descriptor APIs.
  • Updates Guppy programs to explicitly read measurement futures.
  • Raises affected Python packages to 3.12 and updates HUGR/tket dependencies.

Reviewed changes

Copilot reviewed 179 out of 195 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/generate_result_tag_hugr_fixtures.py Adds result-tag fixture generator.
scripts/generate_hugr_qis_test_fixtures.py Adds QIS fixture generator.
scripts/docs/generate_test_hugr_files.py Reads measurement futures.
ruff.toml Documents conservative syntax target.
README.md Updates Python support and example.
python/selene-plugins/pecos-selene-statevec/tests/test_statevec.py Updates Guppy measurements.
python/selene-plugins/pecos-selene-statevec/src/lib.rs Implements batched Selene operations.
python/selene-plugins/pecos-selene-statevec/pyproject.toml Updates Python and dependencies.
python/selene-plugins/pecos-selene-statevec/Cargo.toml Updates Selene revision.
python/selene-plugins/pecos-selene-stabilizer/tests/test_stab.py Updates Guppy measurements.
python/selene-plugins/pecos-selene-stabilizer/src/lib.rs Implements batched Selene operations.
python/selene-plugins/pecos-selene-stabilizer/pyproject.toml Updates Python and dependencies.
python/selene-plugins/pecos-selene-stabilizer/Cargo.toml Updates Selene revision.
python/selene-plugins/pecos-selene-stab-vec/tests/test_stab_vec.py Updates Guppy measurements.
python/selene-plugins/pecos-selene-stab-vec/src/lib.rs Implements batched Selene operations.
python/selene-plugins/pecos-selene-stab-vec/pyproject.toml Updates Python and dependencies.
python/selene-plugins/pecos-selene-stab-vec/Cargo.toml Updates Selene revision.
python/selene-plugins/pecos-selene-stab-mps/src/lib.rs Implements batched Selene operations.
python/selene-plugins/pecos-selene-stab-mps/pyproject.toml Updates Python and dependencies.
python/selene-plugins/pecos-selene-stab-mps/Cargo.toml Updates Selene revision.
python/selene-plugins/pecos-selene-mast/src/lib.rs Implements batched Selene operations.
python/selene-plugins/pecos-selene-mast/pyproject.toml Updates Python and dependencies.
python/selene-plugins/pecos-selene-mast/Cargo.toml Updates Selene revision.
python/quantum-pecos/tests/qec/test_record_vs_meas_id_semantics.py Reads measurement futures.
python/quantum-pecos/tests/qec/test_guppy_output_dem.py Migrates output and provenance tests.
python/quantum-pecos/tests/qec/test_guppy_dem_builder.py Updates Guppy measurements.
python/quantum-pecos/tests/qec/test_guppy_dem_build.py Updates Guppy measurements.
python/quantum-pecos/tests/qec/test_from_guppy_result_tags.py Updates tagged measurements.
python/quantum-pecos/tests/qec/test_from_guppy_dem.py Updates DEM test programs.
python/quantum-pecos/tests/qec/surface/test_pauli_mask_harvest.py Marks known Z-frame failure.
python/quantum-pecos/tests/qec/surface/test_noise_parameters.py Updates Guppy measurements.
python/quantum-pecos/tests/qec/surface/test_check_plan.py Expects Guppy output calls.
python/quantum-pecos/tests/pecos/test_sim_api_integration.py Updates Guppy measurements.
python/quantum-pecos/tests/pecos/test_selene_sim_parity.py Migrates outputs and arrays.
python/quantum-pecos/tests/pecos/test_selene_interface_integration.py Updates Guppy measurements.
python/quantum-pecos/tests/pecos/test_noise_builder_setter_names.py Updates Guppy measurements.
python/quantum-pecos/tests/pecos/slr/ast_tests/test_ast_roundtrip.py Updates generated-code expectation.
python/quantum-pecos/tests/pecos/slr/ast_tests/test_ast_codegen_guppy.py Updates generated-code expectations.
python/quantum-pecos/tests/guppy/test_yz_gates.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_working_guppy_pipeline.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_variant_scoped.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_v_gates.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_static_tuples.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_selene_hugr_compilation.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_selene_direct_integration.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_selene_build_process.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_rotation_extension.py Adapts optimized rotation tests.
python/quantum-pecos/tests/guppy/test_reset.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_real_quantum_circuits.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_qubit_allocation_limits.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_python_side_compilation.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_project_z.py Reads projection futures.
python/quantum-pecos/tests/guppy/test_program_fuzz.py Updates generated programs.
python/quantum-pecos/tests/guppy/test_noise_models.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_multi_module_handling.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_hugr_to_llvm_parsing.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_hugr_structure.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_hugr_compiler_parity.py Adapts Guppy 1 compiler parity.
python/quantum-pecos/tests/guppy/test_hugr_compilation.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_guppy_with_results.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_guppy_simple_pipeline.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_guppy_sim_builder.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_guppy_selene_pipeline.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_guppy_result_mechanisms.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_guppy_llvm_pipeline.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_guppy_execute_llvm.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_for_loop.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_explicit_engine_override.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_dynamic_circuits.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_current_pipeline_capabilities.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_crz_angle_arithmetic.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_comprehensive_guppy_features.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_check_hugr_format.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_call_measurement_returns.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_branch_measurement_carry.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_arithmetic_support.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_advanced_types.py Reads measurement futures.
python/quantum-pecos/tests/guppy/test_advanced_gates.py Reads measurement futures.
python/quantum-pecos/src/pecos/slr/ast/codegen/guppy.py Emits Guppy 1-compatible code.
python/quantum-pecos/src/pecos/qec/surface/decode.py Accepts unbound aggregate outputs.
python/quantum-pecos/src/pecos/qec/dem.py Updates certified result binding.
python/quantum-pecos/src/pecos/programs/__init__.py Updates Guppy examples.
python/quantum-pecos/src/pecos/guppy_gen/color.py Migrates generated color programs.
python/quantum-pecos/src/pecos/compilation_pipeline.py Generalizes HUGR comment.
python/quantum-pecos/src/pecos/circuit_converters/hugr_to_dag.py Updates Guppy examples.
python/quantum-pecos/README.md Updates supported Python versions.
python/quantum-pecos/pyproject.toml Upgrades dependencies and Python floor.
python/pecos-rslib/tests/test_hugr_integration.py Reads measurement futures.
pyproject.toml Raises workspace Python floor.
exp/zluppy/src/lib.rs Reformats Python documentation.
exp/zluppy/pyproject.toml Raises Python floor.
exp/zlup/tests/proptest.rs Excludes reserved identifiers.
exp/zlup/src/semantic.rs Simplifies slice filling.
exp/zlup/src/codegen/hugr.rs Uses measurement tokens and reads.
exp/pecos-stab-tn/Cargo.toml Updates Clippy lint configuration.
examples/surface_code_circuits/surface_code_d7.py Migrates measurements and arrays.
examples/surface_code_circuits/surface_code_d5.py Migrates measurements and arrays.
examples/surface_code_circuits/surface_code_d3.py Migrates measurements and arrays.
examples/simple_run_guppy_api.py Updates Guppy examples.
examples/rust_hugr_example.py Updates Guppy examples.
examples/guppy_integration_example.py Updates Guppy examples.
examples/guppy_builder_demo.py Updates Guppy examples.
docs/workflows/guppy-dem-decoding.md Updates Guppy measurements.
docs/user-guide/runtime-qis-tracing.md Updates Guppy measurement.
docs/user-guide/qec-guppy.md Updates Guppy APIs.
docs/user-guide/inferred-guppy-dem.md Updates inference examples.
docs/user-guide/hugr-simulation.md Updates Guppy measurements.
docs/user-guide/getting-started.md Updates Guppy measurements.
docs/user-guide/dem-from-guppy.md Updates Guppy measurements.
docs/user-guide/cuda-setup.md Updates Python requirements.
docs/user-guide/circuit-representation.md Updates Guppy example.
docs/README.md Updates Guppy measurements.
docs/development/DEVELOPMENT.md Raises development Python floor.
docs/development/ast-infrastructure.md Updates Guppy examples.
crates/pecos-uf-decoder/src/decoder.rs Simplifies optional boundary lookup.
crates/pecos-selene-core/src/lib.rs Implements batched adapter operations.
crates/pecos-selene-core/Cargo.toml Updates Selene revision.
crates/pecos-random/tests/statistical_quality.rs Uses fixed-size slice chunks.
crates/pecos-random/src/scalar_rng.rs Uses fixed-size slice chunks.
crates/pecos-random/src/rng.rs Uses fixed-size slice chunks.
crates/pecos-random/src/rng_pcg.rs Uses fixed-size slice chunks.
crates/pecos-random/src/quality_rng.rs Uses fixed-size slice chunks.
crates/pecos-quantum/src/tick_circuit.rs Simplifies tick ownership transfer.
crates/pecos-qis/src/selene_runtimes.rs Supplies Selene 0.3 defaults.
crates/pecos-qis/Cargo.toml Updates Selene dependencies.
crates/pecos-qis/build_selene.rs Adds base QIS headers.
crates/pecos-phir/src/hugr_parser.rs Supports Guppy 1 HUGR forms.
crates/pecos-hugr/src/result_tags.rs Supports measurement read operations.
crates/pecos-hugr/src/loader.rs Registers Guppy measurement extensions.
crates/pecos-hugr/src/engine/types.rs Tracks ordered measurement outcomes.
crates/pecos-hugr/src/engine/propagation.rs Extends structural value tracing.
crates/pecos-hugr/src/engine/handlers/qsystem.rs Handles future-to-measurement conversion.
crates/pecos-hugr/src/engine/handlers/futures.rs Handles measurement reads.
crates/pecos-hugr/src/engine/handlers/borrow_arr.rs Supports borrowed arrays.
crates/pecos-hugr/src/engine/handlers.rs Routes new extensions.
crates/pecos-hugr/src/engine/control_flow/tailloop.rs Resets loop-local structural state.
crates/pecos-hugr/src/engine/control_flow/conditional.rs Corrects dataflow input counts.
crates/pecos-hugr/src/engine/control_flow/call.rs Completes plain function calls.
crates/pecos-hugr-qis/tests/test_compilation.rs Removes obsolete fixture tests.
crates/pecos-hugr-qis/src/utils.rs Registers Guppy measurement extensions.
crates/pecos-hugr-qis/src/compiler.rs Updates tket LLVM pipeline.
crates/pecos-core/src/bitset.rs Simplifies bitset clearing.
Cargo.toml Upgrades tket and lint settings.
.github/workflows/test-docs-examples.yml Uses Python 3.12.
.github/workflows/python-version-consistency.yml Uses Python 3.12.
.github/workflows/python-test.yml Updates primary Python test floor.
.github/workflows/python-release.yml Updates release test matrix.

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

Comment thread crates/pecos-hugr/src/engine/types.rs
Comment thread docs/user-guide/inferred-guppy-dem.md Outdated
Comment thread python/quantum-pecos/pyproject.toml

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 202 out of 218 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

python/quantum-pecos/tests/guppy/test_hugr_compiler_parity.py:226

  • This turns every Conflicting signature mismatch into a pass, including conflicts introduced by future compiler regressions in newly added or regenerated fixtures. Please restrict the exception to an explicit set of known legacy fixture paths (or mark those cases as expected failures) so this parity test still detects new signature mismatches.

Comment thread python/quantum-pecos/pyproject.toml
@ciaranra

Copy link
Copy Markdown
Member

Re-review at 1cdc072 — blocking findings resolved

All items I raised as blocking are closed. Verified by re-running the earlier reproductions against this head rather than by reading the diff.

Finding Status
Array result-ID reversal Fails loudly; new test pins the exact shape array(m0, m1, m2) and asserts the raise
Guessed qubit targets / CX arity Fixed; re-checked for regression — MeasureOp[0], CX[1, 2]
Two sequential conditionals Now raises UnsupportedHugrStructureError — this was the last silent-wrong path
Loop + conditional Raises
abi3 chain Five crates on abi3-py312, both CIBW_BUILD on cp312-*, manylinux smoke on cp312-cp312, comments updated
Two non-existent clippy lints Removed
exp/zlup floor, tomli fallbacks Bumped / removed

tests/selene + tests/qec: 1514 passed, 1 skipped, 2 xfailed.

On the new consistency guard

I mutation-tested scripts/check_python_workspace.py rather than trusting it. Each of the four rows fails independently, with a message naming the file and the expectation:

[abi3]  ABI3 feature must be exactly 'abi3-py312', found ['abi3-py310']
[cibw]  expected two 'CIBW_BUILD: "cp312-*"' release-wheel targets
[floor] [project].requires-python must be >=3.12
[smoke] expected four '/opt/python/cp312-cp312/bin/python' smoke commands

That is the durable half of the packaging fix — the alignment can no longer silently rot, which is what let the four rows drift apart in the first place.

The converter's contract is also coherent now: it lowers the shapes it supports and raises on the rest. A completeness invariant would still be stronger than shape-by-shape rejection, since it would cover shapes nobody has thought to enumerate — but the safety property is achieved either way.

Not blocking, still open

  • The hand-written #[repr(C)] mirror and std::mem::transmute in selene_runtime.rs, and the descriptor re-resolved at all 15 per-operation call sites where upstream resolves once at load.
  • Operation::Custom still discards its tag/data/len and is dropped silently — the same shape as the RPP bug that was fixed, one match arm below it.
  • No rust-version declared, despite as_chunks_mut putting the effective floor at Rust 1.88.
  • Four tests remain weakened: the result_tags.rs indirect_call_is_nontrivial_control_flow tautology, the selene_runtimes.rs argument test that asserts a constant's shape back at itself, two relaxed engine.rs call assertions, and rz_calls >= 1.

Each is independently landable and none gates this PR.

Scope of this review

#489 correctly parks the array-output capability with the right question attached, and I have added some evidence there that should narrow it.

To be clear about what this review does and does not certify: it means the findings I raised are closed, not that the PR is defect-free. The panel never ran the full test suite, never exercised a real selene-sim host end-to-end, and reviewed the regenerated .hugr fixtures structurally rather than semantically. Merge readiness and CI are yours to call.

@ciaranra

Copy link
Copy Markdown
Member

Adversarial cross-review of 6eebab3..4fb593a

The commits after 6eebab3 — including a +320/−171 rewrite of hugr_to_ast.py and a new +90-line consistency guard — had not had independent review; my earlier rounds verified them only by re-running the previous findings' reproductions. This round covers that window with a fresh panel: three Claude Opus 5 subagents (converter behaviour, consistency guard, and a boundary-probing arm) plus an OpenAI Codex CLI read-only arm, all blind to each other, with fusion and verification by Claude Opus 5. Every claim below was re-checked against the code or by execution.

First, a concern of mine that turned out to be unfounded

I went in expecting the new UnsupportedHugrStructureError paths to be over-broad — rejecting programs that previously converted correctly. They are not. One arm ran 36 Guppy programs against each historical version of the converter inside a single guppylang 1.0.1 environment, which isolates the converter change from the Guppy version bump. Every shape now rejected was already producing wrong output beforehand:

program at 6eebab3 at 4fb593a
if a and b: x(q2) WHILE m1: <whole program>, X dropped rejected
cx(q1, q3) after an if, 3 live qubits CX q[3] — arity 1 correct
cz(q1, q0) CZ q[0,0] correct
conditional with both arms returning wrong qubit in the else arm correct

Confirmed correct at this head across straight-line 1–3 qubit programs, if with and without else, if plus a straight-line tail, both-arms-return, two-qubit gates inside a branch, asymmetric branches with a join, a qubit allocated inside a branch, empty-then, reversed operand order, and inlined helper calls. The rewrite is a substantial net correctness win, and the rejections are the right stopgap versus the silent qubit corruption they replaced.

Also retired: the shipped guppy_gen generators were already unconvertible at 6eebab3 (via TailLoop), so this range did not newly break them.

Items this PR owes

1. The consistency guard fires on legitimate maintenance edits. Both reproduced:

[add a third, CORRECT `CIBW_BUILD: "cp312-*"`]   error: expected two 'CIBW_BUILD: "cp312-*"' release-wheel targets
[normalize quoting to 'cp312-*']                 error: expected two 'CIBW_BUILD: "cp312-*"' release-wheel targets

The first is the problem: the check counts occurrences rather than validating each one, so more compliant targets is an error. pecos-rslib-cuda and -exp already carry abi3 features but have no release job — adding one is the obvious next step and hits this immediately. The four-occurrence smoke-command check has the same shape: hoisting the repeated interpreter path into a shell variable, or adding a fifth assertion, both fail. A guard that blocks unrelated work gets deleted rather than fixed.

Root cause is self-inflicted: python-version-consistency.yml runs bare python with no dependency install, so the script is stdlib-only and cannot parse YAML. Adding PyYAML (or using uv run, as the Justfile already does) allows reading jobs.*.steps[*].env.CIBW_BUILD and asserting each value equals cp312-* with no count constraint. The abi3 check has the same fix available for free — tomllib is already imported, so parsing [features] instead of regexing the file text would stop comments from tripping it.

Related false negatives worth closing while you are in there: ABI3_MANIFESTS is a hardcoded five-entry tuple, so a new crate added with abi3-py310 passes green — and because pyo3 selects the lowest enabled abi3-py3X across unified features, that one crate would silently lower the whole workspace floor. The exp/ pyproject paths are hardcoded too, while the workflow trigger is already the broader exp/**/pyproject.toml.

2. exp/zlup/uv.lock is stale. exp/zlup/pyproject.toml:5 moved to >=3.12 but exp/zlup/uv.lock:3 still records >=3.10. Verified: cd exp/zlup && uv lock --check reports "The lockfile at uv.lock needs to be updated". No workflow references exp/zlup, so CI does not catch it. One uv lock in that directory.

3. Validation ordering gives the wrong diagnosis for every TailLoop program. _analyze_cfg (which now calls _validate_control_flow_shape) runs before _check_for_loops at hugr_to_ast.py:256-259, so the accurate message is shadowed:

make_surface_code(distance=3, num_rounds=1, basis="Z")
  at 6eebab3:  "HUGR contains TailLoop structure (while/for loop)."   <- accurate
  at 4fb593a:  "HUGR CFG has sequential or nested conditionals"       <- wrong reason

Diagnostics only — these never converted — but it will mislead whoever picks up the converter work.

4. for _ in range(n) is now rejected (hugr_to_ast.py:409-431), where it previously produced a WHILE with a fabricated condition and a lost trip count. Strictly an improvement in honesty, but for loops are ordinary QEC vocabulary and there is now no path for them at all. Worth an explicit note in whatever documents the supported envelope.

Pre-existing, and explicitly not blocking this PR

Two arms independently found, and I reproduced, defects that predate #480 — I confirmed against the merge base that params was never populated and _get_condition_variable was already used the same way. These should not gate a dependency upgrade, but they are more severe than anything above and deserve their own issue:

  • Branch conditions are fabricated, not read from the predicate wire (hugr_to_ast.py:988-997). _get_condition_variable() returns the last measurement seen. Reproduced: with a measured before b, if a: x(q) emits IfStmt cond=VarExpr(name='m1') — the wrong measurement. Reported alongside: s = not r; if s: loses the negation, and a purely classical if n > 2: invents m0, producing QASM that reads a creg before the measurement that fills it, while validate() returns valid=True.
  • While conditions and trip counts are lost the same way (:964-966); test_simple_loop asserts the body and qubit index but never the condition.
  • Rotation angles are dropped (:1117-1123). Reproduced: rz(q, pi/4) emits RZ params=().
  • CFG output ports are renumbered with enumerate (:1029), so any classical value carried across a conditional shifts every qubit binding. The correct approach already exists in the same file at _capture_cfg_qubit_ports:811-851, which filters by typed port.
  • No notion of function boundaries (:312) — all FuncDefns' blocks merge into one dict and Call is never handled. The shipped d=3 surface HUGR has 4 FuncDefn, 5 CFG, 20 DataflowBlock, 3 TailLoop, 3 Call.
  • A silent flat fallback at :734-742 concatenates blocks in dict order when no pattern is recognised, discarding control flow — contrary to the fail-loud stance the rest of this work adopts.

The context that frames all of these: every Guppy program PECOS itself ships is currently unconvertible. All six guppy_gen factories raise, before and after this PR. The converter has only ever handled hand-written toy circuits, and the subset that does convert is the more dangerous half, because it yields plausible ASTs with wrong conditions that validation accepts. That is an argument for marking this module experimental until the predicate wire is actually read — not an argument against this PR.

Verdict

The four items above are small and belong here; items 1 and 2 are the ones I would not merge without, since a guard that blocks legitimate edits will not survive contact and a stale lockfile is one command. The converter work in this range is a genuine improvement and I would ship it as-is.

Scope of this review, as before: it certifies that the findings raised were examined, not that the PR is defect-free. No panel ran the full suite or exercised a real selene-sim host end-to-end.

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

Comment thread crates/pecos-phir/src/hugr_parser.rs
Comment thread python/quantum-pecos/src/pecos/qec/surface/decode.py Outdated
@ciaranra

Copy link
Copy Markdown
Member

Panel on 4fb593a..3a6ec79, plus the physics check nobody had run

Two things to report: the toolchain upgrade produces correct physics — which closes the caveat I have attached to every previous comment — and four small items in this range that I would fix here. Pre-existing defects found along the way are filed separately (#497, #498, #499) rather than raised as blockers.

Panel: three fresh Claude Opus 5 subagents (physics equivalence, provenance end-state, workspace guard) plus an OpenAI Codex CLI read-only arm on the three newest commits, all blind to each other; fusion and verification by Claude Opus 5. Every item below was re-checked against the code or by execution.

The consistency guard rewrite is correct in both directions

I re-ran the mutation matrix from my last comment. The false positives are gone and nothing was loosened:

edit before now
add a third, correct CIBW_BUILD error passes
normalize quote style error passes
downgrade abi3 / CIBW / floor / smoke interpreter caught caught, with value-based messages
new crate carrying abi3-py310 passed green caught

Parsing YAML under uv run --frozen instead of counting strings was the right call, and globbing manifests closed the false negative as well. exp/zlup/uv.lock now passes uv lock --check.

Physics verification

Seven simulator backends, 59 analytic programs, seeded and reproducible. Every deterministic prediction held with zero violations over tens of thousands of shots; every probabilistic one landed within ~2σ. The hazards this PR specifically created were all exercised directly and are clean:

  • Conditionals fire in both polarities — if b and if not b each correlate 20000/20000.
  • Teleportation of |1⟩ recovers 1 in 20000/20000 with both Bell bits 50/50; |+⟩ read in X basis gives 0 in 20000/20000.
  • Loop measurements are independent and correctly ordered — 8 fresh coins per shot, adjacent-bit agreement 0.5, deterministic pattern [1,0,0,1,0,0,1,0] reproduced 4000/4000.
  • Resets exact (post-reset 0 in 20000/20000); no cross-shot contamination (lag-1 and Wald–Wolfowitz |z| ≤ 1.05 on all seven backends).
  • All four Bell states pinned in two bases; GHZ3/GHZ4 with zero forbidden strings.
  • Surface-code LER decreases with distance below threshold, increases above it, ≈50% at p=0.3.

Clifford-only backends refuse non-Clifford programs loudly rather than answering wrongly. So the batched-operation ABI, the measurement-future rewrite, and the per-shot state clearing are all behaving.

Four items I would fix in this PR

1. tket.global_phase and tket.modifier are missing from the extension registry this PR curated. crates/pecos-hugr/src/loader.rs was edited here to drop bool and add guppy and measurement, but global_phase was not added. Consequence: rz(q, angle(2.0)) fails to load on both PECOS paths (Failed to load HUGR) while direct Selene runs it — at exactly ±2 half-turns guppylang drops the Rz and emits tket.global_phase.global_phase. ±1, ±1.5, 2.5, 3 and 4 half-turns all work, so this is a narrow but real hole. The same omission is in crates/pecos-hugr-qis/src/utils.rs, and crates/pecos-phir/src/hugr_parser.rs is missing four.

tket 0.21 exposes tket::extension::tket_extensions() -> [Arc<Extension>; 8] containing exactly the right set, including global_phase and modifier. Using it at all three sites removes the need to maintain the list by hand.

2. TailLoop bypasses the new PHIR rejection. hugr_parser.rs:264 rejects Conditional | Call | CallIndirect | LoadFunction, but TailLoop appears nowhere in the file, and process_children only handles extension ops — so a TailLoop container is skipped and its nested quantum operations are silently omitted. That is the exact failure mode the commit was written to close, left open for one container type.

3. The same rejection is scoped too broadly. reject_control_flow iterates hugr.nodes() — every node in the module — before find_operations_container selects the entrypoint. An unused helper containing a conditional therefore rejects an otherwise straight-line entrypoint that previously converted. Restricting it to the entrypoint's reachable hierarchy would fix that without weakening item 2.

4. The workflow path triggers do not cover crates/**/Cargo.toml. The script now globs every manifest, but python-version-consistency.yml triggers only on pyproject.toml, python/**, exp/** and the root Cargo.toml. Since the workspace includes crates/pecos* by glob, a new crate there with a stale abi3 feature still escapes CI. Script fixed, trigger not.

On the two provenance guards

#498 documents a live mis-binding in the SZZ surface path: abstract == traced_qis holds for the default path in both bases and fails for SZZ in both, and correcting the reversed final IDs restores parity. I reproduced this independently.

The underlying reversal may well predate this PR — I could not establish that either way. What is in scope here is that the two guards added during this review are shaped so the case passes: decode.py:1049 accepts a contiguous run in either direction, and dem.py:897 excludes the final tag by name, which is the only array-valued output the surface path has. Renaming the tag makes the guard fire with the correct diagnosis. Whatever is decided about the root cause, those two lines are worth revisiting here, since they were written in response to my earlier comments.

One pattern, four instances

Each of these came from a hand-maintained enumeration that missed a member: the extension registry (missed global_phase), the PHIR rejection denylist (missed TailLoop), ABI3_MANIFESTS (missed unlisted crates), the CIBW occurrence counts (missed a third job). Three have already been converted to derived or globbed forms in this PR, and they are the ones that now behave. tket_extensions() is the same conversion available for the fourth; an allowlist walk — raise on any node that is neither a handled op nor an explicitly-known-safe container — is the equivalent for the PHIR rejection.

Filed separately

As always, this certifies that what was examined held up, not that the PR is defect-free.

@ciaranra

Copy link
Copy Markdown
Member

Verification at 2643800 — all four items from my last comment are fixed

Confirmed by execution, not by reading the diff:

item evidence
Extension registries Hand-maintained lists replaced by tket::extension::tket_extensions() at all three sites. The previously-failing rz(q, angle(2.0)) (exact ±2 half-turns → tket.global_phase) now loads and runs end-to-end on both the internal HUGR engine and the Selene path, with the expected ~50/50 outcomes after an H
TailLoop bypass In the rejection match (hugr_parser.rs:269); 147 pecos-phir tests pass including the new coverage
Rejection scope Now once(entrypoint).chain(hugr.descendants(entrypoint)) rather than every module node, so unreachable helpers no longer poison a straight-line entrypoint
CI trigger crates/**/Cargo.toml added for both push and pull_request

Rust suites fresh-compiled: 272 tests green. CI: 87 pass, 7 skipped.

A trap for anyone verifying locally: test_global_phase_lowering_loads fails against a venv whose pecos_rslib_llvm wheel predates this commit — the registry fix lives in Rust, so Python cannot see it until the wheel is rebuilt (just build-debug). I hit exactly this false failure before rebuilding; after the rebuild it passes 6/6. CI builds wheels fresh, so it is not affected.

The one open thread

The only unresolved item from my reviews is the pair of provenance guard lines tied to #498 (the SZZ reversed final binding): decode.py:1049 accepts descending contiguity, and dem.py:897 excludes the final tag from the array cross-check. Since the reversal's root cause (Guppy/Selene lowering vs. PECOS trace capture) is undetermined and tracked in #498, resolving it there rather than in this PR is a reasonable call — I flag it only so the merge decision is made with it in view.

Everything else raised across these review rounds is closed, converted into tests, or parked with a tracking issue (#489, #493, #494, #497, #498, #499).

@ciaranra

Copy link
Copy Markdown
Member

One last request before merge: make SZZ fail loud instead of silently wrong

From the review side this is the final item. Everything else across the rounds is closed and verified; this PR is otherwise merge-ready.

As #498 documents, the SZZ surface path currently ships a DEM bound to reversed final measurement IDs — silently. The root cause is undetermined and rightly tracked there, not here. But this PR's own arc has been converting silent-wrong into fail-loud (the converter rejections, the PHIR rejections, the provenance guards), and the SZZ path is the one remaining case where a known-wrong result is still produced without complaint. The same conversion is available here in two lines plus one test annotation:

  1. decode.py:1049 — accept ascending contiguity only:

    step = 1 if final_ids[1] > final_ids[0] else -1

    becomes ascending-only (drop the -1 branch). A descending final then raises the existing "contiguous in one source-order direction" error instead of binding reversed.

  2. dem.py:897 — remove the base_tag != "final" exclusion from the array cross-check. With it gone, the guard fires on the SZZ case with the correct diagnosis (verified during review by renaming the tag: "has measurement id 20, but the generator-certified layout requires 12").

Expected fallout, stated up front: test_from_guppy_szz_surface_code_is_byte_identical_to_reference will fail — it currently passes with the reversed binding, because it feeds ref.get_meta("detectors") from the traced path into from_guppy, so both sides share whatever binding decode.py produced. It should become xfail(strict=True, reason="#498: SZZ final provenance reversed"), the same pattern this PR already uses for the canonical gate-local Z case. Any other SZZ-path test that starts raising is telling the truth and should be xfailed against #498 likewise.

The honest consequence is that SZZ becomes unusable-but-loud until #498 is resolved, instead of usable-but-wrong. Every non-SZZ configuration is unaffected — during review, abstract == traced_qis DEM equality was verified to hold for the default path across d ∈ {3,5}, rounds ∈ {0,1,2}, both bases.

If the judgment instead is that an operational-but-wrong SZZ is preferable in the interim (e.g. because the d=3 mirror may be benign for symmetric patches — unproven either way), that is a defensible owner's call; in that case merge as-is with #498 open, and this comment stands only as the record that the choice was made deliberately.

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 206 out of 223 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

python/quantum-pecos/tests/qec/surface/test_pauli_mask_harvest.py:657

  • This xfail accepts every exception type, so an unrelated setup, tracing, or provenance regression in the Z case would still pass CI. The known divergence is asserted by the comparison below; constrain the marker to AssertionError so only that expected mismatch is tolerated.

Comment thread exp/zlup/src/codegen/hugr.rs Outdated
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