Add CommutativeOptimization to transpiler pipeline

[alexanderivrii]

Summary

Addresses #15452.

This PR replaces CommutativeCancellation by CommutativeOptimization in the default compilation pipeline for continuous basis sets, which is relevant for optimization levels 2 and 3. This should provide higher quality results at similar runtimes.

Since this is an important change, we did not want to do it late in 2.3, but it would be nice to do it early for 2.4, so that we have a sufficient time to test the impact.

Details and Comments:

We do not want to apply this change for Clifford+T transpilation, since we need to fully rethink that pipeline, and this is tracked independently in #15457.

[qiskit-bot]

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

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 21632577862

Details

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• 22 unchanged lines in 5 files lost coverage.
• Overall coverage increased (+1.1%) to 89.064%

---

Files with Coverage Reduction	New Missed Lines	%
crates/circuit/src/parameter/parameter_expression.rs	1	87.17%
crates/transpiler/src/passes/commutation_cancellation.rs	2	90.17%
crates/transpiler/src/passes/unitary_synthesis/mod.rs	2	96.72%
crates/qasm2/src/lex.rs	5	92.54%
crates/qasm2/src/parse.rs	12	97.15%

Totals
Change from base Build 21615363168:	1.1%
Covered Lines:	94481
Relevant Lines:	106082

---

💛 - Coveralls

[alexanderivrii]

I have slightly changed the order of the passes in the default optimization pass manager for optimization_level=2,3: now CommutativeOptimization runs before Optimize1qGatesDecomposition.

The previous order when optimization_level=2 was:

• remove identity equiv
• optimize 1q
• commutative optimization
• (optional) basis translator
• fixed point check

There was one example where this got stuck in an infinite loop with the basis set ["rz", "sx"] when the circuit had two consecutive SX-gates:

• commutative optimization merged the two consecutive SX gates into RX
• basis translator rewrote RX as RZ, SX, RZ, SX, RZ
• remove identity equivs removed the inner RZ gate
• the size and the depth of the circuit wobbled a bit, and the fixed point pass used in optimization level=2 (unlike minimum point pass in level 3) was not able to determine convergence

With the new order, optimize1q brings the circuit to the desired basis, the optional basis translator is no longer needed, and no "wobbling" happens.

Regardless of the problematic example above, I believe that applying CommutativeOptimization before Optimize1q also makes sense in general.

[alexanderivrii]

I have rebased this pass on top of main and locally run the full suite of ASV benchmarks.

Regarding performance. Except for the benchmarks mentioned previously in this comment, there was nothing else statistically significant.

Regarding quality of the output circuit. There are actually two family of benchmarks where the depth has increased:

• In QUEKOTranspilerBench.track_depth_bntf_optimal_depth_25 the depth changed from 211 to 327
• In UtilityScaleBenchmarks.track_qft_depth the depth changed from 2692 to 2815

In both cases the change is due not to interchanging the order of Optimize1qGatesDecomposition and CommutativeOptimization, but to the intrinsic randomness of Sabre during transpilation. In other words, the default transpiler seed chosen in the benchmarks favors the main branch. Running each of the two benchmarks 1000 times each with different random seeds (ranging from 0 to 999) gives the following:

QFT:

• main: average 2-qubit count: 11596, average 2-qubit depth: 2753
• this PR: average 2-qubit count: 11579, average 2-qubit depth: 2742

QUEKO:

• main: average CX-count: 439, average RZ-count: 892, average depth: 242
• this PR: average CX-count: 440, average RZ-count: 896, average depth: 246

There is no statistical difference on these benchmarks.

[mtreinish]

If the benchmarks are running without a fixed seed than this kind of variability is to be expected. We should fix the benchmarks to use a fixed seed. Although that will have the unfortunate consequence of invalidating the historical results since we change the benchmark's code and the sha1 of the code is the default version. I would be more curious about a larger more diverse benchmark set for output quality. Benchpress is what comes to mind, but you'll have the same random noise in the quality benchmarks because benchpress doesn't fix a seed. You could modify the benchpress benchmarks to set a transpiler seed and then use that to compare.

[mtreinish]

I was just looking at this again, can you also update the C transpiler pipeline in crates/transpiler/src/transpiler.rs so that the preset pass manager is kept in sync between the C and Python code paths.

[Cryoris]

This needs rebasing on main now 🙂

[Cryoris]

Is CommutativeCancellation faster than Gucci? If yes we can also only run Gucci in the init stage where we expect it to provide an advantage and keep using the existing flow in lower-level stages.