Improve commutation checking of Pauli product rotations and measurements

[alexanderivrii]

Summary

This PR improves commutation checking of pairs of Pauli-based objects, that is of PauliProductRotationGate and PauliProductMeasurement. Without this PR, we construct the generators for operations for PPRs and PPMs as SparseObservables and then check if the two SparseObservables commute. With this PR, we first instead construct the generatoors as Paulis (represented using Z and X components) and check if two Paulis commute. The latter check is quite a bit faster than the former for large gates.

Based on top of #15810.

Details and comments

I run this on the 100 representative HamLib benchmarks from benchpress using the following script

ham_records = json.load(open("./100_representative.json", "r"))

for i, h in enumerate(ham_records):
    nq = h.pop("ham_qubits")
    terms = h.pop("ham_hamlib_hamiltonian_terms")
    coefficients = h.pop("ham_hamlib_hamiltonian_coefficients")

    # Construct circuit from PPRs
    qc = QuantumCircuit(nq)
    for t, c in zip(terms, coefficients):
        ppr = PauliProductRotationGate(Pauli(t), c)
        qc.append(ppr, range(nq))

    # Convert to DAG
    dag = circuit_to_dag(qc)

    # Run commutative optimization on DAG, measuring the time
    time_start = time.perf_counter()
    dagt = CommutativeOptimization().run(dag)
    time_end = time.perf_counter()
    print(f"Test {i}: {time_end-time_start}")

Here is the plot showing improvement in runtime (where commutation checker is used within CommutativeOptimization) .

Note that CommutativeOptimization does not actually improve the quality of any of these HamLib benchmarks, however it tends to make a huge number of commutativity checks.

Here is an additional experiment is in the spirit of our FT compiler pipeline (in which case CommutativeOptimization can remove/merge many gates).

qc = qft_circuit(1000)
qc = transpile(qc, basis_gates=get_clifford_gate_names()+["rz"])
qc = LitinskiTransformation(fix_clifford=False, use_ppr=True)(qc)
time_start = time.perf_counter()
qc = CommutativeOptimization()(qc)
time_end = time.perf_counter()
print(f"Time: {time_end-time_start:.4f}")

Without this PR, the average time for CommutativeOptimization is 3.46 seconds, with this PR it's 0.29.

LLM tools used

Used copilot to suggest various micro-optimization and alternative implementations, but in the end the implementation and all possible bugs are purely mine.

Additional optimization (as part of CommutativeOptimization)

In CommutativeOptimization pass we now sort qargs for PauliProductRotationGate and PauliProductMeasurement by qubit index. This allows an even more efficient implementation using the standard method to compute intersection of two sorted vectors.

[qiskit-bot]

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

• @Cryoris
• @Qiskit/terra-core
• @ajavadia

[Cryoris]

I think we discussed this offline but I don't remember the answer: would it be faster to sort both qargs (log timing) and then iterate, rather than finding the max (linear)?

[alexanderivrii]

I tried this now and it became 30% slower, see #15815 for how the experiments were run.

[alexanderivrii]

I agree that for robustness it's best to avoid linear scaling with the total number of qubits in the circuit, so I changed the implementation to use a HashMap instead of a vector. For large Pauli strings over all of circuit qubits (as in the HamLib experiment mentioned in the summary) this makes the implementation about 5% slower, however for short Pauli strings with large qubit indices this improves the implementation. I also moved the optional reversing of the operations to be done earlier, so that we are filling the hashmap with the smaller number of qubits. See 06cbb55 and f4a9df6.

[alexanderivrii]

An update: 087362f implements the "sorted vectors" intersection in addition to "unsorted vectors" intersection. Since CommutativeOptimization now canonicalizes PPRs and PPMs by default (canonicalization for PPMs is added in de13108 in this PR), it uses "sorted vectors" intersection.

Running the experiment from the code snippet in this PRs summary (that is, running CommutativeOptimization on our 100 representative HamLib examples) further improves the time by about 25% (in addition to the improvement mentioned in the summary).

