feat: Decode pytket op parameters #644

aborgna-q · 2024-10-09T12:33:59Z

This is an alternative to #635, using pure-rust instead of calling to python so we can keep the pytket decoder on the tket2 crate.

Improves parsing of pytket operation parameters by defining a grammar and parser for sympy expressions using pest (based on the calculator example for infix operation precedence).

Unrecognized operations are still put inside an opaque SympyOp, but that should be easy to change in the future.
I tested multiple sympy expressions to ensure we are able to parse them, but unrecognized ones will also fallback to SympyOp.

This is still missing routing parameters to hugr inputs (#628), as it is blocked by CQCL/hugr#1562.

drive-by: Move the pytket parameter encoding/decoding routines to ::serialize::pytket::param::{de,en}code.

Closes #637.

aborgna-q · 2024-10-09T12:35:21Z

tket2/src/serialize/pytket/encoder.rs

The missing code here got moved to pytket::param::encode

aborgna-q · 2024-10-09T12:36:28Z

Cargo.toml

+pest = "2.7.13"
+pest_derive = "2.7.13"


Using pest for parsing, as it is the one currently used in hugr-model.

aborgna-q · 2024-10-09T12:37:53Z

tket2/src/circuit.rs

-        assert_eq!(circ.num_operations(), 3);
-        assert_eq!(circ.operations().count(), 3);


Circuit operation counts are a bit over the place now, as they haven't been updated in a while to reflect the operations we care about.

We'll probably rewrite this in the -future merge, so I'm ignoring the changes for now.

codecov · 2024-10-09T12:41:31Z

Codecov Report

Attention: Patch coverage is 85.83333% with 34 lines in your changes missing coverage. Please review.

Project coverage is 81.99%. Comparing base (86ffe64) to head (c796c37).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
tket2/src/serialize/pytket/param/encode.rs	77.46%	16 Missing ⚠️
tket2/src/serialize/pytket/param/decode.rs	85.71%	5 Missing and 6 partials ⚠️
tket2/src/serialize/pytket/decoder.rs	95.23%	1 Missing and 3 partials ⚠️
tket2/src/serialize/pytket/encoder.rs	66.66%	1 Missing and 1 partial ⚠️
tket2/src/serialize/pytket/op/native.rs	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #644      +/-   ##
==========================================
+ Coverage   81.85%   81.99%   +0.13%     
==========================================
  Files          48       50       +2     
  Lines        6615     6766     +151     
  Branches     6615     6766     +151     
==========================================
+ Hits         5415     5548     +133     
- Misses        842      854      +12     
- Partials      358      364       +6

Flag	Coverage Δ
rust	`81.99% <85.83%> (+0.13%)`	⬆️

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.

doug-q

This is great. A couple of nits only.

doug-q · 2024-10-09T14:08:37Z

tket2/src/serialize/pytket/decoder.rs

+                    .out_wire(0);
+                LoadedParameter::float(wire)
+            }
+            _ => unreachable!("cannot convert {} to {}", self.typ, typ),


You can remove this unreachable by removing the early return and doing *self here.

doug-q · 2024-10-09T14:24:18Z

tket2/src/serialize/pytket/param/decode.rs

+    },
+}
+
+/// Parse a TKET1 parameter, add


incomplete comment?

doug-q · 2024-10-09T14:29:04Z

tket2/src/serialize/pytket/param/decode.rs

