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

Fix error in Target non-global operation method with ideal gates #7554

Merged
merged 4 commits into from
Jan 24, 2022

Conversation

mtreinish
Copy link
Member

Summary

This commit fixes an issue in the Target
get_non_global_operation_names() method. Previously this method would
crash when you had a target with it's instruction properties set to
None (which is typically used for an ideal simulator).

Details and comments

This commit fixes an issue in the Target
get_non_global_operation_names() method. Previously this method would
crash when you had a target with it's instruction properties set to
None (which is typically used for an ideal simulator).
@mtreinish mtreinish requested a review from a team as a code owner January 21, 2022 21:59
@mtreinish mtreinish added Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable labels Jan 21, 2022
@mtreinish mtreinish added this to the 0.19.2 milestone Jan 21, 2022
@coveralls
Copy link

coveralls commented Jan 21, 2022

Pull Request Test Coverage Report for Build 1740055295

  • 8 of 8 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 83.169%

Totals Coverage Status
Change from base Build 1733142703: 0.03%
Covered Lines: 52024
Relevant Lines: 62552

💛 - Coveralls

This commit fixes an issue when running transpile() with a BackendV2
based backend that has ideal operation defined in it's target. A Target
that has an operation with its qargs set as None means it's ideally
defined on all qubits (or all combinations of qubits for >1 qubit
operations). However, the pre-processing of the target in the
transpile() method was not taking this into account and would fail if
a backend with this set was used.
In the previous commit there were some issues in the tweaks made to the
transpile() function to parse the timing_constraints argument. The basic
flow we wanted for the standalone target case is to create a
TimingConstraints object directly from that Target. However, the
internal function was expect only a dict was passed in and tried to
create a TimingConstraints object from it. The previous commit tried to
address this but neglected the standard use with a dict. This commit
corrects this oversight and tweaks the logic to handle a predefined
TimingConstraint object first and then fallsback to the other behavior.
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.

This all looks sensible to me. Good to plug the gaps in testing that we must have let slip through the first time.

Comment on lines 545 to +549
if target is not None:
if coupling_map is None:
coupling_map = target.coupling_map()
coupling_map = target.build_coupling_map()
if basis_gates is None:
basis_gates = target.operation_names()
basis_gates = target.operation_names
Copy link
Member

Choose a reason for hiding this comment

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

If there were type hints in this function, pylint would have caught this ;)

@mergify mergify bot merged commit fe5acd4 into Qiskit:main Jan 24, 2022
mergify bot pushed a commit that referenced this pull request Jan 24, 2022
* Fix error in Target non-global operation method with ideal gates

This commit fixes an issue in the Target
get_non_global_operation_names() method. Previously this method would
crash when you had a target with it's instruction properties set to
None (which is typically used for an ideal simulator).

* Fix issue with transpile() on BackendV2 with ideal operation

This commit fixes an issue when running transpile() with a BackendV2
based backend that has ideal operation defined in it's target. A Target
that has an operation with its qargs set as None means it's ideally
defined on all qubits (or all combinations of qubits for >1 qubit
operations). However, the pre-processing of the target in the
transpile() method was not taking this into account and would fail if
a backend with this set was used.

* Tweak timing constraints parsing in transpile()

In the previous commit there were some issues in the tweaks made to the
transpile() function to parse the timing_constraints argument. The basic
flow we wanted for the standalone target case is to create a
TimingConstraints object directly from that Target. However, the
internal function was expect only a dict was passed in and tried to
create a TimingConstraints object from it. The previous commit tried to
address this but neglected the standard use with a dict. This commit
corrects this oversight and tweaks the logic to handle a predefined
TimingConstraint object first and then fallsback to the other behavior.

(cherry picked from commit fe5acd4)
@mtreinish mtreinish deleted the fix-calculation-ideal-backend branch January 24, 2022 14:38
mergify bot added a commit that referenced this pull request Jan 24, 2022
…) (#7561)

* Fix error in Target non-global operation method with ideal gates

This commit fixes an issue in the Target
get_non_global_operation_names() method. Previously this method would
crash when you had a target with it's instruction properties set to
None (which is typically used for an ideal simulator).

* Fix issue with transpile() on BackendV2 with ideal operation

This commit fixes an issue when running transpile() with a BackendV2
based backend that has ideal operation defined in it's target. A Target
that has an operation with its qargs set as None means it's ideally
defined on all qubits (or all combinations of qubits for >1 qubit
operations). However, the pre-processing of the target in the
transpile() method was not taking this into account and would fail if
a backend with this set was used.

* Tweak timing constraints parsing in transpile()

In the previous commit there were some issues in the tweaks made to the
transpile() function to parse the timing_constraints argument. The basic
flow we wanted for the standalone target case is to create a
TimingConstraints object directly from that Target. However, the
internal function was expect only a dict was passed in and tried to
create a TimingConstraints object from it. The previous commit tried to
address this but neglected the standard use with a dict. This commit
corrects this oversight and tweaks the logic to handle a predefined
TimingConstraint object first and then fallsback to the other behavior.

(cherry picked from commit fe5acd4)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants