-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix PauliEvolutionGate
(using product formulas) for all-identity Pauli terms
#13634
base: main
Are you sure you want to change the base?
Conversation
One or more of the following people are relevant to this code:
|
Co-authored-by: Alexander Ivrii <[email protected]>
Pull Request Test Coverage Report for Build 12709550670Details
💛 - Coveralls |
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.
LGTM! There is just a conflict in the test file and a minor docs suggestion.
/// I_q0 X_q1 Y_q2 Z_q3 and will use a RZ rotation angle of 0.4. | ||
/// ``(pauli_string, qubit_indices, rz_rotation_angle)``. An element of the form | ||
/// ``("XIYZ", [0,1,2,3], 2)``, for example, is interpreted in terms of qubit indices as | ||
/// I_q0 X_q1 Y_q2 Z_q3 and will use a RZ rotation angle of 2. |
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.
Given that you reordered the pauli string abovem shouldn't it be:
/// I_q0 X_q1 Y_q2 Z_q3 and will use a RZ rotation angle of 2. | |
/// X_q0 I_q1 Y_q2 Z_q3 and will use a RZ rotation angle of 2. |
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.
Whoops, yes -- good catch!
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.
Also the circuit doesn't match that string -- both fixed in 55a5357!
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.
Is there a point to add another test based on #13644? (calling hamiltonian_variational_ansatz
)?
Summary
The
PauliEvolutionGate
, if used with a product formula synthesis (this is the default),did not correctly handle all-identity terms in the operator:
Details and comments
Some minor corrections to docs are included.