Remove old graph structure during EquivalenceLibrary.set_entry (#11959

) (#11963)

* Remove old graph structure during `EquivalenceLibrary.set_entry`

The previous implementation of `EquivalenceLibrary.set_entry` changed
the equivalence rules attached to particular nodes within the graph
structure, but didn't correctly update the edges, so the graph was no
longer representing the correct data, and queries within
`BasisTranslator` would still use the old equivalence sets.

This correctly clears out the old rules and adds the new structure when
`set_entry` is used.  The inner private variable `_rule_count` is
replaced with `_rule_id`, since it would actually be misleading for its
use to decrement it; this could cause clashes, and what's actually
intended and used is that it's an id for rules.

* Add test with parallel edges

(cherry picked from commit 67e0243)

Co-authored-by: Jake Lishman <[email protected]>

qiskit/circuit/equivalence.py

@@ -45,11 +45,12 @@ def __init__(self, *, base=None):
         if base is None:
             self._graph = rx.PyDiGraph()
             self._key_to_node_index = {}
-            self._rule_count = 0
+            # Some unique identifier for rules.
+            self._rule_id = 0
         else:
             self._graph = base._graph.copy()
             self._key_to_node_index = copy.deepcopy(base._key_to_node_index)
-            self._rule_count = base._rule_count
+            self._rule_id = base._rule_id
 
     @property
     def graph(self) -> rx.PyDiGraph:
@@ -104,12 +105,12 @@ def add_equivalence(self, gate, equivalent_circuit):
             (
                 self._set_default_node(source),
                 target,
-                EdgeData(index=self._rule_count, num_gates=len(sources), rule=equiv, source=source),
+                EdgeData(index=self._rule_id, num_gates=len(sources), rule=equiv, source=source),
             )
             for source in sources
         ]
         self._graph.add_edges_from(edges)
-        self._rule_count += 1
+        self._rule_id += 1
 
     def has_entry(self, gate):
         """Check if a library contains any decompositions for gate.
@@ -142,10 +143,15 @@ def set_entry(self, gate, entry):
             _raise_if_shape_mismatch(gate, equiv)
             _raise_if_param_mismatch(gate.params, equiv.parameters)
 
-        key = Key(name=gate.name, num_qubits=gate.num_qubits)
-        equivs = [Equivalence(params=gate.params.copy(), circuit=equiv.copy()) for equiv in entry]
-
-        self._graph[self._set_default_node(key)] = NodeData(key=key, equivs=equivs)
+        node_index = self._set_default_node(Key(name=gate.name, num_qubits=gate.num_qubits))
+        # Remove previous equivalences of this node, leaving in place any later equivalences that
+        # were added that use `gate`.
+        self._graph[node_index].equivs.clear()
+        for parent, child, _ in self._graph.in_edges(node_index):
+            # `child` should always be ourselves, but there might be parallel edges.
+            self._graph.remove_edge(parent, child)
+        for equivalence in entry:
+            self.add_equivalence(gate, equivalence)
 
     def get_entry(self, gate):
         """Gets the set of QuantumCircuits circuits from the library which

releasenotes/notes/fix-equivalence-setentry-5a30b0790666fcf2.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Calling :meth:`.EquivalenceLibrary.set_entry` will now correctly update the internal graph
+    object of the library.  Previously, the metadata would be updated, but the graph structure would
+    be unaltered, meaning that users like :class:`.BasisTranslator` would still use the old rules.
+    Fixed `#11958 <https://github.com/Qiskit/qiskit/issues/11958>`__.

test/python/circuit/test_equivalence.py

@@ -104,24 +104,87 @@ def test_add_double_entry(self):
         self.assertEqual(entry[1], second_equiv)
 
     def test_set_entry(self):
-        """Verify setting an entry overrides any previously added."""
+        """Verify setting an entry overrides any previously added, without affecting entries that
+        depended on the set entry."""
         eq_lib = EquivalenceLibrary()
 
-        gate = OneQubitZeroParamGate()
-        first_equiv = QuantumCircuit(1)
-        first_equiv.h(0)
+        gates = {key: Gate(key, 1, []) for key in "abcd"}
+        target = Gate("target", 1, [])
+
+        old = QuantumCircuit(1)
+        old.append(gates["a"], [0])
+        old.append(gates["b"], [0])
+        eq_lib.add_equivalence(target, old)
+
+        outbound = QuantumCircuit(1)
+        outbound.append(target, [0])
+        eq_lib.add_equivalence(gates["c"], outbound)
+
+        self.assertEqual(eq_lib.get_entry(target), [old])
+        self.assertEqual(eq_lib.get_entry(gates["c"]), [outbound])
+        # Assert the underlying graph structure is correct as well.
+        gate_indices = {eq_lib.graph[node].key.name: node for node in eq_lib.graph.node_indices()}
+        self.assertTrue(eq_lib.graph.has_edge(gate_indices["a"], gate_indices["target"]))
+        self.assertTrue(eq_lib.graph.has_edge(gate_indices["b"], gate_indices["target"]))
+        self.assertTrue(eq_lib.graph.has_edge(gate_indices["target"], gate_indices["c"]))
+
+        new = QuantumCircuit(1)
+        new.append(gates["d"], [0])
+        eq_lib.set_entry(target, [new])
+
+        self.assertEqual(eq_lib.get_entry(target), [new])
+        self.assertEqual(eq_lib.get_entry(gates["c"]), [outbound])
+        # Assert the underlying graph structure is correct as well.
+        gate_indices = {eq_lib.graph[node].key.name: node for node in eq_lib.graph.node_indices()}
+        self.assertFalse(eq_lib.graph.has_edge(gate_indices["a"], gate_indices["target"]))
+        self.assertFalse(eq_lib.graph.has_edge(gate_indices["b"], gate_indices["target"]))
+        self.assertTrue(eq_lib.graph.has_edge(gate_indices["d"], gate_indices["target"]))
+        self.assertTrue(eq_lib.graph.has_edge(gate_indices["target"], gate_indices["c"]))
+
+    def test_set_entry_parallel_edges(self):
+        """Test that `set_entry` works correctly in the case of parallel wires."""
+        eq_lib = EquivalenceLibrary()
+        gates = {key: Gate(key, 1, []) for key in "abcd"}
+        target = Gate("target", 1, [])
 
-        eq_lib.add_equivalence(gate, first_equiv)
+        old_1 = QuantumCircuit(1, name="a")
+        old_1.append(gates["a"], [0])
+        old_1.append(gates["b"], [0])
+        eq_lib.add_equivalence(target, old_1)
 
-        second_equiv = QuantumCircuit(1)
-        second_equiv.append(U2Gate(0, np.pi), [0])
+        old_2 = QuantumCircuit(1, name="b")
+        old_2.append(gates["b"], [0])
+        old_2.append(gates["a"], [0])
+        eq_lib.add_equivalence(target, old_2)
 
-        eq_lib.set_entry(gate, [second_equiv])
+        # This extra rule is so that 'a' still has edges, so we can do an exact isomorphism test.
+        # There's not particular requirement for `set_entry` to remove orphan nodes, so we'll just
+        # craft a test that doesn't care either way.
+        a_to_b = QuantumCircuit(1)
+        a_to_b.append(gates["b"], [0])
+        eq_lib.add_equivalence(gates["a"], a_to_b)
 
-        entry = eq_lib.get_entry(gate)
+        self.assertEqual(sorted(eq_lib.get_entry(target), key=lambda qc: qc.name), [old_1, old_2])
 
-        self.assertEqual(len(entry), 1)
-        self.assertEqual(entry[0], second_equiv)
+        new = QuantumCircuit(1, name="c")
+        # No more use of 'a', but re-use 'b' and introduce 'c'.
+        new.append(gates["b"], [0])
+        new.append(gates["c"], [0])
+        eq_lib.set_entry(target, [new])
+
+        self.assertEqual(eq_lib.get_entry(target), [new])
+
+        expected = EquivalenceLibrary()
+        expected.add_equivalence(gates["a"], a_to_b)
+        expected.add_equivalence(target, new)
+
+        def node_fn(left, right):
+            return left == right
+
+        def edge_fn(left, right):
+            return left.rule == right.rule
+
+        self.assertTrue(rx.is_isomorphic(eq_lib.graph, expected.graph, node_fn, edge_fn))
 
     def test_raise_if_gate_entry_shape_mismatch(self):
         """Verify we raise if adding a circuit and gate with different shapes."""