-
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
Store grouping in Hamiltonian #1515
Conversation
…eAI/pennylane into make_hamiltonian_differentiable
…amiltonian_differentiable
…eAI/pennylane into make_hamiltonian_differentiable
Locally, codecov has 100% coverage for all files I changed, except from one line that I never changed (and which is a NotImplementedError). Also the |
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, mostly minor suggestions!
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Co-authored-by: Olivia Di Matteo <2068515+glassnotes@users.noreply.github.com>
Thanks @glassnotes I think I implemented all your suggestions and also checked the docs once more... |
pennylane/vqe/vqe.py
Outdated
compute_grouping (bool): If True, compute and store information on how to group commuting | ||
observables upon initialization. This information may be accessed when QNodes containing this | ||
Hamiltonian are executed on devices. | ||
grouping_type (str): The type of binary relation between Pauli words. | ||
Can be ``'qwc'``, ``'commuting'``, or ``'anticommuting'``. Ignored if ``compute_grouping`` is False. |
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 wonder if having both of these args is redundant?
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 point. Are you thinking
qml.Hamiltonian(obs, coeffs, grouping=None)
qml.Hamiltonian(obs, coeffs, grouping="qwc")
etc.
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.
yes
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 would find this a barrier for a user - they have to know something about the grouping to use it. My personal main use case would just be compute_grouping=True
, I don't want to look up strings of strategies...
But if you all feel it's better I'll change it!
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 admit I like it because:
- it is explicit
- it avoids the value of one argument affecting the behaviour of another (which I think is always confusing!)
- it matches other libraries, e.g.,
scipy.optimize.minimize()
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.
Cool, will change it then. method
will still be influenced by grouping_type
, but that's ok I hope...
Context:
With the new VQE workflow, we need a way for the user to define whether PennyLane and devices are supposed to use grouping. Grouping observables can take a long time and should therefore only be computed once. A way to meet both requirements is to allow users to store grouping information in the Hamiltonian, which - if found - is used by PennyLane:
Initialization with
compute_groupings=True
stores the indices required to make groups ofcommuting observables and their coefficients. These are used
(or, if not found, computed) by the
get_groupings()
method, which returns theactual groupings.
PennyLane's qnode, which currently splits a tape with
expval(H)
into tapes with a single Pauli measurement each, looks for thegrouping_indices
attribute, and if found, uses groups of Pauli measurements per tape. Also devices that support Hamiltonians can use this strategy if desired.Description of the Change:
Added grouping functionality to the Hamiltonian class, and uses it in
hamiltonian_expand
, which in turn is used by the QNode to splitexpval(H)
into Pauli expectations.Benefits:
One can now control the use of grouping behaviour in the observable.
Possible Drawbacks:
I am quite unsure about the design details. Feedback welcome!