-
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
Deprecating Provider ABC, as abstraction is not very useful #12145
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8802208992Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
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.
Overall this LGTM, I left a couple small comments on the deprecation message but otherwise the current content of this PR looks good.
The only thing missing is a release note documenting and explaining the reasoning behind the deprecation of the classes.
Co-authored-by: Matthew Treinish <[email protected]>
qiskit/providers/provider.py
Outdated
@deprecate_func( | ||
since=1.1, | ||
additional_msg="The abstract Provider and ProviderV1 classes are deprecated and will" | ||
" be removed in 2.0. You can just remove it as the parent class and a `get_backend` method that returns the backends from `self.backend`.", |
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.
technically, there is no need to remove get_backend
.
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.
Oh, I meant when you remove the parent class you should add get_backend
back manually. The one method you get for free from the abstract class is get_backend
so if people are dropping the abstract class as their parent they'll probably want to add that single method.
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.
oopss! I didnt read it properly, you are right.
`Aer.get_backend()` was deprecated as part of the `Provider` abstract class deprecation in Qiskit/qiskit#12145. This PR replaces it with instantiating `AerSimulator()` directly so the cron CI tests can pass again. --------- Co-authored-by: Will Shanks <[email protected]>
Summary
The abstraction
Provider
(andProviderV1
) does not give you much:name
,backends
, and/orget_backend
. That's it. And one of those (get_backend
) is debatable in name.A provider is a collection of backends. Backend is the actual thing that Qiskit cares about. Providers are not structures that Qiskit handles around (like it used to Qiskit/qiskit-ibm-runtime#419), so there is no need to prescribe a specific API. Different providers have, for example, different credential handling. So having a different way to fetch a backend looks like minor to me. If you think the discussion is controversial enough, I could open a short RFC.
Details and comments
see also: #5629