-
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 BaseSamplerV2 #11529
Add BaseSamplerV2 #11529
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 7547884544
💛 - Coveralls |
f84a9c3
to
c87b081
Compare
@@ -200,3 +258,38 @@ def parameters(self) -> tuple[ParameterView, ...]: | |||
List of the parameters in each quantum circuit. | |||
""" | |||
return tuple(self._parameters) | |||
|
|||
|
|||
BaseSampler = BaseSamplerV1 |
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.
@jakelishman Is this the sort of thing we should have a from __future__ import <what?>
so that BaseSampler
gets set to BaseSamplerV2 instead? And that if you import BaseSampler without this import it raises a deprecation warning of some sort using the same trick you used in the other PRs and returns BaseSamplerV1 for it in those cases?
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 guess for 1.0 we might want a future deprecation warning rather than actual deprecation warning for the V1 primitives, and then the actual warning on 1.1?
Moved shots to |
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, lookin' good.
Args: | ||
pubs: An iterable of pub-like objects. For example, a list of circuits | ||
or tuples ``(circuit, parameter_values)``. | ||
shots: The total number of shots to sample for each :class:`.SamplerPub`. |
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.
If SamplerV2 has an execution option and the number of shots of the execution option and that of run
method differ, I guess that of run
will be used. Am I correct?
E.g.,
sampler = SamplerV2(options={"execution": {"shots": 10}})
result = sampler.run([circuit], shots=20).result()
# `result` contains 20 samples instead of 10 samples.
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.
Yes, in this situation the kwargs of run takes precedence over execution.shots. The latter should be thought of as the default value when run gets None.
raise ValueError("shots must be non-negative") | ||
|
||
if isinstance(pub, SamplerPub): | ||
if pub.shots is None and shots is not None: |
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 guess shots
of this method is used as a default value not to overwrite the existing pub.shots
.
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 right, pub.shots takes precedence.
Taken from 11264. Removes Options and adds `shots` attributes to BaseSamplerV2 Co-Authored-By: Takashi Imamichi <[email protected]> Co-Authored-By: Ian Hincks <[email protected]>
Co-authored-by: Ian Hincks <[email protected]>
Co-authored-by: Ian Hincks <[email protected]>
fcd0916
to
4e56f1e
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.
LGTM
Summary
Adds BaseSaamplerV2 base class for V2 Sampler primitives
Depends on #11524
Details and comments
This splits out the base classes in from #11264 and makes the following changes
shots
attributesrun
kwarg to BaseSamplerV2shots
toSamplerPub
See Add BaseEstimatorV2 #11527 for motivation for these changes.