Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(py): Allow using Tk2Ops in the builder #436

Merged
merged 6 commits into from
Jun 27, 2024
Merged

feat(py): Allow using Tk2Ops in the builder #436

merged 6 commits into from
Jun 27, 2024

Conversation

aborgna-q
Copy link
Collaborator

Some module cleanups I come into while writing a notebook with examples.
Most of the diff is just moving code around.

There are two different ways to refer to operations; the commands in tket2.circuit.build and the operations in the tket2.ops.Tk2Op enum.
For the Dfg building examples we defined duplicated versions of Measure, QAlloc, QFree, Not, even though they are already defined as members of Tk2Op.
The solution to this was moving CustomOp from the circuit module into tket2.op and adding a ToCustomOp protocol, so Tk2Ops can be used in the builders.

As part of this I also had to move the type structs (to a tket2.types module) to avoid circular dependencies.

There are still annoying things here; I'd like to move the Command definitions to their own place at some point, and have some way to convert from operations to commands.
But this should be enough for now.

@aborgna-q aborgna-q requested a review from a team as a code owner June 26, 2024 13:43
@aborgna-q aborgna-q requested a review from doug-q June 26, 2024 13:43
@codecov Codecov
Copy link

codecov bot commented Jun 26, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 93.65079% with 4 lines in your changes missing coverage. Please review.

Project coverage is 81.41%. Comparing base (cddcc57) to head (e7a4a4f).
Report is 1 commits behind head on main.

Files Patch % Lines
tket2-py/tket2/ops.py 82.35% 3 Missing ⚠️
tket2-py/tket2/circuit/build.py 83.33% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #436      +/-   ##
==========================================
+ Coverage   81.26%   81.41%   +0.15%     
==========================================
  Files          59       60       +1     
  Lines        5838     5887      +49     
  Branches     5349     5378      +29     
==========================================
+ Hits         4744     4793      +49     
- Misses        854      855       +1     
+ Partials      240      239       -1
Flag Coverage Δ
python 97.05% <90.00%> (-0.50%) ⬇️
rust 79.93% <100.00%> (+0.16%) ⬆️

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
doug-q reviewed Jun 26, 2024
View reviewed changes
Copy link
Contributor

@doug-q doug-q left a comment

Choose a reason for hiding this comment

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

This looks good, but it's unfamiliar to me. I will go through a bit more carefully in the morning.

tket2/src/ops.rs
pub fn into_extension_op(self) -> ExtensionOp {
<Self as MakeRegisteredOp>::to_extension_op(self)
.expect("Failed to convert to extension op.")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing test coverage here, I don't understand the patch well enough to tell if this is concerning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some extra tests

@aborgna-q
Copy link
Collaborator Author

No worries. Maybe I should have assigned Seyon manually, since he's the one that wrote the commands stuff

@aborgna-q aborgna-q requested a review from ss2165 June 26, 2024 14:04
@doug-q
Copy link
Contributor

doug-q commented Jun 26, 2024

No worries. Maybe I should have assigned Seyon manually, since he's the one that wrote the commands stuff

I am most happy to do it, I find it a great way to learn.

doug-q
doug-q reviewed Jun 27, 2024
View reviewed changes
Copy link
Contributor

@doug-q doug-q left a comment

Choose a reason for hiding this comment

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

Looks great. I really appreciate your incremental commits, they made this very easy to review. A couple of doc fix suggestions, will approve once you have decided on those.

tket2-py/tket2/ops.py Outdated Show resolved Hide resolved
tket2-py/tket2/ops.py Outdated Show resolved Hide resolved
tket2-py/tket2/ops.py
def to_custom(self) -> CustomOp:
extension_name = "quantum.tket2"
gate_name = self.name
return CustomOp(extension_name, gate_name, [QB_T], [QB_T])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the lack of coverage here is ok, because TkOp.custom_op does have coverage, so the calling codepaths are tested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tests that cover Tk2Op (test_pauli_prop.py) currently uses the Pauli* commands definition, which is a different thing from these.

I specifically did not touch the commands in this PR since they are getting reworked once we start using the hugr builder.

This method should get some coverage once we get there :)

