Fix InverseCancellation for symmetric pairs

[Cryoris]

Symmetric gates should get cancelled irrespective of the qubit order. This did not happen in InverseCancellation, which didn't consistently cancel CS and CSdg gates in the default setup since their qubit argument were out of order. Coincidentally it did cancel these gates in a specific order as a by product since the boolean logic was not grouped correctly, as reported in #15855. This commit fixes this behavior.

This commit does not more generally enable handling of symmetric, user-supplied gates.

Fixes #15855. Alternative fix to #16124.

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:

[qiskit-bot]

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

• @Qiskit/terra-core

[jakelishman]

This is definitely a more complete/appropriate "fix", though we're bordering on "new feature" here imo, and we might want a separate backport PR to 2.4 if there's a demonstrable bug. I don't actually think there is an observable bug, because this only triggers in the default Rust path, and in the Rust path there's only cs/csdg which are symmetric. For example:

from qiskit.circuit import Gate, QuantumCircuit
from qiskit.transpiler import passes

class Left(Gate):
    def __init__(self):
        super().__init__("left", 2, [])

    def inverse(self):
        return Right()

class Right(Gate):
    def __init__(self):
        super().__init__("right", 2, [])

    def inverse(self):
        return Left()

qc = QuantumCircuit(2)
qc.append(Left(), [0, 1])
qc.append(Right(), [1, 0])

passes.InverseCancellation([(Left(), Right())])(qc).draw()

shows no cancellation (but they do cancel if you set the qargs the same).

Perhaps let's either treat the existing PR as "refactor for future correctness", or retarget it to 2.5 and try to extend the symmetric flag to Python-space too?

[coveralls]

Coverage Report for CI Build 25503982007

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 decreased (-0.01%) to 87.608%

Details

• Coverage decreased (-0.01%) from the base build.
• Patch coverage: 17 of 17 lines across 1 file are fully covered (100%).
• 21 coverage regressions across 4 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

21 previously-covered lines in 4 files lost coverage.

File	Lines Losing Coverage	Coverage
crates/qasm2/src/parse.rs	12	97.15%
crates/qasm2/src/lex.rs	7	91.52%
crates/circuit/src/parameter/symbol_expr.rs	1	73.84%
crates/qasm2/src/expr.rs	1	93.82%

---

Coverage Stats

Relevant Lines:	122067
Covered Lines:	106940
Line Coverage:	87.61%
Coverage Strength:	961843.05 hits per line

---

💛 - Coveralls

[Cryoris]

This is definitely a more complete/appropriate "fix", though we're bordering on "new feature" here imo, and we might want a separate backport PR to 2.4 if there's a demonstrable bug. I don't actually think there is an observable bug, because this only triggers in the default Rust path, and in the Rust path there's only cs/csdg which are symmetric.

The observable bug is that on 2.4 CSdg-CS cancel but CS-CSdg don't lol (that's related to the incorrect precedence in the boolean check reported in the original issue). So on 2.4 we have

qc = QuantumCircuit(2)
qc.cs(1, 0)
qc.csdg(0, 1)
tqc = InverseCancellation()(qc)  # gates are not cancelled, but should be

but

qc = QuantumCircuit(2)
qc.csdg(0, 1)
qc.cs(1, 0)
tqc = InverseCancellation()(qc)  # gates are cancelled 

Based on this I think it would be good to backport this, but expose the symmetry feature to Python for 2.5, no?

[jakelishman]

Ah ok, then yeah I'm good to backport as a bug fix.

[alexanderivrii]

I am wondering whether we really need this very-very specific feature "eliminate symmetric CS/CSdg gates" in InverseCancellation, given that it does not eliminate other pairs of symmetric gates, see for instance

qc = QuantumCircuit(4)
qc.cz(0, 2)
qc.cz(0, 2)
qct = InverseCancellation()(qc)
print(qct)

We could definitely add other symmetric gates, though is there a need for this, since CommutativeOptimization already does this?

In the review to #16124, I was suggesting to only include the actual bug-fix which we would then backport to 2.4.

[jakelishman]

Sasha: I'd do this PR over the other just because it also fixes the bug, but doesn't break the coincidental-but-actually-correct cs(0,1)/csdg(1,0) cancellation.

[jakelishman]

If you write the bugfix note like this, we need to handle the cz case Sasha mentioned too.

[alexanderivrii]

From reading the release note, it seems like we are asserting that InverseCancellation should cancel all symmetric pairs of gates, which it does not.

[alexanderivrii]

Lol, this is the second time I start writing a comment and see that Jake already beat me to it.

[alexanderivrii]

My only other concern is if the extra change has a negative impact on performance, after all InverseCancellation is one of the few passes that runs in optimization_level=0. Honestly, I don't think it has any performance impact, but should we run ASV just to make sure?

An aside: CommutativeOptimization does not cancel the left-right pairs from Jake's previous example even when the qargs are the same (it only handle inverse StandardGates), should we extend it to handle PyGates as well? (This breaks its undocumented purpose to strictly supersede all the other cancellation passes).

[jakelishman]

Sahsa: you mean O1 right? We don't run an optimisation loop at all at O0.

[alexanderivrii]

Jake: I thought we run it in the init stage with O0, but I was wrong. So I should correct my comment to O1, where we do run it in both init and optimization stages.

[jakelishman]

Yeah, O0 is totally null in optimisations - the init of O0 only does the necessary 3q unrolling that routing needs.

As to asv: I don't think cs/csdg will show up in the asv benchmarks anywhere, so I doubt we actually would even have asv coverage.