-
Notifications
You must be signed in to change notification settings - Fork 382
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 BaseEstimator import to support qiskit 2.0 #2327
base: main
Are you sure you want to change the base?
Conversation
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, this is also causing issues in the qiskit-ibm-runtime
unit tests
In my testing with Aer and Qiskit 2.0, this line is also problematic. Do you want to address it here as well?
The problem addressed by the fix in this PR and the line I pointed out are both problems with trying to do |
Ah, there is a helpful test run that now picks up the Qiskit rc and encounters further issues:
|
45d1bbe
to
e2d93d5
Compare
…te-primitive-imports
… but remove use in tests
…epted as a gate definition
…s to c_if and added a conditional skip depending on qiskit version
a085662
to
bb1869a
Compare
A couple of unit tests with fractional gates were failing because of Qiskit/qiskit#14002. Given that this PR is blocking other efforts, I added a conditional skip for these tests until the issue is addresed. |
The estimator unit tests are currently failing because of another bug in qiskit: Qiskit/qiskit#14003. A fix is on the way. In the meantime, I have chosen to skip transpilation in that specific test, as it's the part of the code that triggers the qiskit bug. |
…ng up the circuit to an arbitrary coupling map, which is something independent of the enable_truncation flag. The test now skips transpilation and exclusively tests the truncation (with enable_truncation=True, the test returns 2 instead of 4 qubits)
@@ -322,7 +322,7 @@ def test_result_order(self): | |||
qc2.ry(np.pi / 2 * param, 0) | |||
qc2.measure_all() | |||
|
|||
estimator = Estimator(approximation=True) | |||
estimator = Estimator(approximation=True, skip_transpilation=True) |
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.
Try run_options = {'shots': None}
to get exact values for the circuits that result in 0.
Alternative: change qc2 to give -1 instead of 0, then you won't have shot noise either. This can be done by changing np.pi / 2
to np.pi
in the circuit, or [1]
to [2]
in the parameter.
(Estimator has no shot noise when the expectation value is exactly -1 or 1)
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.
Thanks! I haven't used the aer estimator in a while and forgot how to set these.
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.
This option is documented at
qiskit-aer/qiskit_aer/primitives/estimator.py
Lines 68 to 75 in 076fe00
When combined with the approximation option, we get the expectation values as follows: | |
* shots is None and approximation=False: Return an expectation value with sampling-noise w/ | |
warning. | |
* shots is int and approximation=False: Return an expectation value with sampling-noise. | |
* shots is None and approximation=True: Return an exact expectation value. | |
* shots is int and approximation=True: Return expectation value with sampling-noise using a | |
normal distribution approximation. |
skip_transpilation=True
.
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.
It's likely. Something to look at in a follow-up.
Summary
Qiskit/qiskit#13877 removes the non-versioned primitive V1 base class aliases. It looks like the
AerSampler
was already using the versioned base class, butAerEstimator
wasn't. This PR updates theBaseEstimator
import toBaseEstimatorV1
.Details and comments
The PR has been extended to handle more incompatibilities with 2.0.