limiting matrix-based commutativity check

[alexanderivrii]

Summary

Fixes #10488.

The matrix-based commutativity check in CommutationChecker requires to construct 2^N x 2^N matrices, leading to memory problems when N is large.

I have added a new argument max_num_qubits to CommutationChecker::commute that allows to skip the commutativity check if the number of qubits of either operation exceeds this amount.

Note that simpler versions of the commutativity check not involving matrix multiplication continue to work as before (e.g., two quantum operations over disjoint sets of qubits commute regardless of the number of qubits).

Details and comments

This actually changes the behavior of commutation checking when the number of qubits is in the range from 4 to 12 (when the matrices could be constructed and multiplied). However personally I don't think we will be missing a lot of optimization opportunities. I am even thinking that we should maybe limit max_num_qubits to 2, what are your thoughts on this?

Implementation-wise, I was a bit unsure whether to make max_num_qubits to be a global constant, or to pass it as the argument to CommutationChecker::__init__, or to pass it as the argument to CommutationChecker::commute (this is what I have chosen at the moment). I can see a potential use-case of having the same instance of CommutationChecker, and then first doing some optimizations based on a lighter-weight version of the check, and then possibly more optimizations based on heavier-weight version of the check (the same instance allows to use the cached results from previous calls).

[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 5656018131

• 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
• 15 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-0.01%) to 85.929%

---

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.76%
qiskit/extensions/unitary.py	1	93.75%
crates/qasm2/src/parse.rs	6	97.13%
crates/qasm2/src/lex.rs	7	91.65%

Totals
Change from base Build 5650486720:	-0.01%
Covered Lines:	73045
Relevant Lines:	85006

---

💛 - Coveralls

[jakelishman]

This looks like a reasonable limit to me, and one that's very unlikely to ever be tripped (the bug report from a stress test notwithstanding), since I'm not aware of much hardware that has support for 4+q operations natively (although I suppose the MS interaction in ions can do that).

Looks like lint's complaining a bit - you can probably safely disable too-many-returns in the function that's tripping it, if you think that combining some of the early returns will make the function harder to understand not easier. The line length thing I think is in the docstring, which black won't touch, so you'll need to edit it manually.

[jakelishman]

The code looks good now, thanks. A reason I'm slightly hesitant to tag this for backport to 0.25.0 is that strictly I think this is more of a new feature, and the bugfix is very low priority. Perhaps we should (in addition to the fix note) put a feature note in about the new option to CommutationChecker, and merge this for the October Qiskit release instead? What do you think, Sasha?

[jakelishman]

Thanks for the quick updates!

For context: Sasha and I spoke privately and agreed that this would be safer as a feature note targetting Qiskit 0.45 rather than the stable 0.25.0 branch, because of the potential changed behaviour. The bugfix is low priority, since it will only trigger with particular very large circuits when no basis gates are set (or a backend that claims to support arbitrary-sized mcx/mct operations).