fix: Don't use opgroup in pytket barrier encoding by aborgna-q · Pull Request #1251 · Quantinuum/tket2

aborgna-q · 2025-11-12T14:32:52Z

We were writing OPAQUE_HUGR to the pytket barrier operation's opgroup when encoding opaque subgraphs, to be able to quickly check if an incoming barrier should be decoded and return errors on invalid payloads.

However, this was an invalid use of the opgroup field which caused pytket to panic when multiple operations with the same opgroup had different signatures.

This PR removes the use of that field, and adds some helpers to check the contents of a barrier payload instead.

codecov · 2025-11-12T14:36:14Z

Codecov Report

❌ Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.12%. Comparing base (6bc3c4e) to head (45c7f9b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
tket/src/serialize/pytket/opaque.rs	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1251      +/-   ##
==========================================
+ Coverage   79.06%   79.12%   +0.05%     
==========================================
  Files         160      160              
  Lines       20421    20412       -9     
  Branches    19489    19480       -9     
==========================================
+ Hits        16145    16150       +5     
+ Misses       3289     3279      -10     
+ Partials      987      983       -4

Flag	Coverage Δ
python	`92.65% <ø> (ø)`
qis-compiler	`100.00% <ø> (ø)`
rust	`78.45% <95.45%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

acl-cqc

Yup, thanks @aborgna-q . Looks about as nice as I think it's possible to be given the tket1 serialization format...so pretty grotty 😆 but LGTM!

acl-cqc · 2025-11-12T15:30:39Z

tket/src/serialize/pytket/encoder.rs

-    /// - `opgroup`: A tket1 operation group identifier, if any.
+    /// - `opgroup`: A tket1 [operation group
+    ///   identifier](https://docs.quantinuum.com/tket/user-guide/manual/manual_circuit.html#modifying-operations-within-circuits),
+    ///   if any.


This is pub wrt the whole crate so you can't just drop the param?

It's part of the public API (custom encoders may call this function)

It's a generic call to emit arbitrary pytket commands, no reason not to allow custom optypes if someone wants to use them.

acl-cqc · 2025-11-12T15:50:57Z

tket/src/serialize/pytket/extension/core.rs

        bits: &[TrackedBit],
        params: &[LoadedParameter],
-        opgroup: Option<&str>,
+        _opgroup: Option<&str>,


:-( I guess there is no plan to deprecate this? (Some third-party encoder/decoder might still use it?)

Same here, it's part of the PytketDecoder trait. We don't lose much by giving the info to the user (pytket commands are normally an Operation + arguments + opgroup)

acl-cqc · 2025-11-12T15:52:08Z

tket/src/serialize/pytket/opaque.rs

+pub use payload::{EncodedEdgeID, OpaqueSubgraphPayload};
+
+#[expect(deprecated)]
+pub use payload::OPGROUP_OPAQUE_HUGR;


Do we need to #[deprecate] this too, or does it inherit that from the original definition?

No deprecation on re-exports, the warning is emitted if the main definition is deprecated.

acl-cqc · 2025-11-12T15:53:25Z

tket/src/serialize/pytket/opaque/payload.rs

    /// A reference to a subgraph tracked by an `OpaqueSubgraphs` registry
    /// in an [`EncodedCircuit`][super::super::circuit::EncodedCircuit]
    /// structure.
+    #[serde(rename = "HugrExternal")]


Does this mean we should declare this as a breaking PR?

These external payloads are meant to be short-lived, and are only valid when decoded by the same runtime that generated them.

You could make the case that we are breaking Inline payloads but I don't expect anyone to be storing pytket circuits with embedded Hugrs, with the aim to recover them atm

ss2165 · 2025-11-12T15:47:07Z

tket/src/serialize/pytket/extension/core.rs

        bits: &[TrackedBit],
        params: &[LoadedParameter],
-        opgroup: Option<&str>,
+        _opgroup: Option<&str>,


deprecate parameter?

#1251 (comment)

It's part of the PytketDecoder trait. We don't lose much by giving the info to the user (pytket commands are normally an Operation + arguments + opgroup)

tket/src/serialize/pytket/opaque.rs

🤖 I have created a release *beep* *boop* --- ## [0.12.11](tket-py-v0.12.10...tket-py-v0.12.11) (2025-11-13) ### Bug Fixes * **ci:** Make wheels compatible with MacOS 15.0 (#1248) ([49c5996](49c5996)) * Don't use opgroup in pytket barrier encoding (#1251) ([49c5996](49c5996)) * guppy_to_circuit always returns num_operations = 0 (#1200) ([49c5996](49c5996)) * **pytket-decoder:** Avoid QAllocating and immediately freeing qubits ([#1256](#1256)) ([228ff52](228ff52)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: Agustín Borgna <[email protected]>

## 🤖 New release * `tket`: 0.16.0 -> 0.17.0 (✓ API compatible changes) * `tket-qsystem`: 0.22.0 -> 0.23.0 (✓ API compatible changes) <details><summary>Changelog</summary> ## `tket` <blockquote> ## [0.17.0](tket-v0.16.0...tket-v0.17.0) - 2026-02-02 ### Bug Fixes - *(encoded-circ)* Track unsupported wires between input and output ([#1224](#1224)) - Multiple fixes to the pytket encoder ([#1226](#1226)) - Don't use opgroup in pytket barrier encoding ([#1251](#1251)) - guppy_to_circuit always returns num_operations = 0 ([#1200](#1200)) - *(pytket-decoder)* Avoid QAllocating and immediately freeing qubits ([#1256](#1256)) - Encoding of opaque subgraphs with no associated qubit/bit ([#1295](#1295)) - [**breaking**] Don't rely on command params for pytket barriers ([#1298](#1298)) - Track output qubits in CircuitInfo ([#1304](#1304)) - Wrongly reused qubit IDs in pytket encoding ([#1358](#1358)) ### New Features - Deprecate local find_tuple_unpack rewrite ([#1188](#1188)) - Add CopyableExpressionAST ([#1209](#1209)) - `NormalizeGuppy` pass to simplify generated structure ([#1220](#1220)) - [**breaking**] pytket EncodedCircuit struct for in-place pytket optimisation ([#1211](#1211)) - [**breaking**] Interval is independent of resource IDs and scope position ([#1205](#1205)) - Don't translate usizes to pytket ([#1241](#1241)) - BorrowSquashPass to elide redundant borrow/return ops ([#1159](#1159)) - [**breaking**] Bump hugr to 0.25.0 ([#1325](#1325)) - Remove order edges in NormalizeGuppy pass ([#1326](#1326)) - [**breaking**] Remove deprecated unpack tuple pass ([#1387](#1387)) ### Refactor - Remove contain_qubits, use TypeUnpacker ([#1283](#1283)) - [**breaking**] Replace Subcircuit with SiblingSubgraph ([#1288](#1288)) - *(metadata)* [**breaking**] Migrate all metadata keys onto the new metadata traits ([#1328](#1328)) </blockquote> ## `tket-qsystem` <blockquote> ## [0.23.0](tket-qsystem-v0.22.0...tket-qsystem-v0.23.0) - 2026-02-02 ### Bug Fixes - [**breaking**] Don't rely on command params for pytket barriers ([#1298](#1298)) - Wrongly reused qubit IDs in pytket encoding ([#1358](#1358)) ### New Features - `NormalizeGuppy` pass to simplify generated structure ([#1220](#1220)) - Allow running arbitrary serializable pytket passes on hugrs ([#1266](#1266)) - BorrowSquashPass to elide redundant borrow/return ops ([#1159](#1159)) - [**breaking**] Bump hugr to 0.25.0 ([#1325](#1325)) - Remove order edges in NormalizeGuppy pass ([#1326](#1326)) - hide new public funcs introduced by linearization ([#1333](#1333)) ### Testing - regenerate guppy_opt examples, and count gates ([#1249](#1249)) - run pytket on guppy_opt tests, measure (very limited) success ([#1250](#1250)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).

fix: Don't use opgroup in pytket barrier encoding

92c6657

aborgna-q requested a review from a team as a code owner November 12, 2025 14:32

aborgna-q requested a review from ss2165 November 12, 2025 14:32

aborgna-q added 2 commits November 12, 2025 14:56

Add an arbitrary data payload to barrier test

3d46272

Deprecate const instead of removing

66517fd

acl-cqc approved these changes Nov 12, 2025

View reviewed changes

ss2165 reviewed Nov 12, 2025

View reviewed changes

Review comments

45c7f9b

aborgna-q added this pull request to the merge queue Nov 12, 2025

Merged via the queue into main with commit 5aa4800 Nov 12, 2025
24 checks passed

aborgna-q deleted the ab/dont-set-opgroup branch November 12, 2025 16:48

hugrbot mentioned this pull request Nov 12, 2025

chore: release #1189

Merged

aborgna-q mentioned this pull request Nov 13, 2025

fix: guppy_to_circuit always returns num_operations = 0 #1200

Merged

hugrbot mentioned this pull request Nov 13, 2025

chore(py): release tket-py 0.12.11 #1253

Merged

hugrbot mentioned this pull request Feb 5, 2026

chore: release #1394

Merged

Conversation

aborgna-q commented Nov 12, 2025

Uh oh!

codecov bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Nov 12, 2025 •

edited

Loading