On the other hand, skipping canonicalization for PPRs/PPMs in CommutativeOptimization and sorting them inside commutation checker (as suggested in #15815) makes the total runtime slower by about 30%.

Note that CommutativeOptimization may check commutation of the same PPR with many other PPRs, so what these experiments show is that it's better to sort each PPR once and then use "sorted vectors" intersection, rather than sort each PPR every time it's used.

[Cryoris]

The changes look good to me, but the test coverage is a bit thin. Could we (a) add tests checking PPM commutation with Gucci and (b) scramble the indices of the existing PPR tests in Gucci a bit more (right now there's only 1 index swap)?

[alexanderivrii]

Marking this "on hold" because we need to fix commutation of pauli product measurement instructions with the same clbit (see #16023).

[alexanderivrii]

Following the bugfix in #16023, I have updated the commutation checker to efficiently check the commutation of two PPMs writing to the same clbit (and removed the temporary function).

I have also updated the commutative optimization pass to only canonicalize but not try to merge PPM gates (in particular it does not need to worry about commutativity of two PPM gates).

@Cryoris, this is ready for review now.

[coveralls]

Coverage Report for CI Build 24510449984

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.01%) to 87.49%

Details

• Coverage increased (+0.01%) from the base build.
• Patch coverage: 4 uncovered changes across 1 file (115 of 119 lines covered, 96.64%).
• 18 coverage regressions across 5 files.

Uncovered Changes

File	Changed	Covered	%
crates/transpiler/src/passes/commutative_optimization.rs	63	59	93.65%

Coverage Regressions

18 previously-covered lines in 5 files lost coverage.

File	Lines Losing Coverage	Coverage
crates/circuit/src/parameter/symbol_expr.rs	6	73.93%
crates/qasm2/src/parse.rs	6	97.63%
crates/qasm2/src/lex.rs	4	91.77%
crates/circuit/src/parameter/parameter_expression.rs	1	90.53%
crates/transpiler/src/commutation_checker.rs	1	88.27%

---

Coverage Stats

Relevant Lines:	119596
Covered Lines:	104634
Line Coverage:	87.49%
Coverage Strength:	979624.38 hits per line

---

💛 - Coveralls

[Cryoris]

Suggested change

	// To check commutation of two Pauli-based gates, we extract their Pauli generators
	// and check whether they commute.
	// Note that we have previously removed all PPRs equivalent to identity up to a global
	// phase, so this is both a necessary and a sufficient condition.
	// To check commutation of two Pauli-based gates, we extract their Pauli generators
	// and check whether they commute. This is not done through the commutation checker,
	// since we here know that the Pauli strings are sorted by qubit index already, which
	// allows for a more efficient check.
	//
	// Note that we have previously removed all PPRs equivalent to identity up to a global
	// phase, so this is both a necessary and a sufficient condition.

[alexanderivrii]

For additional clarity, I have moved the Pauli-based commutation code to a separate function, and improved comments/docstrings, see fb03d53.

[Cryoris]

unreachable is for unreachable statements in the compiled path -- but this is well reachable if someone calls this function with PPMs. It would be nice to make this safer.

We could for example change the type of op1 to be a &PauliProductRotation which avoids the fallible case and gives the caller the responsibility of giving the right object

[alexanderivrii]

This line is truly unreachable, after the change in a685517. Since we can't merge PPMs with anything, we no longer try to commute it with other gates, meaning that the first gate passed to commute is not a PPM. I have also added explicit tests for circuits with multiple PPMs.

Actually, are there any other gates that can't be merged/canceled with anything?

[alexanderivrii]

Further improved the unreachability message in a67a060.

[Cryoris]

I'm still not thrilled about removing identities here, especially if we might be using a different default tolerance than the RemoveIdentityEquiv pass. The other inconsistent case is if we have a Target and RemoveIdentityEquiv takes it into account but CommutativeOptimization doesn't.

In #16070 we should add this Target support. Until then, can we at least use the same constant from the remove_identity_equiv.rs file as tolerance, to ensure we query it from the same place?

[alexanderivrii]

Done in d13a51a. I now think that we should have a single place of truth to define MINIMUM_TOL for all passes (currently every pass redefines the same constant).

[Cryoris]

Suggested change

	The :class:`.CommutativeOptimization` transpiler pass now removes gates that are quivalent to
	The :class:`.CommutativeOptimization` transpiler pass now removes gates that are equivalent to

[alexanderivrii]

Oops, I can't type a sentence without a typo -- fixed in d13a51a. (And apparently, I can't also type a commit message with typos.)

[Cryoris]

Thanks for the improvements, Sasha!