From 1c2ef54421b4a2836888ad2ed9ead91c15a8c864 Mon Sep 17 00:00:00 2001 From: AlexanderIvrii Date: Tue, 25 Jul 2023 12:04:00 +0300 Subject: [PATCH 1/4] limiting matrix-based commutativity check --- qiskit/circuit/commutation_checker.py | 21 ++++++++++++++++--- ...-commutation-checker-66a1b9e90921621f.yaml | 9 ++++++++ .../circuit/test_commutation_checker.py | 11 ++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml diff --git a/qiskit/circuit/commutation_checker.py b/qiskit/circuit/commutation_checker.py index 5edceab1080f..4730d43feb2b 100644 --- a/qiskit/circuit/commutation_checker.py +++ b/qiskit/circuit/commutation_checker.py @@ -65,10 +65,19 @@ def _hashable_parameters(self, params): return ("fallback", str(params)) def commute( - self, op1: Operation, qargs1: List, cargs1: List, op2: Operation, qargs2: List, cargs2: List - ): + self, + op1: Operation, + qargs1: List, + cargs1: List, + op2: Operation, + qargs2: List, + cargs2: List, + max_num_qubits: int = 3, + ) -> bool: """ - Checks if two Operations commute. + Checks if two Operations commute. The return value of `True` means that the operations truly commute, + and the return value of `False` means that either the operations do not commute or that the commutation + check was skipped (for example, when the operations have conditions or have too many qubits). Args: op1: first operation. @@ -77,6 +86,8 @@ def commute( op2: second operation. qargs2: second operation's qubits. cargs2: second operation's clbits. + max_num_qubits: the maximum number of qubits to consider, the check may be skipped if + the number of qubits for either operation exceeds this amount. Returns: bool: whether two operations commute. @@ -105,6 +116,10 @@ def commute( if not (intersection_q or intersection_c): return True + # Skip the check if the number of qubits for either operation is too large + if len(qargs1) > max_num_qubits or len(qargs2) > max_num_qubits: + return False + # These lines are adapted from commutation_analysis, which is more restrictive than the # check from dag_dependency when considering nodes with "_directive". It would be nice to # think which optimizations from dag_dependency can indeed be used. diff --git a/releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml b/releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml new file mode 100644 index 000000000000..7107200bea07 --- /dev/null +++ b/releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + The maximum number of qubits to consider for matrix multiplication-based commutativity check + in :class:`~.CommutationChecker` is now limited to 3 by default. This avoids trying to + internally allocate arrays of size :math:`2^N \times 2^N`. Simpler versions of commutativity + check (for instance, two quantum operations commute when they are over disjoint sets of qubits) + continue to work without this limit. + Fixed `#10488 `__ diff --git a/test/python/circuit/test_commutation_checker.py b/test/python/circuit/test_commutation_checker.py index cd1b08d0ceb9..16f11ef1beee 100644 --- a/test/python/circuit/test_commutation_checker.py +++ b/test/python/circuit/test_commutation_checker.py @@ -25,6 +25,7 @@ XGate, CXGate, CCXGate, + MCXGate, RZGate, Measure, Barrier, @@ -362,6 +363,16 @@ def test_c7x_gate(self): res = CommutationChecker().commute(XGate(), qargs[:1], [], XGate().control(7), qargs, []) self.assertFalse(res) + def test_wide_gates_over_nondisjoint_qubits(self): + """Test that checking wide gates does not lead to memory problems.""" + res = CommutationChecker().commute(MCXGate(29), list(range(30)), [], XGate(), [0], []) + self.assertFalse(res) + + def test_wide_gates_over_disjoint_qubits(self): + """Test that wide gates still commute when they are over disjoint sets of qubits.""" + res = CommutationChecker().commute(MCXGate(29), list(range(30)), [], XGate(), [30], []) + self.assertTrue(res) + if __name__ == "__main__": unittest.main() From 99c0d6ba9a42e2a5e180652a3ceb68b32bd3e9ab Mon Sep 17 00:00:00 2001 From: AlexanderIvrii Date: Tue, 25 Jul 2023 12:29:55 +0300 Subject: [PATCH 2/4] adding tests with the order of gates fixed --- test/python/circuit/test_commutation_checker.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/python/circuit/test_commutation_checker.py b/test/python/circuit/test_commutation_checker.py index 16f11ef1beee..bfb41e649a12 100644 --- a/test/python/circuit/test_commutation_checker.py +++ b/test/python/circuit/test_commutation_checker.py @@ -367,11 +367,15 @@ def test_wide_gates_over_nondisjoint_qubits(self): """Test that checking wide gates does not lead to memory problems.""" res = CommutationChecker().commute(MCXGate(29), list(range(30)), [], XGate(), [0], []) self.assertFalse(res) + res = CommutationChecker().commute(XGate(), [0], [], MCXGate(29), list(range(30)), []) + self.assertFalse(res) def test_wide_gates_over_disjoint_qubits(self): """Test that wide gates still commute when they are over disjoint sets of qubits.""" res = CommutationChecker().commute(MCXGate(29), list(range(30)), [], XGate(), [30], []) self.assertTrue(res) + res = CommutationChecker().commute(XGate(), [30], [], MCXGate(29), list(range(30)), []) + self.assertTrue(res) if __name__ == "__main__": From a36791076de2fa0364fa5c05ad6d5ce603145e2f Mon Sep 17 00:00:00 2001 From: AlexanderIvrii Date: Tue, 25 Jul 2023 13:30:23 +0300 Subject: [PATCH 3/4] lint tweaks --- qiskit/circuit/commutation_checker.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/qiskit/circuit/commutation_checker.py b/qiskit/circuit/commutation_checker.py index 4730d43feb2b..ad9d4830997a 100644 --- a/qiskit/circuit/commutation_checker.py +++ b/qiskit/circuit/commutation_checker.py @@ -75,9 +75,10 @@ def commute( max_num_qubits: int = 3, ) -> bool: """ - Checks if two Operations commute. The return value of `True` means that the operations truly commute, - and the return value of `False` means that either the operations do not commute or that the commutation - check was skipped (for example, when the operations have conditions or have too many qubits). + Checks if two Operations commute. The return value of `True` means that the operations + truly commute, and the return value of `False` means that either the operations do not + commute or that the commutation check was skipped (for example, when the operations + have conditions or have too many qubits). Args: op1: first operation. @@ -92,6 +93,8 @@ def commute( Returns: bool: whether two operations commute. """ + # pylint: disable=too-many-return-statements + # We don't support commutation of conditional gates for now due to bugs in # CommutativeCancellation. See gh-8553. if ( From a9e5bbb69ab76b64e9597ed6b22b09c17acc5c18 Mon Sep 17 00:00:00 2001 From: AlexanderIvrii Date: Tue, 25 Jul 2023 14:09:13 +0300 Subject: [PATCH 4/4] improved released notes --- ...imits-in-commutation-checker-66a1b9e90921621f.yaml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml b/releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml index 7107200bea07..9b8bc6411083 100644 --- a/releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml +++ b/releasenotes/notes/add-limits-in-commutation-checker-66a1b9e90921621f.yaml @@ -1,9 +1,14 @@ --- -fixes: +features: - | - The maximum number of qubits to consider for matrix multiplication-based commutativity check - in :class:`~.CommutationChecker` is now limited to 3 by default. This avoids trying to + Added a new option ``max_num_qubits`` to :meth:`qiskit.circuit.CommutationChecker.commute` + that specifies the maximum number of qubits to consider for the more expensive + matrix multiplication-based commutativity check. This avoids trying to internally allocate arrays of size :math:`2^N \times 2^N`. Simpler versions of commutativity check (for instance, two quantum operations commute when they are over disjoint sets of qubits) continue to work without this limit. +fixes: + - | + The maximum number of qubits to consider for matrix multiplication-based commutativity check + in :class:`~.CommutationChecker` is now limited to 3 by default. Fixed `#10488 `__