refactor!: Replace Circuit trait with a struct #370

aborgna-q · 2024-05-31T10:38:14Z

Replaces the trait Circuit : HugrView with a struct containing a T: HugrView and a node id indicating the container node for the circuit in the hugr.

There are a few design going into this definition:

Struct with checked parent node type.

The previous Circuit trait asked the user to only use it for dataflow-based hugrs. This of course lead to functions assuming things about the hugr structure and panicking with random errors when passed an otherwise valid hugr.

Bar checking the root node at each method call, this can only be solved with an opaque struct that checks its preconditions at construction time.

The code here will throw a user-friendly error when trying to use incompatible hugrs:
```
Node(0) cannot be used as a circuit parent. A Module is not a dataflow container.
```
Generic T defaulting to Hugr

Most uses in this library manipulate circuits as owned structures (e.g. passing circuits around in badger).
However, sometimes we don't own the hugr so we cannot create a Circuit<Hugr> from it. With the generics definition we can use Circuit<&Hugr> or Circuit<&mut Hugr> to respectively view or modify non-owned structures.
Parent node pointer

Circuits represented as hugrs are not necessarily a flat dataflow structure.
For example, a guppy-defined circuit may include calls to other methods from the module.
By including a pointer to the entry point of the circuit we can keep all this structure, and rewrite it latter as necessary.
This is also useful for non-owned hugrs; we can have a Circuit<&Hugr> pointing to a region in the hugr without needing to modify anything else.

Closes #112.

BREAKING CHANGE: Replaced the Circuit trait with a wrapper struct.

codecov · 2024-05-31T10:42:42Z

Codecov Report

Attention: Patch coverage is 88.88889% with 50 lines in your changes are missing coverage. Please review.

Project coverage is 85.59%. Comparing base (58f0de4) to head (6f32713).

Files	Patch %	Lines
tket2/src/circuit.rs	88.57%	14 Missing and 2 partials ⚠️
tket2/src/optimiser/badger/hugr_pqueue.rs	45.45%	6 Missing ⚠️
tket2/src/portmatching/matcher.rs	76.92%	4 Missing and 2 partials ⚠️
tket2/src/json.rs	54.54%	4 Missing and 1 partial ⚠️
tket2/src/circuit/command.rs	80.00%	1 Missing and 3 partials ⚠️
tket2/src/passes/commutation.rs	87.50%	0 Missing and 3 partials ⚠️
tket2/src/optimiser/badger.rs	75.00%	2 Missing ⚠️
tket2/src/passes/chunks.rs	95.00%	2 Missing ⚠️
tket2/src/rewrite.rs	90.00%	0 Missing and 2 partials ⚠️
tket2/src/circuit/hash.rs	90.90%	1 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
- Coverage   85.72%   85.59%   -0.13%     
==========================================
  Files          45       45              
  Lines        4741     4893     +152     
  Branches     4290     4442     +152     
==========================================
+ Hits         4064     4188     +124     
- Misses        497      525      +28     
  Partials      180      180

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

ss2165

huge, thanks

ss2165 · 2024-06-03T12:59:06Z

tket2/src/circuit.rs

+    hugr: T,
+    /// The parent node of the circuit.
+    ///
+    /// This is checked at runtime to ensure that the node is a DFG node.


do we expect to add further restrictions in future? this single requirement is written in many places

A priori we want to support any hugr as long as the top-level node can be interpreted as a runnable circuit.

If anything, we may want to relax this later to accept FuncDecl later?

tket2/src/circuit.rs

ss2165 · 2024-06-03T13:16:06Z

tket2/src/json.rs

@@ -37,23 +36,23 @@ const METADATA_Q_REGISTERS: &str = "TKET1_JSON.qubit_registers";
 /// Explicit names for the input bit registers.
 const METADATA_B_REGISTERS: &str = "TKET1_JSON.bit_registers";

-/// A JSON-serialized circuit that can be converted to a [`Hugr`].
+/// A JSON-serialized circuit that can be converted to a [`Circuit`].


specify tket1 format

ss2165 · 2024-06-03T13:25:14Z

tket2/src/rewrite/trace.rs

 ///
 /// This is only tracked if the `rewrite-tracing` feature is enabled and
 /// `enable_rewrite_tracing` is called on the circuit before.
