Add validation to Sampler if there are no measurements #10642

ikkoham · 2023-08-16T06:10:18Z

Summary

This PR resolves TODO comment in BaseSampler.

Details and comments

qiskit-bot · 2023-08-16T06:10:22Z

One or more of the the following people are requested to review this:

@Qiskit/terra-core
@ajavadia
@ikkoham
@levbishop
@t-imamichi

Cryoris

That's a good check to have in place 👍🏻 Could you add a short releasenote for this, too?

qiskit/primitives/base/base_sampler.py

ikkoham · 2023-08-16T06:56:16Z

Yes. I'll add a release note.

coveralls · 2023-08-16T06:57:07Z

Pull Request Test Coverage Report for Build 5921033642

12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
8 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.03%) to 87.274%

Files with Coverage Reduction	New Missed Lines	%
qiskit/primitives/sampler.py	1	95.65%
qiskit/pulse/library/waveform.py	3	93.75%
crates/qasm2/src/lex.rs	4	91.14%

Totals
Change from base Build 5902014500:	0.03%
Covered Lines:	74376
Relevant Lines:	85221

💛 - Coveralls

Co-authored-by: Julien Gacon <gaconju@gmail.com>

Cryoris · 2023-08-18T09:56:37Z

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?

ikkoham · 2023-08-21T01:49:24Z

@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.
First, there were inappropriate error messages QiskitError: No counts for experiment \"0\" in qiskit-ibm-runtime. This PR improves a UX. Also, from another perspective, it should be meant to fix a performance bug, since errors before and after quantum computation execution consume very different amounts of resources.

ikkoham · 2023-08-21T05:38:35Z

This PR requires primitives team review. @t-imamichi or Mariana

t-imamichi · 2023-08-21T10:06:03Z

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?

t-imamichi · 2023-08-21T12:59:39Z

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 BackendSampler, Aer Sampler, and Runtime Sampler, raise an error when invoking job.result() as follows. This PR applies a check of measurement at run method to raise an error before submitting a job.

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)

Traceback (most recent call last):
  File "/Users/ima/tasks/3_2023/qiskit/terra/tmp/sampler.py", line 22, in <module>
    result = job.result()
  File "/Users/ima/tasks/3_2023/qiskit/terra/qiskit/primitives/primitive_job.py", line 55, in result
    return self._future.result()
  File "/opt/homebrew/Cellar/python@3.10/3.10.12_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/concurrent/futures/_base.py", line 458, in result
    return self.__get_result()
  File "/opt/homebrew/Cellar/python@3.10/3.10.12_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/concurrent/futures/_base.py", line 403, in __get_result
    raise self._exception
  File "/opt/homebrew/Cellar/python@3.10/3.10.12_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/Users/ima/tasks/3_2023/qiskit/terra/qiskit/primitives/backend_sampler.py", line 149, in _call
    return self._postprocessing(result, bound_circuits)
  File "/Users/ima/tasks/3_2023/qiskit/terra/qiskit/primitives/backend_sampler.py", line 154, in _postprocessing
    counts = _prepare_counts(result)
  File "/Users/ima/tasks/3_2023/qiskit/terra/qiskit/primitives/backend_estimator.py", line 79, in _prepare_counts
    count = res.get_counts()
  File "/Users/ima/tasks/3_2023/qiskit/terra/qiskit/result/result.py", line 289, in get_counts
    raise QiskitError(f'No counts for experiment "{repr(key)}"')
qiskit.exceptions.QiskitError: 'No counts for experiment "0"'

output with this PR

Traceback (most recent call last):
  File "/Users/ima/tasks/3_2023/qiskit/terra/tmp/sampler.py", line 21, in <module>
    job = sampler.run(qc, shots=100)
  File "/Users/ima/tasks/3_2023/qiskit/terra/qiskit/primitives/base/base_sampler.py", line 134, in run
    circuits = self._validate_circuits(circuits)
  File "/Users/ima/tasks/3_2023/qiskit/terra/qiskit/primitives/base/base_sampler.py", line 176, in _validate_circuits
    raise ValueError(
ValueError: The 0-th circuit does not have Measure instruction. Without measurements, the circuit cannot be sampled from.

t-imamichi · 2023-08-21T13:10:45Z

LGTM. But, let's wait for Mariana's review too.

qiskit/primitives/base/base_sampler.py

coveralls · 2023-08-28T05:29:53Z

Pull Request Test Coverage Report for Build 5995648228

12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
35 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.04%) to 87.245%

Files with Coverage Reduction	New Missed Lines	%
qiskit/primitives/sampler.py	1	95.65%
crates/qasm2/src/lex.rs	4	91.14%
crates/qasm2/src/parse.rs	30	95.74%

Totals
Change from base Build 5985235181:	-0.04%
Covered Lines:	74265
Relevant Lines:	85122

