-
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
fixed wrong time value in Trotterized Unitary #8126 #9388
Changes from 4 commits
4cc8855
9fd6158
c00e6d3
7fc6b11
ac07bd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
--- | ||
fixes: | ||
- | | ||
Fix wrong time value in Trotterized unitary #8126. | ||
When Computing a suzuki decomposition of a Hamiltonian for a given time t, if t is a float, everything works fine. | ||
If t is a Parameter object the value gets squared when it is bound. | ||
This is tested with function test_suzuki_trotterization_time_parameter(self). | ||
Refer to https://github.com/Qiskit/qiskit-terra/issues/8126 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -365,6 +365,18 @@ def test_suzuki_directly(self): | |
) | ||
np.testing.assert_array_almost_equal(evolution.to_matrix(), matrix) | ||
|
||
def test_suzuki_trotterization_time_parameter(self): | ||
"""Test for Suzuki converter""" | ||
|
||
time = 11.0 | ||
ham = (X ^ X ^ X) + (Z ^ Z ^ Z) | ||
u1 = Suzuki(1, order=1).convert(time * ham) | ||
|
||
ti = Parameter("t") | ||
u2 = Suzuki(1, order=1).convert(ti * ham) | ||
u2_t = u2.bind_parameters({ti: time}) | ||
self.assertEqual(u2_t, u1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In addition to asserting the operators are equal, add an assertion that explicitly ensures that the coefficient is 11 and not 121. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will add the expliceit assertion. |
||
|
||
def test_evolved_op_to_instruction(self): | ||
"""Test calling `to_instruction` on a plain EvolvedOp. | ||
|
||
|
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.
A positive condition is clearer than a negative condition:
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 got a DeprecationWarning: The class
qiskit.opflow.evolutions.trotterizations.suzuki.Suzuki
is deprecatied as of qiskit-terra 0.24.0. I propose to add this condition in the Suzuki class in opflow, however this will only be a temporary aprovement because the opflow class is deprecated. In thw future PauliEvolutionGate could be used.