Improve single-qubit gate count in `TwoQubitControlledUDecomposer` by ShellyGarion · Pull Request #16123 · Qiskit/qiskit

ShellyGarion · 2026-05-03T08:09:28Z

This PR reduces the total number of 1-qubit unitaries needed to synthesize a general 2-qubit unitary using TwoQubitControlledUDecomposer from 24 to 8.

This is in particular useful for the peephole optimization in #13419.

Details:

The TwoQubitControlledUDecomposer consists of the following steps.

We start from a general 4x4 unitary U.
Then we find a cannonical gate W (Weyl gate) s.t.
U(0,1) = c2r(0) c2l(1) W(0,1) c1r(0) c1l(1)
this yields 4 1-qubit unitary gates.
Then, we write the Weyl gate W as:
W(0,1) = RXX(0,1) RYY(0,1) RZZ(0,1)
Now, we rewrite RYY(0,1) and RZZ(0,1) as:
RYY(0,1) = sdg(0) sdg(0,1) RXX(0,1) s(0,1) s(0,1)
RZZ(0,1) = h(0) h(0,1) h(0,1) h(0,1) h(0,1)
yielding 8 additional 1-qubit unitary gates.
If V is the target controlled-U gate (which is locally equivalent to RXX), then we rewrite RXX using V:
RXX(0,1) = k2r(0) k2l(1) V(0,1) k1r(0) k1l(1)
yielding 3*4 1-qubit unitary gates.

This gives a total of:
3*4 + 8 + 4 = 24 1-qubit unitary gates.

In this PR we multiply the 1-qubit unitary matrices betwen the 2-qubit gates, to get at most 8 1-qubit unitary gates (in the general case where 3 2-qubit gates are needed).

AI/LLM disclosure

I didn't use LLM tooling, or only used it privately.
I used the following tool to help write this PR description:
I used the following tool to generate or modify code:

coveralls · 2026-05-03T08:40:51Z

Coverage Report for CI Build 25432069188

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.06%) to 87.627%

Details

Coverage increased (+0.06%) from the base build.
Patch coverage: 6 uncovered changes across 1 file (125 of 131 lines covered, 95.42%).
318 coverage regressions across 9 files.

Uncovered Changes

File	Changed	Covered	%
crates/synthesis/src/two_qubit_decompose/controlled_u_decomposer.rs	131	125	95.42%

Coverage Regressions

318 previously-covered lines in 9 files lost coverage.

File	Lines Losing Coverage	Coverage
crates/circuit/src/circuit_drawer.rs	73	95.26%
qiskit/circuit/quantumcircuit.py	71	94.46%
crates/circuit/src/circuit_data.rs	52	87.17%
crates/circuit/src/parameter/parameter_expression.rs	51	91.04%
crates/circuit/src/register_data.rs	42	69.74%
crates/transpiler/src/passes/basis_translator/compose_transforms.rs	16	84.67%
crates/circuit/src/interner.rs	9	97.02%
crates/qasm2/src/lex.rs	3	92.03%
crates/circuit/src/parameter/symbol_expr.rs	1	74.05%

Coverage Stats


Relevant Lines:	122012
Covered Lines:	106916
Line Coverage:	87.63%
Coverage Strength:	961075.11 hits per line

💛 - Coveralls

qiskit-bot · 2026-05-06T10:03:34Z

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

@Qiskit/terra-core
@levbishop

mtreinish

Overall this looks good, thanks for fixing this. I just had a few comments inline.

mtreinish · 2026-05-06T18:29:07Z

+    fn to_rxx_gate(
+        &self,
+        angle: f64,
+    ) -> PyResult<(TwoQubitGateSequence, SmallVec<[Matrix2<Complex64>; 4]>)> {


Since this is always 4 matrices you don't need the smallvec. Smallvec lets you have a dynamically sized array, like a vec, that is stack allocated up to a specified size and above that size is heap allocated like a normal vec. But we don't need that in this case

Suggested change

) -> PyResult<(TwoQubitGateSequence, SmallVec<[Matrix2<Complex64>; 4]>)> {

) -> PyResult<(TwoQubitGateSequence, [Matrix2<Complex64>; 4])> {

done in 0d11c70

mtreinish · 2026-05-06T18:29:32Z

+                gates,
+                global_phase,
+            },
+            smallvec![k2r, k2l, k1r, k1l],


Suggested change

smallvec![k2r, k2l, k1r, k1l],

[k2r, k2l, k1r, k1l],

done in 0d11c70

mtreinish · 2026-05-06T18:51:55Z

-                let (inv_gate_name, inv_gate_params) = invert_1q_gate(gate);
-                gates.push((inv_gate_name, inv_gate_params, smallvec![0]));
-            }
+        let mut gates = Vec::with_capacity(1);


I don't think we need the vec here anymore there is only going to be one gate now so allocating a vec is extra overhead we don't need yet. We can just return a (RXXEquivalent, f64, [Matrix2<Complex64>; 4])

