From 85e7cdf6a87eaecc4c6fb9f7581f64cedf9602f9 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 7 Mar 2024 08:14:54 +0000 Subject: [PATCH] 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 67e0243abdc62ead580bb1339dca9888d67b0bc7) Co-authored-by: Jake Lishman --- qiskit/circuit/equivalence.py | 22 +++-- ...equivalence-setentry-5a30b0790666fcf2.yaml | 7 ++ test/python/circuit/test_equivalence.py | 85 ++++++++++++++++--- 3 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 releasenotes/notes/fix-equivalence-setentry-5a30b0790666fcf2.yaml diff --git a/qiskit/circuit/equivalence.py b/qiskit/circuit/equivalence.py index 08525905f457..45921c3f2293 100644 --- a/qiskit/circuit/equivalence.py +++ b/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 diff --git a/releasenotes/notes/fix-equivalence-setentry-5a30b0790666fcf2.yaml b/releasenotes/notes/fix-equivalence-setentry-5a30b0790666fcf2.yaml new file mode 100644 index 000000000000..9df1eb36c265 --- /dev/null +++ b/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 `__. diff --git a/test/python/circuit/test_equivalence.py b/test/python/circuit/test_equivalence.py index 2a110f2726a3..db8ed501adca 100644 --- a/test/python/circuit/test_equivalence.py +++ b/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."""