@aborgna-q @doug-q
Apply suggestions from code review
3573ecd 
Co-authored-by: Douglas Wilson <141026920+doug-q@users.noreply.github.com>
@aborgna-q aborgna-q added this pull request to the merge queue Jun 27, 2024
Merged via the queue into main with commit aed8651 Jun 27, 2024
16 checks passed
@aborgna-q aborgna-q deleted the ab/cleanup-python-modules branch June 27, 2024 10:40
This was referenced Jun 27, 2024
Closed
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 28, 2024
@hugrbot @aborgna-q
chore(main): release tket2-py 0.1.0a1 (#435)
ea48d16 
🤖 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 <agustin.borgna@quantinuum.com>
@hugrbot hugrbot mentioned this pull request Aug 1, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Aug 1, 2024
@hugrbot @doug-q
chore: release (#519)
248591b 
## 🤖 New release
* `tket2`: 0.1.0-alpha.2 -> 0.1.0
* `tket2-hseries`: 0.1.0

<details><summary><i><b>Changelog</b></i></summary><p>

## `tket2`
<blockquote>

##
[0.1.0](tket2-v0.1.0-alpha.2...tket2-v0.1.0)
- 2024-08-01

### Bug Fixes
- Single source of truth for circuit names, and better circuit errors
([#390](#390))
- Support non-DFG circuits
([#391](#391))
- Portmatching not matching const edges
([#444](#444))
- Pattern matcher discriminating on opaqueOp description
([#441](#441))
- `extract_dfg` inserting the output node with an invalid child order
([#442](#442))
- Recompile ecc sets after
[#441](#441)
([#484](#484))

### Documentation
- Update tket2-py readme
([#431](#431))
- Better error reporting in portmatching
([#437](#437))
- Improved multi-threading docs for Badger
([#495](#495))

### New Features
- `Circuit::operations` ([#395](#395))
- tuple unpack rewrite ([#406](#406))
- guppy → pytket conversion
([#407](#407))
- Drop linear bits, improve pytket encoding/decoding
([#420](#420))
- *(py)* Allow using `Tk2Op`s in the builder
([#436](#436))
- Initial support for `TailLoop` as circuit parent
([#417](#417))
- Support tuple unpacking with multiple unpacks
([#470](#470))
- Partial tuple unpack ([#475](#475))
- [**breaking**] Compress binary ECCs using zlib
([#498](#498))
- Add timeout options and stats to Badger
([#496](#496))
- Expose advanced Badger timeout options to tket2-py
([#506](#506))

### Refactor
- [**breaking**] Simplify tket1 conversion errors
([#408](#408))
- Cleanup tket1 serialized op structures
([#419](#419))

### Testing
- Add coverage for Badger split circuit multi-threading
([#505](#505))
</blockquote>

## `tket2-hseries`
<blockquote>

##
[0.1.0](https://github.com/CQCL/tket2/releases/tag/tket2-hseries-v0.1.0)
- 2024-08-01

### New Features
- [**breaking**] init tket2-hseries
([#368](#368))
- *(tket2-hseries)* Add `tket2.futures` Hugr extension
([#471](#471))
- Add lazify-measure pass
([#482](#482))
- add results extensions
([#494](#494))
- *(tket2-hseries)* [**breaking**] Add `HSeriesPass`
([#487](#487))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

---------

Co-authored-by: Douglas Wilson <douglas.wilson@quantinuum.com>
@hugrbot hugrbot mentioned this pull request Aug 1, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Aug 15, 2024
@hugrbot @ss2165
chore: release (#521)
2a547c8 
## 🤖 New release
* `tket2`: 0.1.0 -> 0.1.1
* `tket2-hseries`: 0.1.0 -> 0.1.1

<details><summary><i><b>Changelog</b></i></summary><p>

## `tket2`
<blockquote>

##
[0.1.0](tket2-v0.1.0-alpha.2...tket2-v0.1.0)
- 2024-08-01

### Bug Fixes
- Single source of truth for circuit names, and better circuit errors
([#390](#390))
- Support non-DFG circuits
([#391](#391))
- Portmatching not matching const edges
([#444](#444))
- Pattern matcher discriminating on opaqueOp description
([#441](#441))
- `extract_dfg` inserting the output node with an invalid child order
([#442](#442))
- Recompile ecc sets after
[#441](#441)
([#484](#484))

### Documentation
- Update tket2-py readme
([#431](#431))
- Better error reporting in portmatching
([#437](#437))
- Improved multi-threading docs for Badger
([#495](#495))

### New Features
- `Circuit::operations` ([#395](#395))
- tuple unpack rewrite ([#406](#406))
- guppy → pytket conversion
([#407](#407))
- Drop linear bits, improve pytket encoding/decoding
([#420](#420))
- *(py)* Allow using `Tk2Op`s in the builder
([#436](#436))
- Initial support for `TailLoop` as circuit parent
([#417](#417))
- Support tuple unpacking with multiple unpacks
([#470](#470))
- Partial tuple unpack ([#475](#475))
- [**breaking**] Compress binary ECCs using zlib
([#498](#498))
- Add timeout options and stats to Badger
([#496](#496))
- Expose advanced Badger timeout options to tket2-py
([#506](#506))

### Refactor
- [**breaking**] Simplify tket1 conversion errors
([#408](#408))
- Cleanup tket1 serialized op structures
([#419](#419))

### Testing
- Add coverage for Badger split circuit multi-threading
([#505](#505))
</blockquote>

## `tket2-hseries`
<blockquote>

##
[0.1.1](tket2-hseries-v0.1.0...tket2-hseries-v0.1.1)
- 2024-08-15

### New Features
- *(tket2-hseries)* make result operation internals public
([#542](#542))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

---------

Co-authored-by: Seyon Sivarajah <seyon.sivarajah@cambridgequantum.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doug-q doug-q doug-q approved these changes

@ss2165 ss2165 Awaiting requested review from ss2165

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aborgna-q @doug-q