fix issue/12311 with fractional gate in basis #12511

ewinston · 2024-06-05T09:33:22Z

Summary

This fixes an issue where an exception would be raised if transpiling a unitary gate to a backend with an RZZGate in its basis however transpiling with only basis_gates specified would transpile fine.

fixes #12311

Details and comments

In this case, where RZZ is the only entangling gate in the basis, decomposition of a 2q unitary gate wouldn't get synthesized in the UnitarySynthesis pass but would instead be synthesized in the HighLevelSynthesis pass because this would call the instruction's _define method which calls two_qubit_cnot_decompose. A subsequent BasisTranslator pass ultimately maps the cx to the rzz basis. I'm not sure whether this indirect path to unitary synthesis is intentional.

This pr resolves the specific example posed in the issue.

qiskit-bot · 2024-06-05T09:33:27Z

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

@Qiskit/terra-core

coveralls · 2024-06-05T09:53:58Z

Pull Request Test Coverage Report for Build 9381719291

Details

2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
8 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.01%) to 89.6%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	2	93.64%
crates/qasm2/src/parse.rs	6	97.61%

Totals
Change from base Build 9372719342:	0.01%
Covered Lines:	62374
Relevant Lines:	69614

💛 - Coveralls

coveralls · 2024-06-05T14:16:33Z

Pull Request Test Coverage Report for Build 9385447583

Details

2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
8 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.009%) to 89.597%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	2	93.64%
crates/qasm2/src/parse.rs	6	97.61%

Totals
Change from base Build 9372719342:	0.009%
Covered Lines:	62372
Relevant Lines:	69614

💛 - Coveralls

mtreinish · 2024-06-06T19:28:02Z

qiskit/transpiler/passes/synthesis/unitary_synthesis.py

@@ -782,15 +782,15 @@ def _replace_parameterized_gate(op):
        def is_supercontrolled(gate):
            try:
                operator = Operator(gate)
-            except QiskitError:
+            except (QiskitError, TypeError):  # TypeError: parametrized gate


What is the source of a TypeError here? Is it coming from __array__ ?

I ask because I'm wondering if this is the correct fix, if it's coming from __array__ because the gate is parameterized in the target, I'm wondering if we should instantiate it with a fixed angle instead. Basically update _replace_parameterized_gate to handle RZZ too, or instead maybe try to generalize it. A parameterized gate in the target represents that any angle value is valid input so we're free to evaluate the matrix at a fixed angle if we want.

Yes this seems to be the cause. I assigned it as done in _replace_parameterized_gate for now. I don't love the idea of implicitly making that choice here but at least there's precedence. c45d02d

coveralls · 2024-07-18T12:41:44Z

Pull Request Test Coverage Report for Build 10082610521

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

1 of 2 (50.0%) changed or added relevant lines in 1 file are covered.
692 unchanged lines in 33 files lost coverage.
Overall coverage increased (+0.02%) to 89.939%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	1	2	50.0%

Files with Coverage Reduction	New Missed Lines	%
qiskit/circuit/library/standard_gates/swap.py	1	98.18%
qiskit/circuit/duration.py	1	70.27%
crates/accelerate/src/synthesis/permutation/utils.rs	1	99.02%
crates/qasm2/src/expr.rs	1	94.02%
qiskit/transpiler/passes/layout/sabre_pre_layout.py	1	98.88%
qiskit/circuit/random/utils.py	1	94.87%
qiskit/transpiler/passes/routing/commuting_2q_gate_routing/commuting_2q_gate_router.py	1	99.12%
qiskit/transpiler/passes/scheduling/alap.py	2	96.72%
qiskit/transpiler/passes/scheduling/asap.py	2	96.92%
crates/accelerate/src/convert_2q_block_matrix.rs	2	94.0%

Totals
Change from base Build 9976077291:	0.02%
Covered Lines:	65988
Relevant Lines:	73370

💛 - Coveralls

mtreinish

This LGTM, I just have a small nit in the tests for future proofing (since I just had to deal with related failures caused by it on a different PR). I'll just apply that and enqueue this for merge.

