-
Notifications
You must be signed in to change notification settings - Fork 141
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
Port BackendSampler and BackendEstimator from Qiskit 1.1 #1678
Conversation
0de4683
to
75ecd7e
Compare
Pull Request Test Coverage Report for Build 9089237108Details
💛 - 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.
Thanks @t-imamichi the improvement to performance will be nice.
We just need backend_sampler_v2.py and backend_estimator_v2.py.
I'm a bit confused. If we wait for qiskit 1.1, can't we use BackendSamplerV2 and BackendEstimatorV2 from there, and not have any such implementations in this repo?
I think BackendSamplerV2 and BackendEstimatorV2 were copied from qiskit to qiskit-ibm-runtime because they are not in qiskit 1.0 release yet (but they will be part of qiskit 1.1 release).
So, if we can update the requirement qiskit-ibm-runtime/requirements.txt Line 10 in 58722ee
|
1987b5f
to
d99fac2
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.
Thanks @t-imamichi. Let's then merge this PR and wait for #1690.
from qiskit.primitives.containers.pub_result import PubResult | ||
|
||
|
||
class SamplerPubResult(PubResult): |
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'm not confident that including this is worth it: it may cause isinstance confusion when people have one in qiskit and here.
However, this is not a blocker for me.
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 depends on how we control the compatibility with Qiskit 1.0.
If we still allow users to use Qiskit 1.0.*, we need SamplerPubResult
here. Yes, it would be confusing to Qiskit 1.1 users.
But if #1690 is merged, we can simply remove all backend primitives as well as SamplerPubResult.
I'm curious about the status of #1690
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.
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.
Yeah I can help @t-imamichi with that issue this sprint (based on what needs to be split out of #1700). Then we can cut a new release.
Superseded by #1700 |
Summary
(Update May 22)
If #1690 is merged, we don't need this PR anymore. We can remove all files related to backend primitives.
Port Qiskit/qiskit#12291
I copied
SamplerPubResult
so that Qiskit 1.0 works.If we will update the requirement with
qiskit>=1.1
after Qiskit 1.1 release, we don't have to keep them.We just need
backend_sampler_v2.py
andbackend_estimator_v2.py
.Fixes #1631
If #1690
Details and comments
microbenchmarking
main branch
this PR