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.

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: Load pytket circuit as a function definition #672

Merged
merged 22 commits into from
Dec 6, 2024

Conversation

tatiana-s
Copy link
Contributor

@tatiana-s tatiana-s commented Nov 29, 2024

Closes #657, however the following limitations still exist which are covered by further issues:

@tatiana-s tatiana-s requested a review from a team as a code owner November 29, 2024 10:54
@tatiana-s tatiana-s requested a review from aborgna-q November 29, 2024 10:54
@tatiana-s tatiana-s requested review from mark-koch and removed request for aborgna-q November 29, 2024 10:54
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 11 lines in your changes missing coverage. Please review.

Project coverage is 92.63%. Comparing base (50f71b9) to head (0864123).

Files with missing lines Patch % Lines
guppylang/definition/pytket_circuits.py 91.11% 8 Missing ⚠️
guppylang/decorator.py 80.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #672      +/-   ##
==========================================
- Coverage   92.75%   92.63%   -0.13%     
==========================================
  Files          68       69       +1     
  Lines        7720     7833     +113     
==========================================
+ Hits         7161     7256      +95     
- Misses        559      577      +18     

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

Copy link
Collaborator

@mark-koch mark-koch left a comment

Choose a reason for hiding this comment

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

Looks good! Mostly just some nits and suggestions.

It looks like the coverage report doesn't include the test you've written for some reason. It should be running with tket2 installed, so no idea what's going on there.

Speaking of tests, could you also add some error tests for every GuppyError that is thrown. See the tests/error directory

guppylang/checker/errors/py_errors.py Outdated Show resolved Hide resolved
guppylang/definition/pytket_circuits.py Outdated Show resolved Hide resolved
Comment on lines 159 to 160
# TODO: Error if hugr isn't FuncDefn?
if node_data and isinstance(node_data.op, FuncDefn):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, we should throw an InternalGuppyError

input_types.remove(Bool)

else:
raise GuppyError(PytketNotCircuit(self.defined_at))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to just raise this as a Python TypeError in guppy.pytket. No need to wait until compilation to throw this error

This comment was marked as outdated.

Comment on lines 251 to 262
# Negative index slicing doesn't work when num_returns is 0.
if num_returns == 0:
return CallReturnWires(
regular_returns=[],
inout_returns=list(call[0:]),
)
else:
# Circuit function returns are the other way round to Guppy returns.
return CallReturnWires(
regular_returns=list(call[-num_returns:]),
inout_returns=list(call[:-num_returns]),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider doing

Suggested change
# Negative index slicing doesn't work when num_returns is 0.
if num_returns == 0:
return CallReturnWires(
regular_returns=[],
inout_returns=list(call[0:]),
)
else:
# Circuit function returns are the other way round to Guppy returns.
return CallReturnWires(
regular_returns=list(call[-num_returns:]),
inout_returns=list(call[:-num_returns]),
)
num_inouts = len(list(call)) - num_returns
return CallReturnWires(
regular_returns=list(call[inout_returns:]),
inout_returns=list(call[:inout_returns]),
)

to avoid the 0 edge case

Comment on lines 71 to 72
def foo(q: qubit) -> None:
result = guppy_circ(q)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def foo(q: qubit) -> None:
result = guppy_circ(q)
def foo(q: qubit) -> bool:
return guppy_circ(q)

This way, validation checks that the bool is wired correctly

@tatiana-s
Copy link
Contributor Author

The reason coverage is not taking the tests into account is because -- extra pytket isn't actually installing tket2 - there is some issue with clashing versions when trying to install tket2 which is why it was commented out I assume.

@tatiana-s tatiana-s requested a review from mark-koch December 4, 2024 16:50
guppylang/definition/pytket_circuits.py Outdated Show resolved Hide resolved
guppylang/definition/pytket_circuits.py Outdated Show resolved Hide resolved
guppylang/checker/errors/py_errors.py Outdated Show resolved Hide resolved
@tatiana-s tatiana-s requested a review from mark-koch December 6, 2024 09:55
Copy link
Collaborator

@mark-koch mark-koch left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@tatiana-s tatiana-s added this pull request to the merge queue Dec 6, 2024
Merged via the queue into main with commit b21b7e1 Dec 6, 2024
3 checks passed
@tatiana-s tatiana-s deleted the ts/circuit-functions branch December 6, 2024 13:46
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2024
🤖 I have created a release *beep* *boop*
---


## [0.14.0](v0.13.1...v0.14.0)
(2024-12-19)


### ⚠ BREAKING CHANGES

* Lists in `py(...)` expressions are now turned into Guppy arrays
instead of lists.
* `dirty_qubit` function removed
* measure_return renamed to `project_z`

### Features

* add `maybe_qubit` stdlib function
([#705](#705))
([a49f70e](a49f70e)),
closes [#627](#627)
* add measure_array and discard_array quantum function
([#710](#710))
([3ad49ff](3ad49ff))
* Add method to load pytket circuit without function stub
([#712](#712))
([ee1e3de](ee1e3de))
* Add Option type to standard library
([#696](#696))
([45ea6b7](45ea6b7))
* Allow generic nat args in statically sized ranges
([#706](#706))
([f441bb8](f441bb8)),
closes [#663](#663)
* Array comprehension
([#613](#613))
([fdc0526](fdc0526)),
closes [#614](#614)
[#616](#616)
[#612](#612)
* Implicit coercion of numeric types
([#702](#702))
([df4745b](df4745b)),
closes [#701](#701)
* Load `pytket` circuit as a function definition
([#672](#672))
([b21b7e1](b21b7e1))
* Make arrays iterable
([#632](#632))
([07b9871](07b9871))
* qsystem std functions with updated primitives
([#679](#679))
([b0f041f](b0f041f))
* remove dirty_qubit
([#698](#698))
([78e366b](78e366b))
* Turn py expression lists into arrays
([#697](#697))
([d52a00a](d52a00a))
* Unpacking assignment of iterable types with static size
([#688](#688))
([602e243](602e243))
* update to hugr 0.10 and tket2 0.6
([#725](#725))
([63ea7a7](63ea7a7))


### Bug Fixes

* Accept non-negative int literals and py expressions as nats
([#708](#708))
([a93d4fe](a93d4fe)),
closes [#704](#704)
* Allow borrowing inside comprehensions
([#723](#723))
([02b6ab0](02b6ab0)),
closes [#719](#719)
* Detect unsupported default arguments
([#659](#659))
([94ac7e3](94ac7e3)),
closes [#658](#658)
* docs build command
([#729](#729))
([471b74c](471b74c))
* Ensure `int`s can be treated as booleans
([#709](#709))
([6ef6d60](6ef6d60)),
closes [#681](#681)
* Fix array execution bugs
([#731](#731))
([0f6ceaa](0f6ceaa))
* Fix implicit modules in IPython shells
([#662](#662))
([4ecb5f2](4ecb5f2)),
closes [#661](#661)
* Properly report error for unsupported constants
([#724](#724))
([d0c2da4](d0c2da4)),
closes [#721](#721)
* Properly report errors for unsupported expressions
([#692](#692))
([7f24264](7f24264)),
closes [#691](#691)
* remove use of deprecated Ellipsis
([#699](#699))
([b819a84](b819a84))


### Documentation

* Fix docs build ([#700](#700))
([684f485](684f485)),
closes [#680](#680)
* fix README.md and quickstart.md
([#654](#654))
([abb0221](abb0221))

---
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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Load circuit as a function definition
3 participants