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!: Allow array arguments to load_pytket #858

Merged
merged 14 commits into from
Mar 18, 2025
Merged

feat!: Allow array arguments to load_pytket #858

merged 14 commits into from
Mar 18, 2025

Conversation

tatiana-s
Copy link
Contributor

@tatiana-s tatiana-s commented Mar 13, 2025

Closes #671

Allows you to load pytket circuits as function definitions that use arrays as both inputs and outputs:

circ = Circuit(2)
circ.measure_all()

guppy.load_pytket("guppy_circ", circ)

@guppy
def foo(q: array[qubit, 2]) -> array[qubit, 2]:
    return guppy_circ(q)

Note that:

  • If using more registers than just the default register, this relies on the user sticking to lexicographic order for registers based on their names, with the default qubit and bit registers being q and c respectively. This is done because the tket2 conversion to HUGR relies on this order for qubit / bit ordering. It may be worth treating default registers differently in the future but that depends on user preferences
  • I think the original idea was to use arrays when the user specifies them as such in the explicit function signature - I've decided to go with the implicit version as this a) seems to be used more often by users and b) would have been more complicated to implement with more questions with regards to how much flexibility you want to give / would be expected in the signature definition, but this should be a follow up issue (Support arrays in explicit pytket circuit signatures #859)

BREAKING CHANGE: load_pytket takes arrays by default (pass use_arrays=False for qubit arguments)

@tatiana-s tatiana-s requested a review from a team as a code owner March 13, 2025 14:06
@tatiana-s tatiana-s requested a review from aborgna-q March 13, 2025 14:06
@ss2165
Copy link
Member

ss2165 commented Mar 13, 2025

is it feasible to just have one guppy.load method? If the behaviour can't be inferred automatically I suggest guppy.load_pytket does arrays by default and you add a kwarg to have individual wires.

@codecov-commenter
Copy link

codecov-commenter commented Mar 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.22%. Comparing base (75eb441) to head (b1e8ef2).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #858      +/-   ##
==========================================
+ Coverage   93.19%   93.22%   +0.03%     
==========================================
  Files          74       74              
  Lines        8711     8755      +44     
==========================================
+ Hits         8118     8162      +44     
  Misses        593      593              

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tatiana-s
Copy link
Contributor Author

is it feasible to just have one guppy.load method? If the behaviour can't be inferred automatically I suggest guppy.load_pytket does arrays by default and you add a kwarg to have individual wires.

I don't see how you would infer it at the point of loading (you have no idea whether the user wants to use arrays or values, and if I am not mistaken you can't create a guppy function that takes either arrays or values) but I've added an optional argument to load_pytket - this does make this a breaking change though

@tatiana-s tatiana-s changed the title feat: Add load_pytket_with_arrays method feat!: Allow array arguments to load_pytket Mar 13, 2025
Comment on lines 514 to 521
def load_pytket(
self, name: str, input_circuit: Any, module: GuppyModule | None = None
self,
name: str,
input_circuit: Any,
use_arrays: bool = True,
module: GuppyModule | None = None,
) -> RawLoadPytketDef:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Positional bool arguments are confusing. Could we have use_arrays be kwargs-only?
That would also avoid having a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's still a breaking change since if the default is true, previous code that uses load_pytket with individual qubit inputs will stop working

Comment on lines 217 to 218
# Pytket circuit hugr has qubit and bool wires in the opposite
# order.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Opposite order to what?

The tket2 conversion should respect pytket's input qubit order, and have qubits before bits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The guppy function has regular bit returns first before inout qubit returns

@tatiana-s tatiana-s requested a review from aborgna-q March 17, 2025 11:04
@tatiana-s tatiana-s added the wait to merge This PR to be merged after all dependencies are ready label Mar 18, 2025
@tatiana-s tatiana-s added this to the March breaking release milestone Mar 18, 2025
@ss2165 ss2165 force-pushed the ts/pytket-arrays branch from 2fa10ee to b1e8ef2 Compare March 18, 2025 14:04
@ss2165 ss2165 enabled auto-merge March 18, 2025 14:04
@ss2165 ss2165 added this pull request to the merge queue Mar 18, 2025
Merged via the queue into main with commit 37b8b80 Mar 18, 2025
3 checks passed
@ss2165 ss2165 deleted the ts/pytket-arrays branch March 18, 2025 14:12
@NathanCQC
Copy link
Collaborator

NIIIIIIICCCCEEEE ONNNNEEE

github-merge-queue bot pushed a commit that referenced this pull request Mar 18, 2025
🤖 I have created a release *beep* *boop*
---


## [0.17.0](v0.16.0...v0.17.0)
(2025-03-18)


### ⚠ BREAKING CHANGES

* `load_pytket` takes arrays by default (pass `use_arrays=False` for
qubit arguments)
* `Option` is now a builtin type.
* `angle.{__mul__, __rmul__, __truediv__, __rtruediv__` now take a
`float` instead of an `int`.

### Features

* add `get_current_shot()` to qsystem module
([#806](#806))
([3632ec6](3632ec6))
* add `Option.unwrap_nothing()` method
([#829](#829))
([abb1aa1](abb1aa1)),
closes [#810](#810)
* add barrier operation to builtins
([#849](#849))
([cf0bcfb](cf0bcfb))
* Allow array arguments to `load_pytket`
([#858](#858))
([37b8b80](37b8b80))
* Allow explicit application of type arguments
([#821](#821))
([8f90c04](8f90c04)),
closes [#770](#770)
* Generalise scalar angle operations to float
([#824](#824))
([d3f5c7f](d3f5c7f))
* Implement `float` to `int` and `nat` casts
([#831](#831))
([b56d66c](b56d66c)),
closes [#794](#794)
* **qsystem:** add Random number generation module
([08fbf47](08fbf47))
* Switch to improved iterator protocol
([#833](#833))
([348dfdc](348dfdc))


### Bug Fixes

* Correctly handle assignments of arrays in control-flow
([#845](#845))
([32ded02](32ded02)),
closes [#844](#844)
* Define `len` of arrays using Guppy
([#863](#863))
([6868ff6](6868ff6)),
closes [#804](#804)
* Fix array comprehensions with generic element type
([#865](#865))
([50df0db](50df0db)),
closes [#864](#864)
* Fix compiler diagnostics when calling `check` instead of `compile`
([#854](#854))
([9993338](9993338))
* Fix diagnostic spans for indented code
([#856](#856))
([d9fc9fd](d9fc9fd)),
closes [#852](#852)
* Fix error message for conditional shadowing of global variables
([#815](#815))
([bdaae11](bdaae11)),
closes [#772](#772)
* Fix linearity checking for array copies
([#841](#841))
([d9b085f](d9b085f)),
closes [#838](#838)
* Fix mutation of nested arrays
([#839](#839))
([ffb64f9](ffb64f9))
* Fix rendering of line breaks in diagnostics
([#819](#819))
([75efd22](75efd22)),
closes [#818](#818)
* Prevent reordering of operations with side-effects
([#855](#855))
([75eb441](75eb441))


### Documentation

* Fix API docs build
([#843](#843))
([cc1e90c](cc1e90c))

---
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
wait to merge This PR to be merged after all dependencies are ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support arrays in pytket function stubs
5 participants