-
Notifications
You must be signed in to change notification settings - Fork 209
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 QuadraticHamiltonian class #482
Conversation
Pull Request Test Coverage Report for Build 2109658637
💛 - Coveralls |
The CI failure is unrelated to this PR.
|
Was just fixed so I updated your branch - should pass now |
2b8ad87
to
34fda9d
Compare
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.
As outlined here we can proceed with this PR.
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
@mrossinek I have addressed all your comments besides the one related to the copyright checker. If needed, I can manually squash my commits into a single one with a 2022 timestamp. |
Sounds like the copyright should be fine as only the last year is being checked. Please fix CI and then we can progress with merging this 👍 |
This needs a reno release note for this new feature |
@woodsp-ibm I've added a release note. |
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 a minor comment, other than that this lgtm 👍
"""The number of modes this operator acts on.""" | ||
return self._num_modes | ||
|
||
def to_fermionic_op(self) -> FermionicOp: |
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 wondering if we should rename this to to_second_q_op
which is inline with the namings in the properties
module.
If we (in the future) design a common base class for some matrix container that will likely be the name of this function, avoiding the deprecation of this one.
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 think the current name is much better because it precisely describes what the method does. to_second_q_op
is confusing because QuadraticHamiltonian is already a "second-quantized op" (though not a SecondQuantizedOp). From the perspective of a user trying to discover this method, I think they are more likely to guess the current name.
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
qiskit_nature/operators/second_quantization/quadratic_hamiltonian.py
Outdated
Show resolved
Hide resolved
@woodsp-ibm I have addressed your comments. Please take another look. |
I think you missed the f from the string they now need to be like Also although I put the comment in one place for the error message I was meaning it could apply to a number of the error messages in the various checks to show the failing values where that's applicable. |
@woodsp-ibm Done. |
* add QuadraticHamiltonian class * validate input * expose _fermionic_op as to_fermionic_op * use tuple labels and merge loops * update docs * add num_modes argument * document new errors * add num_modes property * update doc * spelling * mypy * from __future__ import annotations * add release note * trigger CI * fix spelling * use type declaration instead of comment * fix doc and error message * fix and improve error messages Co-authored-by: Panagiotis Barkoutsos <[email protected]> Co-authored-by: Manoel Marques <[email protected]> Co-authored-by: Max Rossmannek <[email protected]>
* add QuadraticHamiltonian class * validate input * expose _fermionic_op as to_fermionic_op * use tuple labels and merge loops * update docs * add num_modes argument * document new errors * add num_modes property * update doc * spelling * mypy * from __future__ import annotations * add release note * trigger CI * fix spelling * use type declaration instead of comment * fix doc and error message * fix and improve error messages Co-authored-by: Panagiotis Barkoutsos <[email protected]> Co-authored-by: Manoel Marques <[email protected]> Co-authored-by: Max Rossmannek <[email protected]>
* add QuadraticHamiltonian class * validate input * expose _fermionic_op as to_fermionic_op * use tuple labels and merge loops * update docs * add num_modes argument * document new errors * add num_modes property * update doc * spelling * mypy * from __future__ import annotations * add release note * trigger CI * fix spelling * use type declaration instead of comment * fix doc and error message * fix and improve error messages Co-authored-by: Panagiotis Barkoutsos <[email protected]> Co-authored-by: Manoel Marques <[email protected]> Co-authored-by: Max Rossmannek <[email protected]>
Summary
Part of #460. Adds a data structure to represent quadratic Hamiltonians.
Details and comments
I have implemented a method
_fermionic_op
to convert a QuadraticHamiltonian to a FermionicOp. For now, I have kept that method hidden because I'm not sure what general approach we should take to converting between operator types, e.g., whether we want to have instance methods or whether we want to have a functional style likeqiskit_nature.fermionic_op(quadratic_hamiltonian)