-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Deprecate BasicSimulator.configuration
#13367
Conversation
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 11594412911Warning: 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.
This is a fun cycle of deprecations getting emitted between assemble and the backend. Personally I'm not sure the filterwarnings inline in code because that can hide things when we need to rely on it later. But in this case I think it's fine especially as all of this functionality will be removed at a high level in 2.0.
The only comment I really had was about maybe adding a link to the backend version migration guide in the release note. Although it is also maybe a question of whether we need to improve the docs in the migration guide too? (like is it missing something). But if you're happy with the release note or want to fix them in a follow up, potentially post rc1 that's fine and feel free to enqueue this for merging.
# Note for future: this method is used in `BasicSimulator` and may need to be kept past the | ||
# `assemble` removal deadline (2.0). If it is kept (potentially in a different location), | ||
# we will need an alternative for the backend.configuration() access that currently takes | ||
# place in L566 (`parse_circuit_args`) and L351 (`parse_common_args`) | ||
# because backend.configuration() is also set for removal in 2.0. | ||
# The ultimate goal will be to move away from relying on any kind of `assemble` implementation | ||
# because of how tightly coupled it is to these legacy data structures. But as a transition step, | ||
# given that we would only have to support the subcase of `BasicSimulator`, we could probably just | ||
# inline the relevant config values that are already hardcoded in the basic simulator configuration | ||
# generator. |
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.
FWIW, it should not be hard to rewrite the run_experiment
method of basic simulator to avoid needing qobj. The assemble usage in there is kind of a weird legacy from BaseBackend
which required a qobj get passed to the run()
method.
Here is a quick guide for accessing the most common ``BackendConfiguration`` attributes in the | ||
:class:`BackendV2` model:"" | ||
|
||
BackendV1 model (deprecated) ------------> BackendV2 model | ||
---------------------------- --------------- | ||
backend.configuration().backend_name backend.name | ||
backend.configuration().backend_version backend.backend_version | ||
backend.configuration().n_qubits backend.num_qubits | ||
backend.configuration().num_qubits backend.num_qubits | ||
backend.configuration().basis_gates backend.target.operation_names (*) | ||
backend.configuration().coupling_map backend.target.build_coupling_map() | ||
backend.configuration().local No representation | ||
backend.configuration().simulator No representation | ||
backend.configuration().conditional No representation | ||
backend.configuration().open_pulse No representation | ||
backend.configuration().memory No representation | ||
backend.configuration().max_shots No representation | ||
|
||
(*) Note that ``backend.target.operation_names`` includes ``basis_gates`` and additional | ||
non-gate instructions, in some implementations it might be necessary to filter the output. |
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.
Is it worth just linking to https://github.com/Qiskit/qiskit/blob/main/qiskit/providers/__init__.py#L679 here too?
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.
There isn't a lot of extra information but I added it in case the migration guide gets improvements (I know there's an open issue for that).
Summary
Closes #12843.
Details and comments