-
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 BackendV2Converter class for treating BackendV1 as BackendV2 #8759
Conversation
This commit adds a new class BackendV2Converter which is a BackendV2 implementation that converts an input BackendV1 object into a BackendV2 implementation. This is useful for users that are supporting working with arbitrary providers so that they can standardize on using the newest access patterns even if a provider is still using BackendV1. Similarly, for qiskit's internal usage, this gives us a path to move all of the transpiler internals to use Target and avoid carrying around duplicate information in the PassManagerConfig for passes that haven't been updated. This will enable us to convert input BackendV1 instances to a target once on initial calling and have the transpiler only ever see a target. Fixes Qiskit#8611
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:
|
Pull Request Test Coverage Report for Build 3141055024
💛 - Coveralls |
"""Uses BackendProperties to construct | ||
and return a list of IBMQubitProperties. | ||
""" | ||
qubit_props: List[QubitProperties] = [] |
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.
Annotating intermediate values with type hints? Who are you, and what have you done with Matthew?
except BackendPropertyError: | ||
t_2 = None | ||
qubit_props.append( | ||
QubitProperties( # type: ignore[no-untyped-call] |
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.
Ok, at this point there's no way you're the only author of this.
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.
Heh, yeah I copied this code from the qiskit-ibm-provider
which is based on an initial draft I wrote https://github.com/Qiskit/qiskit-ibm-provider/blob/main/qiskit_ibm_provider/utils/backend_converter.py and massaged a lot to pass mypy (which is run in the provider CI). But thanks for calling that out, I should add @rathishcholarajan and @kt474 to the co-authors list on this PR too since they did a bunch of work on the converter 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.
Overall I really like this idea. I added several comments. My main questions are
- What would be the best place to host this code? Seems like V1 to V2 conversion is provider-specific.
- Can we also address missing frequency parameters in Add a BackendV2 version of FakeOpenPulse2Q #8712 ?
for inst in inst_map.instructions: | ||
for qarg in inst_map.qubits_with_instruction(inst): | ||
sched = inst_map.get(inst, qarg) | ||
if inst in target: |
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.
Alternatively, you can loop over target gates and check inst_map.has(...)
and then add calibration.
To do it fully and most effectively yes it is provider specific thing. Ideally every provider would handle this on their own, but at this point not every provider has migrated to BackendV2 so having this generic converter provides some benefits even if the transform can't handle all the nuances of provider doing itself.
I thought we were talking about expanding the BackendV2 class to support that. I think when we have a defined place for the frequency parameters in |
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 Matthew, this PR almost looks good to me. If there is a good reason to include supported_instructions
I'll approve the PR, otherwise I think it is okey to remove this to simplify the logic there.
Co-authored-by: Naoki Kanazawa <[email protected]>
The supported instructions field is underdocumented and it's not clear that the instructions it contains are something that we want to support directly. Since the only reason we added support for it in the target generation was to handle the delay for ibm backends (mostly in tests as the IBM provider will be using BackendV2 natively soon) this commit removes its usage and adds a new flag for explicitly adding delay to the backend's target.
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 Matthew this looks good to me.
…kit/qiskit#8759) * Add BackendV2Convert class for treating BackendV1 as BackendV2 This commit adds a new class BackendV2Converter which is a BackendV2 implementation that converts an input BackendV1 object into a BackendV2 implementation. This is useful for users that are supporting working with arbitrary providers so that they can standardize on using the newest access patterns even if a provider is still using BackendV1. Similarly, for qiskit's internal usage, this gives us a path to move all of the transpiler internals to use Target and avoid carrying around duplicate information in the PassManagerConfig for passes that haven't been updated. This will enable us to convert input BackendV1 instances to a target once on initial calling and have the transpiler only ever see a target. Fixes Qiskit/qiskit#8611 * Update docstring * Return empty options for _default_options * Remove leftover pylint disable * Expand standard gate conversions and handle missing basis_gates * Fix copy paste error qubit_props_list_from_props() docstring * Add name mapping argument to allow setting custom name-> gate mappings for conversion * Add missing gamma parameter * Fix handling of global ops in configuration * Raise exception on custom gates without mapping * Move name mapping to standard gates module * Fix lint and docs * Use gate object name attribute to build name mapping Co-authored-by: Jake Lishman <[email protected]> * Fix lint * Fix pylint cyclic-import error * Update qiskit/providers/backend_compat.py Co-authored-by: Naoki Kanazawa <[email protected]> * Remove supported_instructions and add option for adding delay The supported instructions field is underdocumented and it's not clear that the instructions it contains are something that we want to support directly. Since the only reason we added support for it in the target generation was to handle the delay for ibm backends (mostly in tests as the IBM provider will be using BackendV2 natively soon) this commit removes its usage and adds a new flag for explicitly adding delay to the backend's target. * Add mention of converter class to API changes section * Add missing flag usage to delay test * Run black Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Naoki Kanazawa <[email protected]>
Summary
This commit adds a new class BackendV2Converter which is a BackendV2 implementation that converts an input BackendV1 object into a BackendV2 implementation. This is useful for users that are supporting working with arbitrary providers so that they can standardize on using the newest access patterns even if a provider is still using BackendV1. Similarly, for qiskit's internal usage, this gives us a path to move all of the transpiler internals to use Target and avoid carrying around duplicate information in the PassManagerConfig for passes that haven't been updated. This will enable us to convert input BackendV1 instances to a target once on initial calling and have the transpiler only ever see a target.
Details and comments
Fixes #8611
Co-authored-by: Kevin Tian [email protected]
Co-authored-by: Rathish Cholarajan [email protected]