+/// Returns the optype given a function name.
+///
+/// If the function name is not recognized, returns `None`.
+fn get_optype(name: &str) -> Option<OpType> {


I think this would be better inlined into parse_function_call, otherwise it's name name is too general and should be changed, maybe get_optype_for_function?.

doug-q · 2024-10-09T14:32:09Z

tket2/src/serialize/pytket/param/decode.rs

+            PytketParam::Constant(4.)
+        ]
+    })]
+    fn parse_param(#[case] param: &str, #[case] expected: PytketParam) {


very nice test. I suggest adding pow to the precendence test (or a new one). One might add 3 - 2 - 1 to verify the left-associativity.

doug-q · 2024-10-09T14:33:47Z

tket2/src/serialize/pytket/param/encode.rs

+
+        let half_turns = const_angle.half_turns();
+        Some(half_turns.to_string())
+    } else if let Some(const_float) = val.get_custom_value::<ConstF64>() {


This value is in radians isn't it? shouldn't we divide by $2\pi$?

pytket param expressions are also in half turns, so no need to convert here.

The problem actually was in the decoding of pi constants. Those should be decoded to the 3.1415... float values rather than a rotation with value 1.
This lets us write expressions like 2alpha / pi correctly.

## 🤖 New release * `tket2`: 0.5.0 -> 0.6.0 (⚠️ API breaking changes) * `tket2-hseries`: 0.5.0 -> 0.6.0 (✓ API compatible changes) ### ⚠️ `tket2` breaking changes ``` --- 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.35.0/src/lints/inherent_method_missing.ron Failed in: LexicographicCostFunction::default_cx, previously in file /private/var/folders/3j/ktpgz6yj0gn05q3x3d0qqndw0000gn/T/.tmpwozYGu/tket2/src/rewrite/strategy.rs:349 --- failure pub_module_level_const_missing: pub module-level const is missing --- Description: A public const is missing, renamed, or changed from const to static. ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/pub_module_level_const_missing.ron Failed in: SYM_OP_ID in file /private/var/folders/3j/ktpgz6yj0gn05q3x3d0qqndw0000gn/T/.tmpwozYGu/tket2/src/extension/sympy.rs:27 SYM_EXPR_NAME in file /private/var/folders/3j/ktpgz6yj0gn05q3x3d0qqndw0000gn/T/.tmpwozYGu/tket2/src/extension/sympy.rs:24 --- failure struct_missing: pub struct removed or renamed --- Description: A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct 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.35.0/src/lints/struct_missing.ron Failed in: struct tket2::extension::SYM_EXPR_T, previously in file /private/var/folders/3j/ktpgz6yj0gn05q3x3d0qqndw0000gn/T/.tmpwozYGu/tket2/src/extension/sympy.rs:140 ``` <details><summary>Changelog</summary> ## `tket2` <blockquote> ## [0.6.0](tket2-v0.5.0...tket2-v0.6.0) - 2024-10-15 ### New Features - *(badger)* `cx` and `rz` const functions and strategies for `LexicographicCostFunction` ([#625](#625)) - Add `tket2.rotation.from_halfturns_unchecked` op ([#640](#640)) - [**breaking**] update to hugr 0.13.0 ([#645](#645)) - Decode pytket op parameters ([#644](#644)) - re-export hugr crate ([#652](#652)) - Extract pytket parameters to input wires ([#661](#661)) ### Refactor - [**breaking**] Remove deprecated exports ([#662](#662)) </blockquote> ## `tket2-hseries` <blockquote> ## [0.4.0](tket2-hseries-v0.3.0...tket2-hseries-v0.4.0) - 2024-09-16 ### New Features - [**breaking**] `HSeriesPass` lowers `Tk2Op`s into `HSeriesOp`s ([#602](#602)) - [**breaking**] simplify angle extension in to a half turns rotation type ([#611](#611)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). Co-authored-by: Agustín Borgna <[email protected]>

aborgna-q added 9 commits October 8, 2024 17:17

feat: Better parsing of pytket op parameters

d83000c

Parse unary minus operation

919075b

module docs

4bfafbe

Simplify visibility

ec3d83c

Add sympy's implicit multiply

92b5975

functions with no params

0155c23

Avoid unnecessary float<->rot conversions

92f9333

docs cleanup

b39a0c2

Undo API-breaking change

59df3dd

aborgna-q requested a review from doug-q October 9, 2024 12:33

aborgna-q requested a review from a team as a code owner October 9, 2024 12:34

aborgna-q commented Oct 9, 2024

View reviewed changes

tket2/src/serialize/pytket/encoder.rs Outdated

Copy link

Collaborator Author

aborgna-q Oct 9, 2024

Choose a reason for hiding this comment

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

The missing code here got moved to pytket::param::encode

aborgna-q commented Oct 9, 2024

View reviewed changes

aborgna-q changed the title ~~feat: Decode pytket parameters~~ feat: Decode pytket op parameters Oct 9, 2024

doug-q approved these changes Oct 9, 2024

View reviewed changes

aborgna-q added 5 commits October 9, 2024 16:39

Fix doc

4923aab

Simplify match

a376a9b

Inline get_optype translation

9332476

Add more tests

ffe7bd3

fix: "pi" in expression should be 3.14..

c796c37

aborgna-q added this pull request to the merge queue Oct 10, 2024

Merged via the queue into main with commit ccf53b5 Oct 10, 2024
16 checks passed

aborgna-q deleted the ab/decode-params branch October 10, 2024 08:16

This was referenced Oct 11, 2024

ci: release-plz fails on CI #654

Closed

chore: release v0.6.0 #626

Merged

hugrbot mentioned this pull request Oct 21, 2024

chore: release v0.7.0 #667

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Decode pytket op parameters #644

feat: Decode pytket op parameters #644

aborgna-q commented Oct 9, 2024

aborgna-q Oct 9, 2024

aborgna-q Oct 9, 2024

aborgna-q Oct 9, 2024

codecov bot commented Oct 9, 2024 •

edited

Loading

doug-q left a comment

doug-q Oct 9, 2024

doug-q Oct 9, 2024

doug-q Oct 9, 2024

doug-q Oct 9, 2024

doug-q Oct 9, 2024

aborgna-q Oct 9, 2024 •

edited

Loading

		assert_eq!(circ.num_operations(), 3);
		assert_eq!(circ.operations().count(), 3);

		pest = "2.7.13"
		pest_derive = "2.7.13"

feat: Decode pytket op parameters #644

feat: Decode pytket op parameters #644

Conversation

aborgna-q commented Oct 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 9, 2024 • edited Loading

Codecov Report

doug-q left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aborgna-q Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Oct 9, 2024 •

edited

Loading

aborgna-q Oct 9, 2024 •

edited

Loading