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: Add almost all missing pure quantum ops from pytket to Tk2Op #430

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aborgna-q
Copy link
Collaborator

Adds most of the pure gates from https://docs.rs/tket-json-rs/latest/tket_json_rs/optype/enum.OpType.html.

This is still missing some ops:

  • Gates with n-qubit controls. That requires computing the signature dynamically, I'll add an issue for it.
  • Barrier. Related to the previous point.
  • Global Phase. I'm not sure if we want an explicit gate for it? We should discuss it first.
  • PhaseGadget. It's defined in tket-json-rs, but it doesn't appear in the pytket documentation?
  • Boxes. That needs explicit support (Translate TKET1 boxes #44)

I added docstrings, but they are missing the unitary definition. We need to add latex rendering to our rustdocs for that (I'll add an issue).

I filled some commutation properties in Tk2Op::qubit_commutation, but I'm sure its missing some more.

Closes #428

@aborgna-q aborgna-q requested a review from cqc-alec June 25, 2024 15:28
@aborgna-q aborgna-q marked this pull request as ready for review June 25, 2024 15:28
@aborgna-q aborgna-q requested a review from a team as a code owner June 25, 2024 15:28
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.33%. Comparing base (a6e9e13) to head (11557c6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #430      +/-   ##
==========================================
- Coverage   81.00%   80.33%   -0.67%     
==========================================
  Files          59       59              
  Lines        5838     5971     +133     
  Branches     5349     5440      +91     
==========================================
+ Hits         4729     4797      +68     
- Misses        870      939      +69     
+ Partials      239      235       -4     
Flag Coverage Δ
python 97.74% <100.00%> (+0.19%) ⬆️
rust 78.63% <100.00%> (-0.86%) ⬇️

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.

@ss2165
Copy link
Member

ss2165 commented Jun 25, 2024

I'm not sure we want to do this....I wanted to keep the core opset small and rebase in/out as necessary.

Most of these ops are only relevant to certain passes.

RxF64 = auto()
RyF64 = auto()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not related to this PR, but I wonder why we added the "F64" suffix to Rx, Ry and Rz, but not to other operations that have angle parameters (e.g. U1).

(I actually prefer without, for conciseness.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I was going to propose dropping the suffix in another PR

tket2/src/ops.rs Outdated Show resolved Hide resolved
tket2/src/ops.rs Outdated Show resolved Hide resolved
tket2/src/ops.rs Outdated Show resolved Hide resolved
tket2/src/ops.rs Show resolved Hide resolved
@aborgna-q
Copy link
Collaborator Author

Im not sure we want to do this....I wanted to keep the core opset small and rebase in/out as necessary.

Most of these ops are only relevant to certain passes.

Fair enough, I guess we should make different extensions at some point for the more exotic gates.

We do have some holes though that we could fix now;
e.g. there's Tk2Op::ZZPhase but no XXPhase.

Or do you think we should forego symmetry and keep it as minimal as possible?

@ss2165
Copy link
Member

ss2165 commented Jun 25, 2024

don't care too much about symmetry, zzphase is a native gate on H-series so convenient to add

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.

Define missing pure operations from pytket as Tk2Ops
3 participants