-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat!: simplify angle extension in to a half turns rotation type #611
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #611 +/- ##
==========================================
- Coverage 82.35% 82.22% -0.13%
==========================================
Files 47 47
Lines 6534 6447 -87
Branches 6534 6447 -87
==========================================
- Hits 5381 5301 -80
Misses 788 788
+ Partials 365 358 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
tket2/src/extension/rotation.rs
Outdated
|
||
fn signature(&self) -> hugr::extension::SignatureFunc { | ||
match self { | ||
RotationOp::fromturns => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be a fallible operation. If the float is non-finite we should return None
or an error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sorry I forgot we discussed this, feel free to make the change
@@ -44,7 +44,7 @@ lazy_static! { | |||
futures::EXTENSION.name(), | |||
PRELUDE.name(), | |||
FLOAT_TYPES.name(), | |||
tket2::extension::angle::ANGLE_EXTENSION.name(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this extension does not depend on rotation. Nor did it depend on angle
@cqc-alec I've made the changes I mentioned in comments above, I am happy with this PR otherwise. Would you give another review, especially my last commit? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just minor comments.
On a bike-shedding note, not completely happy with the name "rotation" as it suggests an operation rather than a quantity, but I can't think of a better alternative to "angle".
"fromturns": { | ||
"extension": "tket2.rotation", | ||
"name": "fromturns", | ||
"description": "Construct rotation from number of turns (would be multiples of π in radians).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"description": "Construct rotation from number of turns (would be multiples of π in radians).", | |
"description": "Construct rotation from number of half turns (would be multiples of π in radians).", |
"toturns": { | ||
"extension": "tket2.rotation", | ||
"name": "toturns", | ||
"description": "Convert rotation to number of turns (would be multiples of π in radians).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"description": "Convert rotation to number of turns (would be multiples of π in radians).", | |
"description": "Convert rotation to number of half turns (would be multiples of π in radians).", |
/// The constant π | ||
pub const PI: Self = Self::new_unchecked(1.0); | ||
/// The constant 2π | ||
pub const TAU: Self = Self::new_unchecked(2.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how useful this one is. More useful might be:
pub const ZERO: Self = Self::new_unchecked(0.0);
tket2/src/extension/rotation.rs
Outdated
fn description(&self) -> String { | ||
match self { | ||
RotationOp::fromturns => { | ||
"Construct rotation from number of turns (would be multiples of π in radians)." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Construct rotation from number of turns (would be multiples of π in radians)." | |
"Construct rotation from number of half turns (would be multiples of π in radians)." |
tket2/src/extension/rotation.rs
Outdated
"Construct rotation from number of turns (would be multiples of π in radians)." | ||
} | ||
RotationOp::toturns => { | ||
"Convert rotation to number of turns (would be multiples of π in radians)." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Convert rotation to number of turns (would be multiples of π in radians)." | |
"Convert rotation to number of half turns (would be multiples of π in radians)." |
// TODO constant folding | ||
// https://github.com/CQCL/tket2/issues/405 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe need to update this issue.
@cqc-alec I independently made similar changes to half-turns. I chose to use a hyphen, and also renamed the ops. Happy to remove the hyphen if you prefer. |
## 🤖 New release * `tket2`: 0.3.0 -> 0.4.0 * `tket2-hseries`: 0.3.0 -> 0.4.0 <details><summary><i><b>Changelog</b></i></summary><p> ## `tket2` <blockquote> ## [0.4.0](tket2-v0.3.0...tket2-v0.4.0) - 2024-09-16 ### Bug Fixes - angle type docstring to say 2pi ([#607](#607)) - Fix broken ConstAngle::TAU ([#609](#609)) ### New Features - [**breaking**] simplify angle extension in to a half turns rotation type ([#611](#611)) </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> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/).
## 🤖 New release * `tket2`: 0.4.0 -> 0.5.0 (⚠️ API breaking changes) * `tket2-hseries`: 0.4.0 -> 0.5.0 (⚠️ API breaking changes) ###⚠️ `tket2` breaking changes ``` --- failure enum_marked_non_exhaustive: enum marked #[non_exhaustive] --- Description: A public enum has been marked #[non_exhaustive]. Pattern-matching on it outside of its crate must now include a wildcard pattern like `_`, or it will fail to compile. ref: https://doc.rust-lang.org/cargo/reference/semver.html#attr-adding-non-exhaustive impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/enum_marked_non_exhaustive.ron Failed in: enum InvalidPatternMatch in /tmp/.tmp7zsbCi/tket2/tket2/src/portmatching/matcher.rs:376 enum MatcherSerialisationError in /tmp/.tmp7zsbCi/tket2/tket2/src/portmatching/matcher.rs:401 enum CircuitError in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:458 enum CircuitError in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:458 enum RewriterSerialisationError in /tmp/.tmp7zsbCi/tket2/tket2/src/rewrite/ecc_rewriter.rs:209 enum PullForwardError in /tmp/.tmp7zsbCi/tket2/tket2/src/passes/commutation.rs:187 enum CircuitLoadError in /tmp/.tmp7zsbCi/tket2/tket2/src/serialize/guppy.rs:114 enum CircuitLoadError in /tmp/.tmp7zsbCi/tket2/tket2/src/serialize/guppy.rs:114 enum CircuitMutError in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:495 enum CircuitMutError in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:495 --- failure enum_tuple_variant_changed_kind: An enum tuple variant changed kind --- Description: A public enum's exhaustive tuple variant has changed to a different kind of enum variant, breaking possible instantiations and patterns. ref: https://doc.rust-lang.org/reference/items/enumerations.html impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/enum_tuple_variant_changed_kind.ron Failed in: variant PullForwardError::NoQbInCommand in /tmp/.tmp7zsbCi/tket2/tket2/src/passes/commutation.rs:194 variant PullForwardError::NoCommandForQb in /tmp/.tmp7zsbCi/tket2/tket2/src/passes/commutation.rs:200 variant CircuitMutError::DeleteNonEmptyWire in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:504 variant CircuitMutError::InvalidPortOffset in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:512 variant CircuitMutError::DeleteNonEmptyWire in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:504 variant CircuitMutError::InvalidPortOffset in /tmp/.tmp7zsbCi/tket2/tket2/src/circuit.rs:512 ``` ###⚠️ `tket2-hseries` breaking changes ``` --- failure enum_marked_non_exhaustive: enum marked #[non_exhaustive] --- Description: A public enum has been marked #[non_exhaustive]. Pattern-matching on it outside of its crate must now include a wildcard pattern like `_`, or it will fail to compile. ref: https://doc.rust-lang.org/cargo/reference/semver.html#attr-adding-non-exhaustive impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/enum_marked_non_exhaustive.ron Failed in: enum HSeriesPassError in /tmp/.tmp7zsbCi/tket2/tket2-hseries/src/lib.rs:38 enum LazifyMeasurePassError in /tmp/.tmp7zsbCi/tket2/tket2-hseries/src/lazify_measure.rs:45 enum LowerTk2Error in /tmp/.tmp7zsbCi/tket2/tket2-hseries/src/extension/hseries/lower.rs:41 --- failure enum_tuple_variant_changed_kind: An enum tuple variant changed kind --- Description: A public enum's exhaustive tuple variant has changed to a different kind of enum variant, breaking possible instantiations and patterns. ref: https://doc.rust-lang.org/reference/items/enumerations.html impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/enum_tuple_variant_changed_kind.ron Failed in: variant LowerTk2Error::Unlowered in /tmp/.tmp7zsbCi/tket2/tket2-hseries/src/extension/hseries/lower.rs:57 ``` <details><summary><i><b>Changelog</b></i></summary><p> ## `tket2` <blockquote> ## [0.5.0](tket2-v0.4.0...tket2-v0.5.0) - 2024-09-30 ### Bug Fixes - Support hugr packages, fix the notebooks ([#622](#622)) ### New Features - Add an explicit struct for the tket2 sympy op ([#616](#616)) - Support encoding float and sympy ops ([#618](#618)) </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> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/).
🤖 I have created a release *beep* *boop* --- ## [0.4.0](tket2-py-v0.3.0...tket2-py-v0.4.0) (2024-10-01) ### ⚠ BREAKING CHANGES * Made all errors `non_exhaustive`, and renamed some fields for clarity. * "tket2.angle" extension replaced with "tket2.rotation" extension with rotation type and simplified set of operations. * TryFrom implementations for extension op structs removed, use `cast` ### Features * `BadgerOptimiser.load_precompiled`, `BadgerOptimiser.compile_eccs` and `passes.badger_pass` now take an optional `cost_fn` parameter to specify the cost function to minimise. Supported values are `'cx'` (default behaviour) and `'rz'`. ([83ebfcb](83ebfcb)) * simplify angle extension in to a half turns rotation type ([#611](#611)) ([0723937](0723937)) * Support encoding float and sympy ops ([#618](#618)) ([74dcbf7](74dcbf7)) * **tket2-hseries:** cli extension dumping ([#584](#584)) ([abf292f](abf292f)) ### Bug Fixes * remove TryFrom for extension ops use `cast` ([#592](#592)) ([5ca29af](5ca29af)) * Support hugr packages, fix the notebooks ([#622](#622)) ([1cf9dcb](1cf9dcb)) ### Documentation * Add tket2-py module docstring ([#539](#539)) ([8ef7a57](8ef7a57)) ### Miscellaneous Chores * Replace thiserror with derive_more 1.0 ([#624](#624)) ([2250ce7](2250ce7)) --- 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]> Co-authored-by: Agustín Borgna <[email protected]>
## 🤖 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><i><b>Changelog</b></i></summary><p> ## `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> </p></details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). Co-authored-by: Agustín Borgna <[email protected]>
Closes #610
Closes #608
Closes #605
BREAKING CHANGE: "tket2.angle" extension replaced with "tket2.rotation" extension with rotation type and simplified set of operations.