-
Notifications
You must be signed in to change notification settings - Fork 603
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
QNGOptimizer_Hamiltonian #2524
QNGOptimizer_Hamiltonian #2524
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov Report
@@ Coverage Diff @@
## master #2524 +/- ##
=======================================
Coverage 99.47% 99.47%
=======================================
Files 244 244
Lines 19423 19433 +10
=======================================
+ Hits 19321 19331 +10
Misses 102 102
Continue to review full report at Codecov.
|
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.
@KetpuntoG this looks like a great fix 💪 Just had one comment, otherwise this is looking 💯
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.
@KetpuntoG this is a great bugfix! Fantastic job fixing a very complicated part of PennyLane. Like Antal, I only have very minor comments.
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 to me! Great fix 👏 The approval is conditioned on addressing Josh's comment on the changelog and the use of isinstance
.
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Thank you very much for your comments @josh146 @antalszava ! This PR has helped me to realize how much I still have to understand about pennylane 😄 |
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 great @KetpuntoG!
Context:
When using
QNGOptimizer
withDoubleExcitation
an error occurred because theDoubleExcitation
generator is a Hamiltonian and there is no diagonalization method.Description of the Change:
A function has been added to check if the generator is a Hamiltonian with more than one term (since if it has only one term it can be treated as a simple operator). If it is a Hamiltonian, we expand the operation.
Benefits:
QNGOptimizer can now be used with more gates
Possible Drawbacks:
Perhaps in other cases it is better not to expand (?)
Related GitHub Issues:
#2502