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 BackendV2Converter class for treating BackendV1 as BackendV2 #8759

Merged
merged 25 commits into from
Sep 28, 2022

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Sep 14, 2022

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 kevin.tian@ibm.com
Co-authored-by: Rathish Cholarajan rathishc24@gmail.com

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
@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Sep 14, 2022
@mtreinish mtreinish added this to the 0.22 milestone Sep 14, 2022
@mtreinish mtreinish requested review from a team and jyu00 as code owners September 14, 2022 20:49
@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:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Sep 14, 2022

Pull Request Test Coverage Report for Build 3141055024

  • 126 of 157 (80.25%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.0009%) to 84.466%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/providers/backend_compat.py 116 147 78.91%
Totals Coverage Status
Change from base Build 3139567056: -0.0009%
Covered Lines: 60079
Relevant Lines: 71128

💛 - Coveralls

qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
"""Uses BackendProperties to construct
and return a list of IBMQubitProperties.
"""
qubit_props: List[QubitProperties] = []
Copy link
Member

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?

qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
except BackendPropertyError:
t_2 = None
qubit_props.append(
QubitProperties( # type: ignore[no-untyped-call]
Copy link
Member

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.

Copy link
Member Author

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.

qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
@mtreinish mtreinish changed the title Add BackendV2Convert class for treating BackendV1 as BackendV2 Add BackendV2Converter class for treating BackendV1 as BackendV2 Sep 14, 2022
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 left a 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

qiskit/providers/backend_compat.py Show resolved Hide resolved
qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Show resolved Hide resolved
qiskit/providers/backend_compat.py Show resolved Hide resolved
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:
Copy link
Contributor

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.

qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
@mtreinish
Copy link
Member Author

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.

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.

* Can we also address missing frequency parameters in #8712 ?

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 BackendV2/Target this converter class should be updated to handle that.

qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/circuit/library/standard_gates/__init__.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Show resolved Hide resolved
test/python/providers/test_fake_backends.py Show resolved Hide resolved
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 left a 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.

qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
qiskit/providers/backend_compat.py Show resolved Hide resolved
qiskit/providers/backend_compat.py Outdated Show resolved Hide resolved
mtreinish and others added 6 commits September 27, 2022 04:15
Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
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.
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 left a 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.

@mergify mergify bot merged commit 4a857f6 into Qiskit:main Sep 28, 2022
@mtreinish mtreinish deleted the v1-to-v2-compat branch September 28, 2022 07:10
ElePT pushed a commit to ElePT/qiskit-ibm-provider that referenced this pull request Oct 9, 2023
…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 <jake.lishman@ibm.com>

* Fix lint

* Fix pylint cyclic-import error

* Update qiskit/providers/backend_compat.py

Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>

* 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 <jake.lishman@ibm.com>
Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility classes for converting BackendV1 to BackendV2 to facilitate migration
5 participants