From 258fb234487431ab7b53c3e214d9e05e2fb3a876 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 14 Nov 2023 17:15:33 -0500 Subject: [PATCH] Speed up InverseCancellation when there's nothing to cancel (#11211) * Speed up InverseCancellation when there's nothing to cancel In the cases when there is nothing to cancel in the DAGCircuit the InverseCancellation pass would iterate over the entire circuit twice, once to search for self inverse gates and then a second time to search for other inverse pairs. This is typically fairly fast because the actual search is done in rust, but there is a python callback function that is called for each node. Depending on the size of the circuit this could add up to a significant amount of time. This commit updates the logic in the pass to first check if there are any operations being asked to cancel for either stage, and if there are then only do the iteration if there are any gates in the circuit in the set of intructions that will be cancelled, which means we'll need to do an iteration. These checks are all O(1) for any sized dag, so they're much lower overhead and will mean the pass executes much faster in the case when there isn't anything to cancel. * Speed-up when there is a partial match Building off of the previous commit this speeds up the inverse cancellation pass when only some of the inverse pairs are not present in the DAG, but others are present. In this case the previous commit would still iterate over the full dag multiple times even when we know some of the inverse pairs are not present in the DAG. This commit updates the logic to not call collect_runs() if we know it's going to be empty or there is no cancellation opportunity. * Expand test coverage * Add more tests * Fix handling of parameterized gates This commit adds a fix for an issue that was caught while tuning the performance of the pass. If a parameterized self inverse was passed in the pass would incorrectly treat all instances of that parameterized' gate as being a self inverse without checking the parameter values. This commit corrects this oversight to handle this case to only cancel gates when the optimization is correct. --- .../optimization/inverse_cancellation.py | 47 +++++-- ...terized-self-inverse-7cb2d68b273640f8.yaml | 22 ++++ .../transpiler/test_inverse_cancellation.py | 117 +++++++++++++++++- 3 files changed, 175 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/fix-parameterized-self-inverse-7cb2d68b273640f8.yaml diff --git a/qiskit/transpiler/passes/optimization/inverse_cancellation.py b/qiskit/transpiler/passes/optimization/inverse_cancellation.py index 35dae7d85f07..560cdcd707f4 100644 --- a/qiskit/transpiler/passes/optimization/inverse_cancellation.py +++ b/qiskit/transpiler/passes/optimization/inverse_cancellation.py @@ -58,12 +58,16 @@ def __init__(self, gates_to_cancel: List[Union[Gate, Tuple[Gate, Gate]]]): self.self_inverse_gates = [] self.inverse_gate_pairs = [] + self.self_inverse_gate_names = set() + self.inverse_gate_pairs_names = set() for gates in gates_to_cancel: if isinstance(gates, Gate): self.self_inverse_gates.append(gates) + self.self_inverse_gate_names.add(gates.name) else: self.inverse_gate_pairs.append(gates) + self.inverse_gate_pairs_names.update(x.name for x in gates) super().__init__() @@ -76,11 +80,13 @@ def run(self, dag: DAGCircuit): Returns: DAGCircuit: Transformed DAG. """ + if self.self_inverse_gates: + dag = self._run_on_self_inverse(dag) + if self.inverse_gate_pairs: + dag = self._run_on_inverse_pairs(dag) + return dag - dag = self._run_on_self_inverse(dag, self.self_inverse_gates) - return self._run_on_inverse_pairs(dag, self.inverse_gate_pairs) - - def _run_on_self_inverse(self, dag: DAGCircuit, self_inverse_gates: List[Gate]): + def _run_on_self_inverse(self, dag: DAGCircuit): """ Run self-inverse gates on `dag`. @@ -91,14 +97,27 @@ def _run_on_self_inverse(self, dag: DAGCircuit, self_inverse_gates: List[Gate]): Returns: DAGCircuit: Transformed DAG. """ + op_counts = dag.count_ops() + if not self.self_inverse_gate_names.intersection(op_counts): + return dag # Sets of gate runs by name, for instance: [{(H 0, H 0), (H 1, H 1)}, {(X 0, X 0}] - gate_runs_sets = [dag.collect_runs([gate.name]) for gate in self_inverse_gates] - for gate_runs in gate_runs_sets: + for gate in self.self_inverse_gates: + gate_name = gate.name + gate_count = op_counts.get(gate_name, 0) + if gate_count <= 1: + continue + gate_runs = dag.collect_runs([gate_name]) for gate_cancel_run in gate_runs: partitions = [] chunk = [] for i in range(len(gate_cancel_run) - 1): - chunk.append(gate_cancel_run[i]) + if gate_cancel_run[i].op == gate: + chunk.append(gate_cancel_run[i]) + else: + if chunk: + partitions.append(chunk) + chunk = [] + continue if gate_cancel_run[i].qargs != gate_cancel_run[i + 1].qargs: partitions.append(chunk) chunk = [] @@ -112,7 +131,7 @@ def _run_on_self_inverse(self, dag: DAGCircuit, self_inverse_gates: List[Gate]): dag.remove_op_node(node) return dag - def _run_on_inverse_pairs(self, dag: DAGCircuit, inverse_gate_pairs: List[Tuple[Gate, Gate]]): + def _run_on_inverse_pairs(self, dag: DAGCircuit): """ Run inverse gate pairs on `dag`. @@ -123,8 +142,16 @@ def _run_on_inverse_pairs(self, dag: DAGCircuit, inverse_gate_pairs: List[Tuple[ Returns: DAGCircuit: Transformed DAG. """ - for pair in inverse_gate_pairs: - gate_cancel_runs = dag.collect_runs([pair[0].name, pair[1].name]) + op_counts = dag.count_ops() + if not self.inverse_gate_pairs_names.intersection(op_counts): + return dag + + for pair in self.inverse_gate_pairs: + gate_0_name = pair[0].name + gate_1_name = pair[1].name + if gate_0_name not in op_counts or gate_1_name not in op_counts: + continue + gate_cancel_runs = dag.collect_runs([gate_0_name, gate_1_name]) for dag_nodes in gate_cancel_runs: i = 0 while i < len(dag_nodes) - 1: diff --git a/releasenotes/notes/fix-parameterized-self-inverse-7cb2d68b273640f8.yaml b/releasenotes/notes/fix-parameterized-self-inverse-7cb2d68b273640f8.yaml new file mode 100644 index 000000000000..7ca796cdd2a5 --- /dev/null +++ b/releasenotes/notes/fix-parameterized-self-inverse-7cb2d68b273640f8.yaml @@ -0,0 +1,22 @@ +--- +fixes: + - | + Fixed an issue with the :class:`~.InverseCancellation` pass where it would + incorrectly cancel gates passed in as self inverses with a parameter + value, if a run of gates had a different parameter value. For example:: + + from math import pi + + from qiskit.circuit.library import RZGate + from qiskit.circuit import QuantumCircuit + from qiskit.transpiler.passes import InverseCancellation + + inverse_pass = InverseCancellation([RZGate(0)]) + + qc = QuantumCircuit(1) + qc.rz(0, 0) + qc.rz(pi, 0) + + inverse_pass(qc) + + would previously have incorrectly cancelled the two rz gates. diff --git a/test/python/transpiler/test_inverse_cancellation.py b/test/python/transpiler/test_inverse_cancellation.py index 686b01dd5d7c..c39b531c1b0f 100644 --- a/test/python/transpiler/test_inverse_cancellation.py +++ b/test/python/transpiler/test_inverse_cancellation.py @@ -22,7 +22,17 @@ from qiskit.transpiler.passes import InverseCancellation from qiskit.transpiler import PassManager from qiskit.test import QiskitTestCase -from qiskit.circuit.library import RXGate, HGate, CXGate, PhaseGate, XGate, TGate, TdgGate +from qiskit.circuit.library import ( + RXGate, + HGate, + CXGate, + PhaseGate, + XGate, + TGate, + TdgGate, + CZGate, + RZGate, +) class TestInverseCancellation(QiskitTestCase): @@ -271,6 +281,111 @@ def test_cx_do_not_wrongly_cancel(self): self.assertIn("cx", gates_after) self.assertEqual(gates_after["cx"], 2) + def test_no_gates_to_cancel(self): + """Test when there are no gates to cancel.""" + qc = QuantumCircuit(2) + qc.cx(0, 1) + qc.cx(1, 0) + inverse_pass = InverseCancellation([HGate()]) + new_circ = inverse_pass(qc) + self.assertEqual(qc, new_circ) + + def test_some_cancel_rules_to_cancel(self): + """Test when there are some gates to cancel.""" + qc = QuantumCircuit(2) + qc.cx(0, 1) + qc.cx(1, 0) + qc.h(0) + qc.h(0) + inverse_pass = InverseCancellation([HGate(), CXGate(), CZGate()]) + new_circ = inverse_pass(qc) + self.assertNotIn("h", new_circ.count_ops()) + + def test_no_inverse_pairs(self): + """Test when there are no inverse pairs to cancel.""" + qc = QuantumCircuit(1) + qc.s(0) + qc.sdg(0) + inverse_pass = InverseCancellation([(TGate(), TdgGate())]) + new_circ = inverse_pass(qc) + self.assertEqual(qc, new_circ) + + def test_some_inverse_pairs(self): + """Test when there are some but not all inverse pairs to cancel.""" + qc = QuantumCircuit(1) + qc.s(0) + qc.sdg(0) + qc.t(0) + qc.tdg(0) + inverse_pass = InverseCancellation([(TGate(), TdgGate())]) + new_circ = inverse_pass(qc) + self.assertNotIn("t", new_circ.count_ops()) + self.assertNotIn("tdg", new_circ.count_ops()) + + def test_some_inverse_and_cancelled(self): + """Test when there are some but not all pairs to cancel.""" + qc = QuantumCircuit(2) + qc.s(0) + qc.sdg(0) + qc.t(0) + qc.tdg(0) + qc.cx(0, 1) + qc.cx(1, 0) + qc.h(0) + qc.h(0) + inverse_pass = InverseCancellation([HGate(), CXGate(), CZGate(), (TGate(), TdgGate())]) + new_circ = inverse_pass(qc) + self.assertNotIn("h", new_circ.count_ops()) + self.assertNotIn("t", new_circ.count_ops()) + self.assertNotIn("tdg", new_circ.count_ops()) + + def test_half_of_an_inverse_pair(self): + """Test that half of an inverse pair doesn't do anything.""" + qc = QuantumCircuit(1) + qc.t(0) + qc.t(0) + qc.t(0) + inverse_pass = InverseCancellation([(TGate(), TdgGate())]) + new_circ = inverse_pass(qc) + self.assertEqual(new_circ, qc) + + def test_parameterized_self_inverse(self): + """Test that a parameterized self inverse gate cancels correctly.""" + qc = QuantumCircuit(1) + qc.rz(0, 0) + qc.rz(0, 0) + inverse_pass = InverseCancellation([RZGate(0)]) + new_circ = inverse_pass(qc) + self.assertEqual(new_circ, QuantumCircuit(1)) + + def test_parameterized_self_inverse_not_equal_parameter(self): + """Test that a parameterized self inverse gate doesn't cancel incorrectly.""" + qc = QuantumCircuit(1) + qc.rz(0, 0) + qc.rz(3.14159, 0) + qc.rz(0, 0) + inverse_pass = InverseCancellation([RZGate(0)]) + new_circ = inverse_pass(qc) + self.assertEqual(new_circ, qc) + + def test_controlled_gate_open_control_does_not_cancel(self): + """Test that a controlled gate with an open control doesn't cancel.""" + qc = QuantumCircuit(2) + qc.cx(0, 1) + qc.cx(0, 1, ctrl_state=0) + inverse_pass = InverseCancellation([CXGate()]) + new_circ = inverse_pass(qc) + self.assertEqual(new_circ, qc) + + def test_backwards_pair(self): + """Test a backwards inverse pair works.""" + qc = QuantumCircuit(1) + qc.tdg(0) + qc.t(0) + inverse_pass = InverseCancellation([(TGate(), TdgGate())]) + new_circ = inverse_pass(qc) + self.assertEqual(new_circ, QuantumCircuit(1)) + if __name__ == "__main__": unittest.main()