mtreinish · 2026-05-06T18:57:43Z

+        global_phase += weyl_gates.global_phase;
+        weyl_gates.global_phase = global_phase;


Is there a reason to no just do?

Suggested change

global_phase += weyl_gates.global_phase;

weyl_gates.global_phase = global_phase;

weyl_gates.global_phase += global_phase;

mtreinish · 2026-05-06T18:58:23Z

-        target_decomposed: TwoQubitWeylDecomposition,
+        target_decomposed: &TwoQubitWeylDecomposition,
        atol: f64,
+        c_mats: SmallVec<[Matrix2<Complex64>; 4]>,


Same comment here about the type we don't need the small vec since there are always 4:

Suggested change

c_mats: SmallVec<[Matrix2<Complex64>; 4]>,

c_mats: [Matrix2<Complex64>; 4],

done in 0d11c70

mtreinish · 2026-05-06T19:10:28Z

+                rzz_k2r = rzz_mats[2]
+                    .try_inverse()
+                    .expect("matrix k2r is not invertible"); // before RZZ(c), qubit 0


Do you need to preserve the original input here? It doesn't look like it's used anywhere else. If not I would replace this by try_inverse_mut() which will invert the matrix in place and avoid an extra copy.

Actually we don't need to invert these matrices, since these are the original matrices calculated in to_rxx_gates.
This code is removed now, and the function to_rxx_gates was updated to cover this option of returning the original K matirces in 0d11c70

mtreinish · 2026-05-06T19:13:32Z

+        let mut target_1q_basis_list = EulerBasisSet::new();
+        target_1q_basis_list.add_basis(self.euler_basis);


It's not a huge deal, but it would maybe be better to do this once per decomposer. This is a fixed value based on the euler_basis set when we build the struct so it feels a bit unnecessary to reconstruct the EulerBasisSet for each 1q component of the decomposition.

ShellyGarion added 2 commits May 3, 2026 10:21

add a function append_1q_sequence

54cab06

use append_1q_sequence function

c5efa44

ShellyGarion added this to the 2.5.0 milestone May 3, 2026

ShellyGarion requested a review from mtreinish May 3, 2026 08:09

ShellyGarion added synthesis Rust This PR or issue is related to Rust code in the repository labels May 3, 2026

ShellyGarion added this to Qiskit 2.5 May 3, 2026

github-project-automation Bot moved this to Ready in Qiskit 2.5 May 3, 2026

ShellyGarion added 10 commits May 5, 2026 12:53

reduce 1-qubit gtaes, need to debug failing tests

8ecc1ca

update matrix calculation. still need to fix failing tests

4fefebc

minor fix

52158f8

fix bugs, tests are passing now!

01627ba

rename gates1 to weyl_gates

cc29100

fix clippy errors

59e5e9c

move H, S, SDG matrices to common

f5f33b1

simplify append_1q_sequence, remove invert_1q_gate

e421d4d

add tests for the number of gates

527aba1

add another test

53e4d17

ShellyGarion marked this pull request as ready for review May 6, 2026 10:03

ShellyGarion requested a review from a team as a code owner May 6, 2026 10:03

ShellyGarion changed the title ~~[WIP] Improve single-qubit gate count in TwoQubitControlledUDecomposer~~ Improve single-qubit gate count in TwoQubitControlledUDecomposer May 6, 2026

ShellyGarion added 2 commits May 6, 2026 14:08

invert matrices inplace using try_inverse_mut

dd65e2e

add release notes

92165a5

ShellyGarion added the Changelog: Added Add an "Added" entry in the GitHub Release changelog. label May 6, 2026

mtreinish reviewed May 6, 2026

View reviewed changes

mtreinish self-assigned this May 6, 2026

ShellyGarion added 2 commits May 7, 2026 14:29

not inverting the k matrices twice

0d11c70

invert the RXX equivalent gate in to_rxx_gate

73faeab

	) -> PyResult<(TwoQubitGateSequence, SmallVec<[Matrix2<Complex64>; 4]>)> {
	) -> PyResult<(TwoQubitGateSequence, [Matrix2<Complex64>; 4])> {

		global_phase += weyl_gates.global_phase;
		weyl_gates.global_phase = global_phase;

	global_phase += weyl_gates.global_phase;
	weyl_gates.global_phase = global_phase;
	weyl_gates.global_phase += global_phase;

	c_mats: SmallVec<[Matrix2<Complex64>; 4]>,
	c_mats: [Matrix2<Complex64>; 4],

		let mut target_1q_basis_list = EulerBasisSet::new();
		target_1q_basis_list.add_basis(self.euler_basis);

Conversation

ShellyGarion commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI/LLM disclosure

Uh oh!

coveralls commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 25432069188

Coverage increased (+0.06%) to 87.627%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

qiskit-bot commented May 6, 2026

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ShellyGarion commented May 3, 2026 •

edited

Loading

coveralls commented May 3, 2026 •

edited

Loading