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 InverseCancellation pass to handle gates of different names. #7383

Conversation

kdk
Copy link
Member

@kdk kdk commented Dec 8, 2021

As reported in GH-7378, the InverseCancellation optimization pass
previously assumed that inverse gate pairs would always share a
common name, but this is not universally true. This commit updates
the pass to search for runs containing any of the gate names in the
gates_to_cancel list, instead of only the first.

Co-authored-by: Bruno Schmitt bruno.schmitt@epfl.ch

Summary

Details and comments

As reported in QiskitGH-7378, the InverseCancellation optimization pass
previously assumed that inverse gate pairs would always share a
common name, but this is not universally true. This commit updates
the pass to search for runs containing any of the gate names in the
gates_to_cancel list, instead of only the first.

Co-authored-by: Bruno Schmitt <bruno.schmitt@epfl.ch>
@kdk kdk added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Dec 8, 2021
@kdk kdk added this to the 0.19.1 milestone Dec 8, 2021
@kdk kdk requested a review from a team as a code owner December 8, 2021 22:28
@kdk kdk linked an issue Dec 8, 2021 that may be closed by this pull request
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

This LGTM, this was clearly a bug assuming the names were equal for inverse pairs so this is the simple fix that we can easily backport for 0.19.1. But, I also agree with @boschmitt in #7378 having a way to transform things into a canonical form is probably a good/better way to handle this longer term as part of 0.20.0.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1556245160

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.128%

Totals Coverage Status
Change from base Build 1556105026: 0.0%
Covered Lines: 51791
Relevant Lines: 62303

💛 - Coveralls

@mergify mergify bot merged commit d9ab98f into Qiskit:main Dec 8, 2021
mergify bot pushed a commit that referenced this pull request Dec 8, 2021
As reported in GH-7378, the InverseCancellation optimization pass
previously assumed that inverse gate pairs would always share a
common name, but this is not universally true. This commit updates
the pass to search for runs containing any of the gate names in the
gates_to_cancel list, instead of only the first.

Co-authored-by: Bruno Schmitt <bruno.schmitt@epfl.ch>

Co-authored-by: Bruno Schmitt <bruno.schmitt@epfl.ch>
(cherry picked from commit d9ab98f)
mergify bot added a commit that referenced this pull request Dec 9, 2021
…) (#7384)

As reported in GH-7378, the InverseCancellation optimization pass
previously assumed that inverse gate pairs would always share a
common name, but this is not universally true. This commit updates
the pass to search for runs containing any of the gate names in the
gates_to_cancel list, instead of only the first.

Co-authored-by: Bruno Schmitt <bruno.schmitt@epfl.ch>

Co-authored-by: Bruno Schmitt <bruno.schmitt@epfl.ch>
(cherry picked from commit d9ab98f)

Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.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.

InverseCancellation cannot deal with inverse gates with different names
3 participants