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

Add rule for GPI gate #804

Merged
merged 2 commits into from
Feb 21, 2024
Merged

Add rule for GPI gate #804

merged 2 commits into from
Feb 21, 2024

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Feb 15, 2024

Closes #551.

Adding rule for implementing GPI gate as R(pi) gate with arbitrary azimuthal angle.
I believe that I don't need to do anything in Qibo, right?

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8bb0327) 63.89% compared to head (16e375d) 63.94%.
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #804      +/-   ##
==========================================
+ Coverage   63.89%   63.94%   +0.04%     
==========================================
  Files          49       49              
  Lines        5792     5799       +7     
==========================================
+ Hits         3701     3708       +7     
  Misses       2091     2091              
Flag Coverage Δ
unittests 63.94% <100.00%> (+0.04%) ⬆️

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.

Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

Thanks @andrea-pasquale. The implementation looks correct to me, but it would be if @igres26 or @renatomello who have more experience with IONQ can also confirm.

We should keep in mind that the matrix representation of this gate has det=-1, unlike U3, GPI2 and others that have det=1, so we may have similar issues to the ones we had with the Hadamard.

Copy link
Contributor

@igres26 igres26 left a comment

Choose a reason for hiding this comment

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

I think there is a global phase difference of what we are doing and the explicit matrix. We might want to note that.

@andrea-pasquale
Copy link
Contributor Author

Thanks for the review @igres26 @stavros11.
I've added a comment (16e375d) with a brief explanation. Let me know if this is ok or if you would like a stronger message like a warning.

@stavros11 stavros11 merged commit d123db5 into main Feb 21, 2024
24 checks passed
@stavros11 stavros11 deleted the add_gpi branch February 21, 2024 09:53
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.

Add all gates currently available in Qibo to Qibolab
3 participants