From 6291b2e7d72ba7ffd2251e715ceafa9759a8f49a Mon Sep 17 00:00:00 2001 From: AlexanderIvrii Date: Wed, 4 Sep 2024 09:11:37 +0300 Subject: [PATCH 1/2] bug fix, test, reno --- .../passes/optimization/hoare_opt.py | 16 +++++---- .../notes/fix-hoare-opt-56d1ca6a07f07a2d.yaml | 5 +++ test/python/transpiler/test_hoare_opt.py | 35 +++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/fix-hoare-opt-56d1ca6a07f07a2d.yaml diff --git a/qiskit/transpiler/passes/optimization/hoare_opt.py b/qiskit/transpiler/passes/optimization/hoare_opt.py index a77f1985f696..b6bb0aa3cbce 100644 --- a/qiskit/transpiler/passes/optimization/hoare_opt.py +++ b/qiskit/transpiler/passes/optimization/hoare_opt.py @@ -203,7 +203,10 @@ def _traverse_dag(self, dag): """ import z3 - for node in dag.topological_op_nodes(): + # It does not look safe to iterate over DAG while removing and replacing + # some of its nodes. + nodes = list(dag.topological_op_nodes()) + for node in nodes: gate = node.op ctrlqb, ctrlvar, trgtqb, trgtvar = self._seperate_ctrl_trgt(node) @@ -212,11 +215,12 @@ def _traverse_dag(self, dag): remove_ctrl, new_dag, qb_idx = self._remove_control(gate, ctrlvar, trgtvar) if remove_ctrl: - dag.substitute_node_with_dag(node, new_dag) - gate = gate.base_gate - node.op = gate.to_mutable() - node.name = gate.name - node.qargs = tuple((ctrlqb + trgtqb)[qi] for qi in qb_idx) + # We are replacing a node by a new node over a smaller number of qubits. + # This can be done using substitute_node_with_dag, which furthermore returns + # a mapping from old node ids to new nodes. + mapped_nodes = dag.substitute_node_with_dag(node, new_dag) + node = list(mapped_nodes.values())[0] + gate = node.op _, ctrlvar, trgtqb, trgtvar = self._seperate_ctrl_trgt(node) ctrl_ones = z3.And(*ctrlvar) diff --git a/releasenotes/notes/fix-hoare-opt-56d1ca6a07f07a2d.yaml b/releasenotes/notes/fix-hoare-opt-56d1ca6a07f07a2d.yaml new file mode 100644 index 000000000000..7548dcc99564 --- /dev/null +++ b/releasenotes/notes/fix-hoare-opt-56d1ca6a07f07a2d.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed a bug in :class:`.HoareOptimizer` where a controlled gate was simplified + by removing its controls but the new gate was not handled correctly. diff --git a/test/python/transpiler/test_hoare_opt.py b/test/python/transpiler/test_hoare_opt.py index b3ae3df97678..ae2f9d9ae1e0 100644 --- a/test/python/transpiler/test_hoare_opt.py +++ b/test/python/transpiler/test_hoare_opt.py @@ -661,6 +661,41 @@ def test_multiple_pass(self): self.assertEqual(result2, circuit_to_dag(expected)) + def test_remove_control_then_identity(self): + """This first simplifies a gate by removing its control, then removes the + simplified gate by canceling it with another gate. + See: https://github.com/Qiskit/qiskit-terra/issues/13079 + """ + # ┌───┐┌───┐┌───┐ + # q_0: ┤ X ├┤ X ├┤ X ├ + # └─┬─┘└───┘└─┬─┘ + # q_1: ──■─────────┼── + # ┌───┐ │ + # q_2: ┤ X ├───────■── + # └───┘ + circuit = QuantumCircuit(3) + circuit.cx(1, 0) + circuit.x(2) + circuit.x(0) + circuit.cx(2, 0) + + simplified = HoareOptimizer()(circuit) + + # The CX(1, 0) gate is removed as the control qubit q_1 is initially 0. + # The CX(2, 0) gate is first replaced by X(0) gate as the control qubit q_2 is at 1, + # then the two X(0) gates are removed. + # + # q_0: ───── + # + # q_1: ───── + # ┌───┐ + # q_2: ┤ X ├ + # └───┘ + expected = QuantumCircuit(3) + expected.x(2) + + self.assertEqual(simplified, expected) + if __name__ == "__main__": unittest.main() From d819a424df9297eb2792dce77ef4079f9a9f8c17 Mon Sep 17 00:00:00 2001 From: AlexanderIvrii Date: Wed, 4 Sep 2024 15:38:25 +0300 Subject: [PATCH 2/2] suggestions from code review; lint --- qiskit/transpiler/passes/optimization/hoare_opt.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/qiskit/transpiler/passes/optimization/hoare_opt.py b/qiskit/transpiler/passes/optimization/hoare_opt.py index b6bb0aa3cbce..e47074fc6142 100644 --- a/qiskit/transpiler/passes/optimization/hoare_opt.py +++ b/qiskit/transpiler/passes/optimization/hoare_opt.py @@ -203,23 +203,23 @@ def _traverse_dag(self, dag): """ import z3 - # It does not look safe to iterate over DAG while removing and replacing - # some of its nodes. + # Pre-generate all DAG nodes, since we later iterate over them, while + # potentially modifying and removing some of them. nodes = list(dag.topological_op_nodes()) for node in nodes: gate = node.op - ctrlqb, ctrlvar, trgtqb, trgtvar = self._seperate_ctrl_trgt(node) + _, ctrlvar, trgtqb, trgtvar = self._seperate_ctrl_trgt(node) ctrl_ones = z3.And(*ctrlvar) - remove_ctrl, new_dag, qb_idx = self._remove_control(gate, ctrlvar, trgtvar) + remove_ctrl, new_dag, _ = self._remove_control(gate, ctrlvar, trgtvar) if remove_ctrl: # We are replacing a node by a new node over a smaller number of qubits. # This can be done using substitute_node_with_dag, which furthermore returns # a mapping from old node ids to new nodes. mapped_nodes = dag.substitute_node_with_dag(node, new_dag) - node = list(mapped_nodes.values())[0] + node = next(iter(mapped_nodes.values())) gate = node.op _, ctrlvar, trgtqb, trgtvar = self._seperate_ctrl_trgt(node)