Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Changes from all commits
5a85098
7bfc023
765fb0a
9db4a48
e9fd26f
215c637
ffae216
07f1d91
9fea202
7080cd5
4e56f1e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 thatBaseSampler
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?
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 ofrun
will be used. Am I correct?E.g.,
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.
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 existingpub.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.