💛 - 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>

1ucian0 · 2023-08-29T11:48:06Z

i just saw the email of this closing. Is this PR related to #10706 ?

t-imamichi · 2023-08-30T06:01:22Z

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>

add validation

55b513a

ikkoham added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: None Do not include in changelog mod: primitives Related to the Primitives module labels Aug 16, 2023

ikkoham requested review from t-imamichi and a team as code owners August 16, 2023 06:10

Cryoris reviewed Aug 16, 2023

View reviewed changes

qiskit/primitives/base/base_sampler.py Outdated Show resolved Hide resolved

qiskit/primitives/base/base_sampler.py Show resolved Hide resolved

ikkoham and others added 3 commits August 16, 2023 15:57

Update qiskit/primitives/base/base_sampler.py

de9c68b

Co-authored-by: Julien Gacon <gaconju@gmail.com>

add reno

20f6cc2

add test

f390825

ikkoham requested a review from Cryoris August 16, 2023 13:17

ikkoham added 2 commits August 17, 2023 12:49

Merge branch 'main' into primitives/sampler-with-no-measure

74700c6

Merge branch 'main' into primitives/sampler-with-no-measure

1e370e8

ikkoham added Changelog: Bugfix Include in the "Fixed" section of the changelog and removed Changelog: None Do not include in changelog labels Aug 18, 2023

Merge branch 'main' into primitives/sampler-with-no-measure

a6a8e78

t-imamichi previously approved these changes Aug 21, 2023

View reviewed changes

Merge branch 'main' into primitives/sampler-with-no-measure

40a495c

chriseclectic reviewed Aug 22, 2023

View reviewed changes

qiskit/primitives/base/base_sampler.py Outdated Show resolved Hide resolved

reversed iterate

a22b14c

ikkoham dismissed t-imamichi’s stale review via a22b14c August 23, 2023 08:24

t-imamichi approved these changes Aug 24, 2023

View reviewed changes

Merge branch 'main' into primitives/sampler-with-no-measure

15114c4

t-imamichi added this pull request to the merge queue Aug 29, 2023

Merged via the queue into Qiskit:main with commit 3bd1d46 Aug 29, 2023

mergify bot mentioned this pull request Aug 29, 2023

Add validation to Sampler if there are no measurements (backport #10642) #10722

Merged

ikkoham mentioned this pull request Aug 30, 2023

Sampler measurement assumptions #10706

Open

ikkoham deleted the primitives/sampler-with-no-measure branch September 21, 2023 02:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add validation to Sampler if there are no measurements #10642

Add validation to Sampler if there are no measurements #10642

Uh oh!

ikkoham commented Aug 16, 2023

Uh oh!

qiskit-bot commented Aug 16, 2023

Uh oh!

Cryoris left a comment

Uh oh!

Uh oh!

Uh oh!

ikkoham commented Aug 16, 2023

Uh oh!

coveralls commented Aug 16, 2023 •

edited

Loading

Uh oh!

Cryoris commented Aug 18, 2023

Uh oh!

ikkoham commented Aug 21, 2023

Uh oh!

ikkoham commented Aug 21, 2023

Uh oh!

t-imamichi commented Aug 21, 2023

Uh oh!

t-imamichi commented Aug 21, 2023 •

edited

Loading

Uh oh!

t-imamichi commented Aug 21, 2023

Uh oh!

Uh oh!

coveralls commented Aug 28, 2023

Uh oh!

1ucian0 commented Aug 29, 2023

Uh oh!

t-imamichi commented Aug 30, 2023

Uh oh!

Uh oh!

Add validation to Sampler if there are no measurements #10642

Add validation to Sampler if there are no measurements #10642

Uh oh!

Conversation

ikkoham commented Aug 16, 2023

Summary

Details and comments

Uh oh!

qiskit-bot commented Aug 16, 2023

Uh oh!

Cryoris left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ikkoham commented Aug 16, 2023

Uh oh!

coveralls commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 5921033642

💛 - Coveralls

Uh oh!

Cryoris commented Aug 18, 2023

Uh oh!

ikkoham commented Aug 21, 2023

Uh oh!

ikkoham commented Aug 21, 2023

Uh oh!

t-imamichi commented Aug 21, 2023

Uh oh!

t-imamichi commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-imamichi commented Aug 21, 2023

Uh oh!

Uh oh!

coveralls commented Aug 28, 2023

Pull Request Test Coverage Report for Build 5995648228

💛 - Coveralls

Uh oh!

1ucian0 commented Aug 29, 2023

Uh oh!

t-imamichi commented Aug 30, 2023

Uh oh!

Uh oh!

coveralls commented Aug 16, 2023 •

edited

Loading

t-imamichi commented Aug 21, 2023 •

edited

Loading