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
Adding functionality iterative_qpe
#4804
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4804 +/- ##
==========================================
- Coverage 99.65% 99.64% -0.01%
==========================================
Files 382 383 +1
Lines 34500 34253 -247
==========================================
- Hits 34380 34132 -248
- Misses 120 121 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work! Looks pretty much good to go. I left some stylistic comments. The only other thing I can think to add if we have time would be one integration test which tests a full workflow like: given a hamiltonian compute its ground state energy using IQPE.
Other than that, once the comments are addressed I am happy to approve 💯
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Guillermo! Such an exciting feature 😄 . Just one comment about testing gradients with other interfaces, but otherwise looks good. Once addressed, I'm ready to approve 🚀
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
I've added test for |
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
[sc-45848] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some stylistic comments, otherwise I think this is good to go! I am approving for now, feel free to merge once the comments are resolved!
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one question but otherwise I'm ready to approve :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
>>> print(qml.draw(circuit)()) | ||
|
||
1: ──H─╭●────────────H──┤↗│ │0⟩──H─╭●────────────Rϕ(-1.57)──H──┤↗│ │0⟩──H─╭●────────────Rϕ(-1.57)──Rϕ(-0.79)──H──┤↗│ │0⟩─┤ Sample Sample Sample | ||
0: ──X─╰RZ(2.00)⁴⋅⁰──────║──────────╰RZ(2.00)²⋅⁰──║──────────────║──────────╰RZ(2.00)¹⋅⁰──║──────────║──────────────────────┤ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! This was something we modified at the end to work with torch instead of tensorflow.
It is an integer. It is updated in this PR
Adding functionality
iterative_qpe