test/python/transpiler/test_basis_translator.py

* linting * linting...relax check for 3-qubit circuit * update test docstrings * black update * bind RZZ to pi/2 if Rzz(theta) in basis * Apply suggestions from code review --------- Co-authored-by: Matthew Treinish <[email protected]> (cherry picked from commit aaaf107)

* linting * linting...relax check for 3-qubit circuit * update test docstrings * black update * bind RZZ to pi/2 if Rzz(theta) in basis * Apply suggestions from code review --------- Co-authored-by: Matthew Treinish <[email protected]> (cherry picked from commit aaaf107) Co-authored-by: ewinston <[email protected]>

This reverts commit 3baff4c.

* linting * linting...relax check for 3-qubit circuit * update test docstrings * black update * bind RZZ to pi/2 if Rzz(theta) in basis * Apply suggestions from code review --------- Co-authored-by: Matthew Treinish <[email protected]>

ewinston added 3 commits June 5, 2024 04:24

linting

15e5f4a

linting...relax check for 3-qubit circuit

8021e04

update test docstrings

69386ec

ewinston requested review from alexanderivrii, ShellyGarion and a team as code owners June 5, 2024 09:33

black update

e5e59fd

mtreinish reviewed Jun 6, 2024

View reviewed changes

1ucian0 added Changelog: Bugfix Include in the "Fixed" section of the changelog labels Jun 10, 2024

ewinston added 2 commits July 18, 2024 00:39

Merge branch 'main' into issue/12311

d2eb307

bind RZZ to pi/2 if Rzz(theta) in basis

c45d02d

1ucian0 added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Jul 19, 2024

ElePT added this to the 1.2.0 milestone Jul 22, 2024

mtreinish reviewed Jul 24, 2024

View reviewed changes

test/python/transpiler/test_basis_translator.py Outdated Show resolved Hide resolved

test/python/transpiler/test_basis_translator.py Outdated Show resolved Hide resolved

Apply suggestions from code review

7cdbeac

mtreinish approved these changes Jul 24, 2024

View reviewed changes

mtreinish enabled auto-merge July 24, 2024 19:11

mtreinish modified the milestones: 1.2.0, 1.1.2 Jul 24, 2024

mtreinish added this pull request to the merge queue Jul 24, 2024

Merged via the queue into Qiskit:main with commit aaaf107 Jul 24, 2024
15 checks passed

mergify bot mentioned this pull request Jul 24, 2024

fix issue/12311 with fractional gate in basis (backport #12511) #12813

Merged

mtreinish added a commit that referenced this pull request Jul 25, 2024

Revert "fix issue/12311 with fractional gate in basis (#12511) (#12813)"

bb47ea6

This reverts commit 3baff4c.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix issue/12311 with fractional gate in basis #12511

fix issue/12311 with fractional gate in basis #12511

ewinston commented Jun 5, 2024

qiskit-bot commented Jun 5, 2024

coveralls commented Jun 5, 2024 •

edited

Loading

coveralls commented Jun 5, 2024 •

edited

Loading

mtreinish Jun 6, 2024

ewinston Jul 18, 2024

coveralls commented Jul 18, 2024 •

edited

Loading

mtreinish left a comment

fix issue/12311 with fractional gate in basis #12511

fix issue/12311 with fractional gate in basis #12511

Conversation

ewinston commented Jun 5, 2024

Summary

Details and comments

qiskit-bot commented Jun 5, 2024

coveralls commented Jun 5, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9381719291

Details

💛 - Coveralls

coveralls commented Jun 5, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9385447583

Details

💛 - Coveralls

mtreinish Jun 6, 2024

Choose a reason for hiding this comment

ewinston Jul 18, 2024

Choose a reason for hiding this comment

coveralls commented Jul 18, 2024 • edited Loading

Pull Request Test Coverage Report for Build 10082610521

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

mtreinish left a comment

Choose a reason for hiding this comment

coveralls commented Jun 5, 2024 •

edited

Loading

coveralls commented Jun 5, 2024 •

edited

Loading

coveralls commented Jul 18, 2024 •

edited

Loading