Skip to content
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 BackendSampler for Sampler from an arbitrary backend #8668

Merged
merged 16 commits into from
Sep 30, 2022

Conversation

mtreinish
Copy link
Member

Summary

This commit adds a new Sampler implementation to qiskit-terra,
BackendSampler. This sampler implementation enables a sampler object
to be constructed from any backend object. It can be used to enable
backends from providers that don't have native primitive implementations
(which is most providers) to leverage tooling built on primtives. The
API works exactly the same as other sampler implementations except it
takes a required argument on the constructor backend which is used
to specify the Backend object to execute circuits on.

Details and comments

Co-authored-by: Takashi Imamichi 31178928+t-imamichi@users.noreply.github.com
Co-authored-by: Ikko Hamamura ikkoham@users.noreply.github.com

This commit adds a new Sampler implementation to qiskit-terra,
BackendSampler. This sampler implementation enables a sampler object
to be constructed from any backend object. It can be used to enable
backends from providers that don't have native primitive implementations
(which is most providers) to leverage tooling built on primtives. The
API works exactly the same as other sampler implementations except it
takes a required argument on the constructor `backend` which is used
to specify the `Backend` object to execute circuits on.

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

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

@mtreinish mtreinish added the mod: primitives Related to the Primitives module label Sep 2, 2022
@mtreinish mtreinish added this to the 0.22 milestone Sep 2, 2022
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't read it super thoroughly as I'm not the right person to review primitives code, but I had a quick look since the people who I think of as being the right people all co-authored this PR.

I support the idea of the class a lot - I think it helps ease transitions a lot if there's a "new-thing-from-existing-thing" converter available immediately.

qiskit/primitives/backend_sampler.py Outdated Show resolved Hide resolved
qiskit/primitives/backend_sampler.py Outdated Show resolved Hide resolved
test/python/primitives/test_backend_sampler.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Sep 2, 2022

Pull Request Test Coverage Report for Build 3155586842

  • 80 of 83 (96.39%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 84.638%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/primitives/backend_sampler.py 79 82 96.34%
Files with Coverage Reduction New Missed Lines %
qiskit/extensions/quantum_initializer/squ.py 2 79.78%
Totals Coverage Status
Change from base Build 3155580191: 0.2%
Covered Lines: 61388
Relevant Lines: 72530

💛 - Coveralls

@nonhermitian
Copy link
Contributor

nonhermitian commented Sep 13, 2022

I am not sure I follow the logic here. There is a BaseSampler that targets a statevector simulator, which is not really a sampling routine at all, and because of that we need this BackendSampler to do the actual practical work?

Also this taking a Backend instance seems, to me at least, to be going down the dreaded QuantumInstance path. Why can't a Backend instance be able to define its own sampler call like backend.run() of today, e.g. backend.sample() ?

@mtreinish
Copy link
Member Author

I am not sure I follow the logic here. There is a BaseSampler that targets a statevector simulator, which is not really a sampling routine at all, and because of that we need this BackendSampler to do the actual practical work?

For me it's a more pragmatic reason, there is a lot of work people are doing right now to refactor all the algorithms in qiskit.algorithms to work with Sampler and Estimator objects instead of using backends and the QuantumInstance around them. However, as you point out there is no implementation for an arbitrary backend, which basically makes a world where algorithms are implemented solely with primitive objects is one that prevents most backend objects, except for qiskit-ibm-runtime and qiskit-aer, are unable to be used to run algorithms. I pushed this so we have something to bridge the gap and enable people to continue using functionality that has worked in the past. To be able to drop the QuantumInstance and all its usage we need a way to get the primitive object from a Backend to cover the functionality gap in algorithms.

Also this taking a Backend instance seems, to me at least, to be going down the dreaded QuantumInstance path. Why can't a Backend instance be able to define its own sampler call like backend.run() of today, e.g. backend.sample() ?

I agree, I would have preferred if these primitive operations were more tightly integrated with the backend interface too (we also could provide an overridable default implementation as a method very easily). It would have eased the transition and I think would have been a nicer UI for people to work with. But, the interface as defined and starting to be used elsewhere is the standalone class where you pass it a circuit and parameters and then it returns probabilities. I'm quite wary of the scope creep and making things like a new QuantumInstance I'm hopeful that by using the defined semantics of the backend interface and not providing any levers outside of that and what the abstract sampler interface provides will mean this doesn't turn into that.

@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Sep 13, 2022
qiskit/primitives/backend_sampler.py Outdated Show resolved Hide resolved
qiskit/primitives/backend_sampler.py Outdated Show resolved Hide resolved
qiskit/primitives/backend_sampler.py Outdated Show resolved Hide resolved
test/python/primitives/test_backend_sampler.py Outdated Show resolved Hide resolved
Copy link
Member

@levbishop levbishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine

@mergify mergify bot merged commit 6f84c70 into Qiskit:main Sep 30, 2022
@mtreinish mtreinish deleted the backend-sampler branch September 30, 2022 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: primitives Related to the Primitives module priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants