Speed up InverseCancellation when there's nothing to cancel (Qiskit#1…

…1211)

* 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.

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:

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.

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()