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 qft-plugins for custom 'qft' gates #13181

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

alexanderivrii
Copy link
Contributor

Summary

The HighLevelSynthesis transpiler pass no longer raises an exception when encountering a custom gate that is called "qft" but is not an instance of class QFTGate. Instead, the synthesis plugins for QFT gates ignore such a gate, and the gate's definition is used (if provided).

Fixes #13120 and #13174.

The PR is very short and has backport potential (if desired).

@alexanderivrii alexanderivrii requested review from ShellyGarion and a team as code owners September 18, 2024 14:04
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@coveralls
Copy link

Pull Request Test Coverage Report for Build 10923804686

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 9 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.5%) to 88.829%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 91.98%
crates/qasm2/src/parse.rs 6 97.61%
Totals Coverage Status
Change from base Build 10919208944: 0.5%
Covered Lines: 73483
Relevant Lines: 82724

💛 - Coveralls

@alexanderivrii alexanderivrii added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Sep 19, 2024
@alexanderivrii alexanderivrii added this to the 1.2.2 milestone Sep 19, 2024
@mtreinish mtreinish added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Sep 19, 2024
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.

Thanks for the fix, Sasha.

Comment on lines +2324 to +2326
# HighLevelSynthesis should replace the custom gate called "qft"
# by the user-provided definition.
self.assertEqual(Operator(qc2), Operator(qct))
Copy link
Member

Choose a reason for hiding this comment

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

This test confused me at first, because I was expecting the test to be that HighLevelSynthesis had not turned it into a QFT equivalent, but the trick is that qc2 doesn't actually encode a QFT, so the test does the right thing.

I'm fine to merge like this, just commenting more because I got a bit confused and felt like broadcasting that fact.

@jakelishman jakelishman added this pull request to the merge queue Sep 19, 2024
Merged via the queue into Qiskit:main with commit 2245a4f Sep 19, 2024
15 checks passed
mergify bot pushed a commit that referenced this pull request Sep 19, 2024
(cherry picked from commit 2245a4f)

# Conflicts:
#	qiskit/transpiler/passes/synthesis/hls_plugins.py
#	test/python/transpiler/test_high_level_synthesis.py
jakelishman added a commit that referenced this pull request Sep 19, 2024
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2024
(cherry picked from commit 2245a4f)

Co-authored-by: Jake Lishman <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2024
(cherry picked from commit 2245a4f)

Co-authored-by: Jake Lishman <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2024
(cherry picked from commit 2245a4f)

Co-authored-by: Jake Lishman <[email protected]>
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
Status: done
Development

Successfully merging this pull request may close these issues.

Defining a custom gate called qft in QASM apparently collides with qiskit QFT in v1.2
5 participants