fix!: Fix rotation -> float param type conversion #1061

aborgna-q · 2025-08-21T18:06:11Z

Fixes inconsistent encoding/decoding parameters. Now we keep them in half-turns most of the time, unless an specific operation requires them to be in radians.

The ParameterType used while decoding now has variants Rotation | FloatHalfTurns | FloatRadians.
Operation decoders must now specify which one they expect for their inputs.
Decoding a pytket parameter emits float operations over FloatHalfTurns, to avoid unnecessary conversions from/to radians.
The encoder always stores parameters expressions in half-turns, qsystem ops must then add a / pi before emitting their command.

BREAKING CHANGE: (pytket decoder) Renamed LoadedParameterType to ParameterType and changed its variants.
BREAKING CHANGE: (pytket decoder) Made LoadedParameter attributes private, and modified its API to support the new type variants.
BREAKING CHANGE: (pytket encoder) Changed MakeOperationArgs's parameter list to a Cow.

This is not a breaking change for python;

BEGIN_COMMIT_OVERRIDE

fix: Fix erroneous parameters being decoded from pytket for qsystem gates (#1061)

END_COMMIT_OVERRIDE

aborgna-q · 2025-08-21T18:06:32Z

tket-py/src/ops.rs

@@ -217,7 +217,7 @@ impl PyPauliIter {
 //
 // TODO: These can no longer be constructed from Python. Since `hugr-rs 0.14`
 // we need an extension and `OpDef` to defines these.
-// If fixing this, make sure to fix `PyHugrType` too.


Dummy change so this gets a line in the tket-py changelog

codecov · 2025-08-21T18:09:31Z

Codecov Report

❌ Patch coverage is 73.88889% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.13%. Comparing base (51bc0ec) to head (f9b86da).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
tket/src/serialize/pytket/decoder/param.rs	71.21%	19 Missing ⚠️
tket/src/serialize/pytket/decoder/wires.rs	87.17%	5 Missing ⚠️
tket/src/serialize/pytket/extension/bool.rs	0.00%	4 Missing ⚠️
tket/src/serialize/pytket/extension/prelude.rs	0.00%	4 Missing ⚠️
tket-qsystem/src/pytket/qsystem.rs	84.21%	2 Missing and 1 partial ⚠️
tket/src/serialize/pytket/encoder.rs	40.00%	3 Missing ⚠️
tket/src/serialize/pytket/extension/float.rs	62.50%	3 Missing ⚠️
tket/src/serialize/pytket/extension/rotation.rs	66.66%	2 Missing ⚠️
tket-qsystem/src/pytket/tests.rs	0.00%	1 Missing ⚠️
tket/src/serialize/pytket/decoder/param/parser.rs	92.85%	1 Missing ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1061      +/-   ##
==========================================
- Coverage   78.19%   78.13%   -0.06%     
==========================================
  Files         102      102              
  Lines       12783    12886     +103     
  Branches    12504    12607     +103     
==========================================
+ Hits         9996    10069      +73     
- Misses       2130     2162      +32     
+ Partials      657      655       -2

Flag	Coverage Δ
python	`81.00% <ø> (ø)`
rust	`78.07% <73.88%> (-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.

ss2165 · 2025-08-21T18:21:31Z

tket/src/serialize/pytket/decoder/param/parser.rs

-                Rule::multiply => FloatOps::fmul,
-                Rule::divide => FloatOps::fdiv,
-                Rule::power => FloatOps::fpow,
+                Rule::add => RotationOp::radd.into(),


wait why is it just addition that is mapping to rotation..?

Because that's the only arithmetic op in the rotation extension u.u

Using it instead of the float variant lets us avoid from/to conversions for most operations. It's equivalent.

tket/src/serialize/pytket/decoder/wires.rs

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

hugrbot · 2025-08-22T13:51:37Z

This PR contains breaking changes to the public Rust API.

cargo-semver-checks summary


--- failure derive_trait_impl_removed: built-in derived trait no longer implemented ---

Description:
A public type has stopped deriving one or more traits. This can break downstream code that depends on those types implementing those traits.
      ref: https://doc.rust-lang.org/reference/attributes/derive.html#derive
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/derive_trait_impl_removed.ron

Failed in:
type MakeOperationArgs no longer derives Copy, in /home/runner/work/tket2/tket2/PR_BRANCH/tket/src/serialize/pytket/encoder.rs:945

--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_missing.ron

Failed in:
enum tket::serialize::pytket::decoder::LoadedParameterType, previously in file /home/runner/work/tket2/tket2/BASELINE_BRANCH/tket/src/serialize/pytket/decoder/param.rs:15

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/inherent_method_missing.ron

Failed in:
LoadedParameter::float, previously in file /home/runner/work/tket2/tket2/BASELINE_BRANCH/tket/src/serialize/pytket/decoder/param.rs:36
LoadedParameter::as_float, previously in file /home/runner/work/tket2/tket2/BASELINE_BRANCH/tket/src/serialize/pytket/decoder/param.rs:97

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field that is no longer available under its prior name. It may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/struct_pub_field_missing.ron

Failed in:
field typ of struct LoadedParameter, previously in file /home/runner/work/tket2/tket2/BASELINE_BRANCH/tket/src/serialize/pytket/decoder/param.rs:29
field wire of struct LoadedParameter, previously in file /home/runner/work/tket2/tket2/BASELINE_BRANCH/tket/src/serialize/pytket/decoder/param.rs:31

--- failure struct_pub_field_now_doc_hidden: pub struct field is now #[doc(hidden)] ---

Description:
A pub field of a pub struct is now marked #[doc(hidden)] and is no longer part of the public API.
      ref: https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html#hidden
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/struct_pub_field_now_doc_hidden.ron

Failed in:
field LoadedParameter.typ in file /home/runner/work/tket2/tket2/PR_BRANCH/tket/src/serialize/pytket/decoder/param.rs:44
field LoadedParameter.wire in file /home/runner/work/tket2/tket2/PR_BRANCH/tket/src/serialize/pytket/decoder/param.rs:44

ss2165 · 2025-08-22T14:53:00Z

tket-qsystem/src/pytket/qsystem.rs

@@ -83,8 +84,23 @@ impl QSystemEmitter {
            }
        };

+        // We have to convert the parameters expressions (in half-turns) to radians.


comment is wrong way round?

ss2165 · 2025-08-22T14:57:56Z

tket/src/serialize/pytket/decoder.rs

+    /// See [`PytketDecoderContext::load_parameter`] for more details.
+    pub fn load_parameter_with_type(&mut self, param: &str, typ: ParameterType) -> LoadedParameter {
+        self.wire_tracker
+            .load_parameter(&mut self.builder, param, Some(typ))


seems strange to pass the typ to both load and with, but I guess it is optional in the first case

As mentioned in the load_parameter docs, that is a type hint that lets us decide for example between loading a rotation const or a float const. The output type is not guaranteed to have that type.

I guess it'd be less confusing if we change the hint for an assurance and do the with_type internally. I'll make the change.

ss2165 · 2025-08-22T15:00:59Z

tket/src/serialize/pytket/decoder/param/parser.rs

        args: Vec<PytketParam<'a>>,
+        /// The parameter types used for the inputs and outputs of this operation.


lets hope we don't run in to any that have mixed eh...

The set of operations we translate is quite restricted, so hopefully this should be enough 🤞

ss2165 · 2025-08-22T15:08:27Z

tket/src/serialize/pytket/decoder/wires.rs

+                    Some(ParameterType::FloatHalfTurns) => {
+                        let value: Value = ConstF64::new(half_turns).into();
+                        let wire = hugr.add_load_const(value);
+                        LoadedParameter::float_half_turns(wire)
+                    }
+                    _ => {
+                        let value: Value = ConstRotation::new(half_turns).unwrap().into();
+                        let wire = hugr.add_load_const(value);
+                        LoadedParameter::rotation(wire)
+                    }


I still think a pytket expression tree should be loaded in to hugr in one go as just a float expression tree, and then the final root of the tree converted in to a Rotation. I.e. this second non-type hinted case need not exist? I realise that messes with the recursive loading architecture here but it is much cleaner and avoids confusing cases like this. We can leave this for a future refactor since I don't think it actually changes behaviour (other than maybe adding/removing some needless conversions back and forth)

I guess we could change the SympyOp to return a float / add a new op to avoid breaking -exts.

But input params will still be rotations, so we'd be adding unnecessary conversions.
And same for consts.

I realize adding RotationOp::to_halfturns/from_halfturns is not expensive, but neither is checking the type here ¯\(ツ)/¯

tket/src/serialize/pytket/tests.rs

🤖 I have created a release *beep* *boop* --- ## [0.12.3](tket-py-v0.12.2...tket-py-v0.12.3) (2025-08-22) ### Features * Explicit exports for tket_exts ops and types ([#1046](#1046)) ([a32873e](a32873e)) ### Bug Fixes * Fix erroneous parameters being decoded from pytket for qsystem gates ([#1061](#1061)) ([cd42644](cd42644)) --- 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: Seyon Sivarajah <seyon.sivarajah@quantinuum.com> Co-authored-by: Seyon Sivarajah <seyon.sivarajah@cambridgequantum.com>

aborgna-q requested a review from ss2165 August 21, 2025 18:06

aborgna-q requested a review from a team as a code owner August 21, 2025 18:06

aborgna-q commented Aug 21, 2025

View reviewed changes

ss2165 requested changes Aug 21, 2025

View reviewed changes

aborgna-q added 4 commits August 22, 2025 14:11

fix: Fix rotation -> float param type conversion

3075903

fix comment

27b9805

Remove duplicated if branch

9477447

Support half-turn float wires in the decoder

620cea6

aborgna-q force-pushed the ab/fix-decode-param-types branch from 95a71d4 to 620cea6 Compare August 22, 2025 13:45

aborgna-q mentioned this pull request Aug 22, 2025

test: Add pytket->selene execution test CQCL/guppylang#1228

Draft

aborgna-q changed the title ~~fix: Fix rotation -> float param type conversion~~ fix!: Fix rotation -> float param type conversion Aug 22, 2025

fix docs

d569ab7

ss2165 approved these changes Aug 22, 2025

View reviewed changes

aborgna-q added 3 commits August 22, 2025 16:26

Fix qsystem emitter comment

c2079df

make load_parameter return parameters in the requested type

102f2d9

print in test -.-'

f9b86da

aborgna-q added this pull request to the merge queue Aug 22, 2025

Merged via the queue into main with commit cd42644 Aug 22, 2025
20 checks passed

aborgna-q deleted the ab/fix-decode-param-types branch August 22, 2025 15:55

This was referenced Aug 22, 2025

chore(py): release tket-py 0.12.3 #1063

Merged

chore: release #1064

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix!: Fix rotation -> float param type conversion #1061

fix!: Fix rotation -> float param type conversion #1061

Uh oh!

aborgna-q commented Aug 21, 2025 •

edited

Loading

Uh oh!

aborgna-q Aug 21, 2025

Uh oh!

codecov bot commented Aug 21, 2025 •

edited

Loading

Uh oh!

ss2165 Aug 21, 2025

Uh oh!

aborgna-q Aug 21, 2025

Uh oh!

Uh oh!

Uh oh!

hugrbot commented Aug 22, 2025 •

edited

Loading

Uh oh!

ss2165 Aug 22, 2025

Uh oh!

ss2165 Aug 22, 2025

Uh oh!

aborgna-q Aug 22, 2025

Uh oh!

ss2165 Aug 22, 2025

Uh oh!

aborgna-q Aug 22, 2025

Uh oh!

ss2165 Aug 22, 2025

Uh oh!

aborgna-q Aug 22, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

		args: Vec<PytketParam<'a>>,
		/// The parameter types used for the inputs and outputs of this operation.

fix!: Fix rotation -> float param type conversion #1061

fix!: Fix rotation -> float param type conversion #1061

Uh oh!

Conversation

aborgna-q commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hugrbot commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

aborgna-q Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aborgna-q commented Aug 21, 2025 •

edited

Loading

codecov bot commented Aug 21, 2025 •

edited

Loading

hugrbot commented Aug 22, 2025 •

edited

Loading

aborgna-q Aug 22, 2025 •

edited

Loading