-
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
Update qml.generator's "hamiltonian" method to follow new operator arithmetic #5421
Conversation
…into getattr-ham-lc-aliasing
…into getattr-ham-lc-aliasing
…eAI/pennylane into getattr-ham-lc-aliasing
…onian if new_opmath is enabled
Hello. You may have forgotten to update the changelog!
|
…ne into update_qml_generator
[sc-57982] |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## ham-tests #5421 +/- ##
==========================================
Coverage 99.47% 99.48%
==========================================
Files 400 400
Lines 37143 37146 +3
==========================================
+ Hits 36949 36954 +5
+ Misses 194 192 -2 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
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.
I am in favor of this change, i.e. enforcing that generators are Hamiltonian
-like instances for consistency
Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
I wouldn't say we're enforcing anything per se. This changes it specifically if you call |
Thank you so much for this @lillian542 ! Tests seem to have issues, but it is most certainly just random fluctuations |
The test failures came from |
@Qottmann Yep, I have the same issue in my PR (which is why I am still waiting to merge). |
Context:
We are generally treating "Hamiltonian" as meaning alternately
qml.ops.Hamiltonian
orqml.ops.LinearCombination
depending on whether or notnew_opmath
is enabled.The current implementation of
qml.generator(op, format="hamiltonian")
mostly respects that, but usesconvert_to_legacy_H
if the generator is aSum
,Prod
orSProd
even withnew_opmath
is enabled, breaking the convention.Description of the Change:
We add
convert_to_H
, which converts any arithmetic operator to a conceptual Hamiltonian - eitherqml.ops.Hamiltonian
orqml.ops.LinearCombination
. We use the newconvert_to_H
function in theqml.generator
method. Theconvert_to_legacy_H
function continues to convert specifically to a legacy Hamiltonian regardless of the global setting fornew_opmath
.Benefits:
Consistency, no one will ask for a generator with
new_opmath
enabled and get a legacy Hamiltonian.