-pub trait RewriteTracer: Circuit + HugrMut + Sized {
+impl<T: HugrMut> Circuit<T> {


Co-authored-by: Seyon Sivarajah <[email protected]>

The commands iterator did a toposort on the whole Hugr, ignoring the hierarchy. This generated problems with #370, since circuits may now be nested somewhere in the hugr. It would also have caused problems with circuit boxes, but we don't support that yet here. We use a `SiblingGraph` now, to ensure that we only explore the top-level region of the circuit. Subcircuits will be returned as a single command. Adds a `build_module_with_circuit` helper to build circuits inside modules. Closes #42.

## 🤖 New release * `tket2`: 0.1.0-alpha.1 -> 0.1.0-alpha.2 <details><summary>Changelog</summary> <blockquote> ## [0.1.0-alpha.2](tket2-v0.1.0-alpha.1...tket2-v0.1.0-alpha.2) - 2024-06-11 ### Bug Fixes - Commands iterator ignoring the hierarchy. ([#381](#381)) ### New Features - Replace `Circuit::num_gates` with `num_operations` ([#384](#384)) - Utilities for loading compiled guppy circuits ([#393](#393)) ### Refactor - [**breaking**] Replace Circuit trait with a struct ([#370](#370)) - [**breaking**] Rename `tket2::json` into `tket2::serialize::pytket` ([#392](#392)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/).

🤖 I have created a release *beep* *boop* --- ## 0.1.0 (2024-06-28) ### ⚠ BREAKING CHANGES * require `hugr-0.5.0` * Replaced `tket2.circuit.OpConvertError` with `tket2.circuit.TK1ConvertError` in the python lib. * Moved `tket2::json` to `tket2::serialize::pytket` * Replaced the `Circuit` trait with a wrapper struct. * This is a breaking change to the compiled rewriter serialisation format. ### Features * Add a "progress timeout" to badger ([#259](#259)) ([556cf64](556cf64)) * Add missing typing hints ([#352](#352)) ([4990613](4990613)) * bindings for circuit cost and hash ([#252](#252)) ([85ce5f9](85ce5f9)) * drop pyo3 core dep ([#355](#355)) ([9f7d415](9f7d415)) * EccRewriter bindings ([#251](#251)) ([97e2e0a](97e2e0a)) * guppy → pytket conversion ([#407](#407)) ([8c5a487](8c5a487)) * Implement `PyErr` conversion locally in `tket2-py` ([#258](#258)) ([3e1a68d](3e1a68d)) * init tket2-hseries ([#368](#368)) ([61e7535](61e7535)) * pauli propagation use case example ([#333](#333)) ([f46973c](f46973c)) * **py:** Allow using `Tk2Op`s in the builder ([#436](#436)) ([aed8651](aed8651)) * Support any ops in portmatching ([#293](#293)) ([6b05a05](6b05a05)) * **tket2-py:** Bind the `lower_to_pytket` pass in python ([#439](#439)) ([8208324](8208324)) * Use tket1 and tket2 circuits interchangeably everywhere ([#243](#243)) ([eac7acf](eac7acf)) * Utilities for loading compiled guppy circuits ([#393](#393)) ([028779a](028779a)) ### Bug Fixes * failed importlib import ([#254](#254)) ([b077660](b077660)) * induced cycles in depth optimisation ([#264](#264)) ([68c9ff2](68c9ff2)), closes [#253](#253) * Make native py modules behave like python's ([#212](#212)) ([4220038](4220038)), closes [#209](#209) * pytest failing to find `tket2` in CI ([#367](#367)) ([a9df8e6](a9df8e6)) * **tket2-py:** Replace `with_hugr` helpers with `with_circ`, keeping the circuit parent. ([#438](#438)) ([b77b3cb](b77b3cb)) ### Documentation * Add some example notebooks for the python package. ([#443](#443)) ([4ed276c](4ed276c)), closes [#434](#434) * Update tket2-py readme ([6c8f18a](6c8f18a)) ### Code Refactoring * Rename `tket2::json` into `tket2::serialize::pytket` ([#392](#392)) ([93e611c](93e611c)) * Replace Circuit trait with a struct ([#370](#370)) ([ec5dd22](ec5dd22)) * Simplify tket1 conversion errors ([#408](#408)) ([b0b8aff](b0b8aff)) --- 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: Agustin Borgna <[email protected]>

refactor!: Replace Circuit trait with a struct

7edcf8c

aborgna-q requested a review from ss2165 May 31, 2024 10:38

ss2165 approved these changes Jun 3, 2024

View reviewed changes

aborgna-q and others added 3 commits June 3, 2024 16:46

Merge remote-tracking branch 'origin/main' into ab/circuit-generic

419c46f

Add comment about pytket serialisation format

f13c9cf

typo

6f32713

Co-authored-by: Seyon Sivarajah <[email protected]>

aborgna-q force-pushed the ab/circuit-generic branch from 1a62a26 to 6f32713 Compare June 3, 2024 15:53

aborgna-q mentioned this pull request Jun 3, 2024

Serialise circuit entrypoints #378

Open

aborgna-q added this pull request to the merge queue Jun 3, 2024

Merged via the queue into main with commit ec5dd22 Jun 3, 2024
11 checks passed

aborgna-q deleted the ab/circuit-generic branch June 3, 2024 16:03

This was referenced Jun 3, 2024

chore: release v0.1.0-alpha.2 #376

Closed

chore(main): release tket2-py 0.1.0 #361

Closed

This was referenced Jun 4, 2024

Test nested circuit regions & command iterator. #42

Closed

fix: Commands iterator ignoring the hierarchy. #381

Merged

qartik mentioned this pull request Jun 5, 2024

Circuit Iterator showing a command as taking in the same linear unit twice? #382

Closed

This was referenced Jun 6, 2024

chore: release v0.1.0-alpha.2 #388

Closed

chore: release v0.1.0-alpha.2 #396

Merged

hugrbot mentioned this pull request Jun 26, 2024

chore(main): release tket2-py 0.1.0a1 #435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: Replace Circuit trait with a struct #370

refactor!: Replace Circuit trait with a struct #370

aborgna-q commented May 31, 2024 •

edited

Loading

codecov bot commented May 31, 2024 •

edited

Loading

ss2165 left a comment

ss2165 Jun 3, 2024

aborgna-q Jun 3, 2024

ss2165 Jun 3, 2024

ss2165 Jun 3, 2024

refactor!: Replace Circuit trait with a struct #370

refactor!: Replace Circuit trait with a struct #370

Conversation

aborgna-q commented May 31, 2024 • edited Loading

Struct with checked parent node type.

Generic T defaulting to Hugr

Parent node pointer

codecov bot commented May 31, 2024 • edited Loading

Codecov Report

ss2165 left a comment

Choose a reason for hiding this comment

ss2165 Jun 3, 2024

Choose a reason for hiding this comment

aborgna-q Jun 3, 2024

Choose a reason for hiding this comment

ss2165 Jun 3, 2024

Choose a reason for hiding this comment

ss2165 Jun 3, 2024

Choose a reason for hiding this comment

aborgna-q commented May 31, 2024 •

edited

Loading

Generic `T` defaulting to `Hugr`

codecov bot commented May 31, 2024 •

edited

Loading