-
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
Add validation to Sampler if there are no measurements #10642
Add validation to Sampler if there are no measurements #10642
Conversation
One or more of the the following people are requested to review this:
|
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.
That's a good check to have in place 👍🏻 Could you add a short releasenote for this, too?
Yes. I'll add a release note. |
Pull Request Test Coverage Report for Build 5921033642
💛 - Coveralls |
LGTM, but I'm not sure it qualifies a bugfix and backport as this PR is technically an additional safety measure and not fixing a broken behavior. Is this something we need for 0.25.2 or is the next feature release enough? |
@Cryoris Thank. Yes, you're right. This is not a feature bug fix, but I believe this is a error message bug or performance bug fix. |
This PR requires primitives team review. @t-imamichi or Mariana |
I investigated the history of circuit validation. I relaxed one in BaseSampler to support dynamic circuits (see #8708). Does this PR resolve the issue, perhaps? |
Details I used to add a check of measurements #8678, but it was removed for mid-circuit measurement support in the future #8708. As a result, some Sampler implementations such as from qiskit import QuantumCircuit
from qiskit.primitives import Sampler, BackendSampler
from qiskit_aer import AerSimulator
from qiskit_aer.primitives import Sampler as AerSampler
from qiskit_ibm_runtime import Sampler as RuntimeSampler, QiskitRuntimeService, Session
qc = QuantumCircuit(1, 1)
qc.h(0)
if True:
sampler = BackendSampler(backend=AerSimulator())
elif False:
sampler = AerSampler()
else:
service = QiskitRuntimeService(instance="ibm-q/open/main")
backend = "ibmq_qasm_simulator"
session = Session(service=service, backend=backend)
sampler = RuntimeSampler(session=session)
job = sampler.run(qc, shots=100)
result = job.result() # raise an error at result method
print(result) output (main branch)
output with this PR
|
LGTM. But, let's wait for Mariana's review too. |
Pull Request Test Coverage Report for Build 5995648228
💛 - Coveralls |
* add validation * Update qiskit/primitives/base/base_sampler.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * add reno * add test * reversed iterate --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com> (cherry picked from commit 3bd1d46)
* add validation * Update qiskit/primitives/base/base_sampler.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * add reno * add test * reversed iterate --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com> (cherry picked from commit 3bd1d46) Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
i just saw the email of this closing. Is this PR related to #10706 ? |
No. This PR addresses a different issue. |
* add validation * Update qiskit/primitives/base/base_sampler.py Co-authored-by: Julien Gacon <gaconju@gmail.com> * add reno * add test * reversed iterate --------- Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Summary
This PR resolves TODO comment in BaseSampler.
Details and comments