Adapt commutation checker to abstract circuits

[sbrandhsn]

Summary

addresses #11911 and PR #11929

Details and comments

Quantum circuit compilation can be improved by running optimization passes based on commutative analysis before solving the routing problem. Formerly, commutative analysis was only applied to "physical circuits", i.e. entirely specified by Instructions that are readily defined by the unitary matrices of the instructions. Applying commutative analysis before routing requires handling abstract instructions (Operators) that may or may not be defined by a matrix directly. However, the matrix specification of an instruction is required to determine its commutativity.

This PR extends the current commutation analysis that only works on physical circuits defined over Instructions to handle abstract instructions, i.e. checking whether an instruction has a matrix representation and exiting early if otherwise before proceeding to determine the commutation relation.

[qiskit-bot]

One or more of the the following people are requested to review this:

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 8176499038

Details

• 31 of 33 (93.94%) changed or added relevant lines in 1 file are covered.
• 48 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.02%) to 89.306%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/commutation_checker.py	31	33	93.94%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	2	91.44%
qiskit/circuit/instruction.py	8	93.84%
qiskit/qpy/binary_io/circuits.py	38	91.61%

Totals
Change from base Build 8144782269:	0.02%
Covered Lines:	59140
Relevant Lines:	66222

---

💛 - Coveralls

[mtreinish]

Overall this lgtm, I left a couple small comments inline. Could you also add some tests to validate the checker works with annotated operations correctly?

[mtreinish]

Not that I expect this will come up in practice very often, but if this is expensive to compute in some cases wouldn't it be better to only do this once? Right now you'll run this twice, once to check for a skip and later again as part of _commute_matmul.

[sbrandhsn]

Yes, I agree. However, I would need to refactor the commutation checker quite a bit and I wanted to do this PR quickly. :-) Is it okay to leave it as is? I also suspect that this part of the function should not be executed often in practice.

[mtreinish]

I think it's fine to leave it as is, but maybe put a TODO comment or something in there to indicate longer term it'd be better to de-duplicate the Operator() call.

[sbrandhsn]

I shifted the Operator() resolution to _commute_matmul - while this would lead to unnecessary checks for operations that do not have a suitable Operator resolution, I would expect these checks to be comparatively lightweight.

[sbrandhsn]

I added a couple of tests for annotated operations. I was surprised to find that CommutativeCancellation only handles a hard-coded list of 2q gates.

[mtreinish]

I added a couple of tests for annotated operations. I was surprised to find that CommutativeCancellation only handles a hard-coded list of 2q gates.

Oh, yeah that is a surprise, maybe this is a good reason we should migrate to CommutativeInverseCancellation in the preset pass managers instead of CommutativeCancellation. @alexanderivrii is there anything blocking us from doing that (in a different new PR)?

[mtreinish]

Code-wise this LGTM now, I just had an inline suggestion in the release note to provide a bit more context about the feature for users. A code example showing what you can do with it now never hurts either.

[sbrandhsn]

Makes sense, thanks!