diff --git a/qiskit_aer/backends/aer_compiler.py b/qiskit_aer/backends/aer_compiler.py index 22dce5d145..f48d6771d5 100644 --- a/qiskit_aer/backends/aer_compiler.py +++ b/qiskit_aer/backends/aer_compiler.py @@ -12,7 +12,10 @@ """ Compier to convert Qiskit control-flow to Aer backend. """ -from qiskit.circuit import QuantumCircuit + +import itertools + +from qiskit.circuit import QuantumCircuit, Clbit from qiskit.extensions import Initialize from qiskit.pulse import Schedule, ScheduleBlock from qiskit.circuit.controlflow import ( @@ -117,61 +120,76 @@ def _is_dynamic(circuit, optype=None): return bool(optype.intersection(controlflow_types)) # Check via iteration - for inst, _, _ in circuit.data: - if isinstance(inst, controlflow_types): + for instruction in circuit.data: + if isinstance(instruction.operation, controlflow_types): return True return False - def _inline_circuit(self, circ, continue_label, break_label): + def _inline_circuit(self, circ, continue_label, break_label, bit_map=None): """convert control-flow instructions to mark and jump instructions Args: circ (QuantumCircuit): The QuantumCircuit to be compiled continue_label (str): label name for continue. break_label (str): label name for break. + bit_map (dict[Bit, Bit]): mapping of virtual bits in the current circuit to the bit they + represent in the outermost circuit. Returns: QuantumCircuit: QuantumCircuit without control-flow instructions """ - ret = circ.copy() - ret.data = [] - - q2i = {} - for q in ret.qubits: - q2i[q] = len(q2i) - c2i = {} - for c in ret.clbits: - c2i[c] = len(c2i) - - for inst, qargs, cargs in circ.data: - binding_qargs = [q2i[q] for q in qargs] - binding_cargs = [c2i[c] for c in cargs] - if isinstance(inst, ForLoopOp): - self._inline_for_loop_op(inst, ret, binding_qargs, binding_cargs) - elif isinstance(inst, WhileLoopOp): - self._inline_while_loop_op(inst, ret, binding_qargs, binding_cargs) - elif isinstance(inst, IfElseOp): - self._inline_if_else_op(inst, continue_label, break_label, - ret, binding_qargs, binding_cargs) - elif isinstance(inst, BreakLoopOp): - ret.append(AerJump(break_label, ret.num_qubits), - range(ret.num_qubits), []) - elif isinstance(inst, ContinueLoopOp): - ret.append(AerJump(continue_label, ret.num_qubits), - range(ret.num_qubits), []) + ret = circ.copy_empty_like() + bit_map = {bit: bit for bit in itertools.chain(ret.qubits, ret.clbits)} + + for instruction in circ.data: + # The barriers around all control-flow operations is to prevent any non-control-flow + # operations from ending up topologically "inside" a body. This can happen if the body + # is not full width on the circuit, and the other operation uses disjoint bits. + if isinstance(instruction.operation, ForLoopOp): + ret.barrier() + self._inline_for_loop_op(instruction, ret, bit_map) + ret.barrier() + elif isinstance(instruction.operation, WhileLoopOp): + ret.barrier() + self._inline_while_loop_op(instruction, ret, bit_map) + ret.barrier() + elif isinstance(instruction.operation, IfElseOp): + ret.barrier() + self._inline_if_else_op(instruction, continue_label, break_label, ret, bit_map) + ret.barrier() + elif isinstance(instruction.operation, BreakLoopOp): + ret._append( + AerJump(break_label, ret.num_qubits, ret.num_clbits), ret.qubits, ret.clbits + ) + elif isinstance(instruction.operation, ContinueLoopOp): + ret._append( + AerJump(continue_label, ret.num_qubits, ret.num_clbits), ret.qubits, ret.clbits + ) else: - ret.append(inst, qargs, cargs) + ret._append(instruction) return ret - def _convert_c_if_args(self, cond_tuple): - """convert a boolean value to 0 or 1 in c_if elements""" - return [1 if elem is True else 0 if elem is False else elem for elem in cond_tuple] + def _convert_c_if_args(self, cond_tuple, bit_map): + """Convert a condition tuple according to the wire map.""" + if isinstance(cond_tuple[0], Clbit): + return (bit_map[cond_tuple[0]], cond_tuple[1]) + # ClassicalRegister conditions should already be in the outer circuit. + return cond_tuple - def _inline_for_loop_op(self, inst, parent, qargs, cargs): + def _inline_for_loop_op(self, instruction, parent, bit_map): """inline for_loop body while iterating its indexset""" - indexset, loop_parameter, body = inst.params + qargs = [bit_map[q] for q in instruction.qubits] + cargs = [bit_map[c] for c in instruction.clbits] + indexset, loop_parameter, body = instruction.operation.params + inner_bit_map = { + inner: bit_map[outer] + for inner, outer in itertools.chain( + zip(body.qubits, instruction.qubits), + zip(body.clbits, instruction.clbits), + ) + } self._last_flow_id += 1 loop_id = self._last_flow_id @@ -181,21 +199,19 @@ def _inline_for_loop_op(self, inst, parent, qargs, cargs): break_label = f'{loop_name}_end' for index in indexset: continue_label = f'{loop_name}_{index}' - inlined_body = self._inline_circuit(body, - continue_label, - break_label) + inlined_body = self._inline_circuit(body, continue_label, break_label, inner_bit_map) if loop_parameter is not None: inlined_body = inlined_body.bind_parameters({loop_parameter: index}) parent.append(inlined_body, qargs, cargs) - parent.append(AerMark(continue_label, inlined_body.num_qubits), qargs, []) + parent.append(AerMark(continue_label, len(qargs), len(cargs)), qargs, cargs) - if inlined_body: - parent.append(AerMark(break_label, inlined_body.num_qubits), qargs, []) + if inlined_body is not None: + parent.append(AerMark(break_label, len(qargs), len(cargs)), qargs, cargs) - def _inline_while_loop_op(self, inst, parent, qargs, cargs): + def _inline_while_loop_op(self, instruction, parent, bit_map): """inline while_loop body with jump and mark instructions""" - condition_tuple = inst.condition - body, = inst.params + condition_tuple = self._convert_c_if_args(instruction.operation.condition, bit_map) + body, = instruction.operation.params self._last_flow_id += 1 loop_id = self._last_flow_id @@ -204,23 +220,44 @@ def _inline_while_loop_op(self, inst, parent, qargs, cargs): continue_label = f'{loop_name}_continue' loop_start_label = f'{loop_name}_start' break_label = f'{loop_name}_end' - inlined_body = self._inline_circuit(body, continue_label, break_label) - - c_if_args = self._convert_c_if_args(condition_tuple) + inlined_body = self._inline_circuit( + body, + continue_label, + break_label, + { + inner: bit_map[outer] + for inner, outer in itertools.chain( + zip(body.qubits, instruction.qubits), + zip(body.clbits, instruction.clbits), + ) + }, + ) + qargs = [bit_map[q] for q in instruction.qubits] + cargs = [bit_map[c] for c in instruction.clbits] + mark_cargs = cargs.copy() + mark_cargs.extend( + bit_map[c] for c in ( + ( + {condition_tuple[0]} if isinstance(condition_tuple[0], Clbit) + else set(condition_tuple[0]) + ) - set(instruction.clbits) + ) + ) + c_if_args = self._convert_c_if_args(condition_tuple, bit_map) - parent.append(AerMark(continue_label, inlined_body.num_qubits), qargs, []) - parent.append(AerJump(loop_start_label, inlined_body.num_qubits).c_if(*c_if_args), - qargs, []) - parent.append(AerJump(break_label, inlined_body.num_qubits), qargs, []) - parent.append(AerMark(loop_start_label, inlined_body.num_qubits), qargs, []) + parent.append(AerMark(continue_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) + parent.append(AerJump(loop_start_label, len(qargs), len(mark_cargs)).c_if(*c_if_args), + qargs, mark_cargs) + parent.append(AerJump(break_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) + parent.append(AerMark(loop_start_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) parent.append(inlined_body, qargs, cargs) - parent.append(AerJump(continue_label, inlined_body.num_qubits), qargs, []) - parent.append(AerMark(break_label, inlined_body.num_qubits), qargs, []) + parent.append(AerJump(continue_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) + parent.append(AerMark(break_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) - def _inline_if_else_op(self, inst, continue_label, break_label, parent, qargs, cargs): + def _inline_if_else_op(self, instruction, continue_label, break_label, parent, bit_map): """inline true and false bodies of if_else with jump and mark instructions""" - condition_tuple = inst.condition - true_body, false_body = inst.params + condition_tuple = instruction.operation.condition + true_body, false_body = instruction.operation.params self._last_flow_id += 1 if_id = self._last_flow_id @@ -233,20 +270,54 @@ def _inline_if_else_op(self, inst, continue_label, break_label, parent, qargs, c else: if_else_label = if_end_label - c_if_args = self._convert_c_if_args(condition_tuple) + c_if_args = self._convert_c_if_args(condition_tuple, bit_map) + + qargs = [bit_map[q] for q in instruction.qubits] + cargs = [bit_map[c] for c in instruction.clbits] + mark_cargs = cargs.copy() + mark_cargs.extend( + bit_map[c] for c in ( + ( + {condition_tuple[0]} if isinstance(condition_tuple[0], Clbit) + else set(condition_tuple[0]) + ) - set(instruction.clbits) + ) + ) - parent.append(AerJump(if_true_label, true_body.num_qubits).c_if(*c_if_args), qargs, []) - parent.append(AerJump(if_else_label, true_body.num_qubits), qargs, []) - parent.append(AerMark(if_true_label, true_body.num_qubits), qargs, []) - parent.append(self._inline_circuit(true_body, continue_label, break_label), qargs, cargs) + true_bit_map = { + inner: bit_map[outer] + for inner, outer in itertools.chain( + zip(true_body.qubits, instruction.qubits), + zip(true_body.clbits, instruction.clbits), + ) + } - if false_body: - parent.append(AerJump(if_end_label, true_body.num_qubits), qargs, []) - parent.append(AerMark(if_else_label, true_body.num_qubits), qargs, []) - parent.append(self._inline_circuit(false_body, continue_label, break_label), - qargs, cargs) + parent.append( + AerJump(if_true_label, len(qargs), len(mark_cargs)).c_if(*c_if_args), qargs, mark_cargs + ) + parent.append(AerJump(if_else_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) + parent.append(AerMark(if_true_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) + parent.append( + self._inline_circuit(true_body, continue_label, break_label, true_bit_map), qargs, cargs + ) - parent.append(AerMark(if_end_label, true_body.num_qubits), qargs, []) + if false_body: + false_bit_map = { + inner: bit_map[outer] + for inner, outer in itertools.chain( + zip(false_body.qubits, instruction.qubits), + zip(false_body.clbits, instruction.clbits), + ) + } + parent.append(AerJump(if_end_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) + parent.append(AerMark(if_else_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) + parent.append( + self._inline_circuit(false_body, continue_label, break_label, false_bit_map), + qargs, + cargs, + ) + + parent.append(AerMark(if_end_label, len(qargs), len(mark_cargs)), qargs, mark_cargs) def compile_circuit(circuits, basis_gates=None, optypes=None): diff --git a/qiskit_aer/library/control_flow_instructions/jump.py b/qiskit_aer/library/control_flow_instructions/jump.py index 4ae7a09709..2239e17a8a 100644 --- a/qiskit_aer/library/control_flow_instructions/jump.py +++ b/qiskit_aer/library/control_flow_instructions/jump.py @@ -25,5 +25,5 @@ class AerJump(Instruction): _directive = True - def __init__(self, jump_to, num_qubits): - super().__init__("jump", num_qubits, 0, [jump_to]) + def __init__(self, jump_to, num_qubits, num_clbits=0): + super().__init__("jump", num_qubits, num_clbits, [jump_to]) diff --git a/qiskit_aer/library/control_flow_instructions/mark.py b/qiskit_aer/library/control_flow_instructions/mark.py index c2589c30e6..bd63018d17 100644 --- a/qiskit_aer/library/control_flow_instructions/mark.py +++ b/qiskit_aer/library/control_flow_instructions/mark.py @@ -26,5 +26,5 @@ class AerMark(Instruction): _directive = True - def __init__(self, name, num_qubits): - super().__init__("mark", num_qubits, 0, [name]) + def __init__(self, name, num_qubits, num_clbits=0): + super().__init__("mark", num_qubits, num_clbits, [name]) diff --git a/releasenotes/notes/fix-topological-control-flow-e2f1a25098004f00.yaml b/releasenotes/notes/fix-topological-control-flow-e2f1a25098004f00.yaml new file mode 100644 index 0000000000..e528ffb52b --- /dev/null +++ b/releasenotes/notes/fix-topological-control-flow-e2f1a25098004f00.yaml @@ -0,0 +1,29 @@ +--- +fixes: + - | + Fixed incorrect logic in the control-flow compiler that could allow unrelated instructions to + appear "inside" control-flow bodies during execution, causing incorrect results. For example, + previously:: + + from qiskit import QuantumCircuit + from qiskit_aer import AerSimulator + + backend = AerSimulator(method="statevector") + + circuit = QuantumCircuit(3, 3) + circuit.measure(0, 0) + circuit.measure(1, 1) + + with circuit.if_test((0, True)): + with circuit.if_test((1, False)): + circuit.x(2) + + with circuit.if_test((0, False)): + with circuit.if_test((1, True)): + circuit.x(2) + + circuit.measure(range(3), range(3)) + print(backend.run(circuit, method=method, shots=100).result()) + + would print ``{'010': 100}`` as the nested control-flow operations would accidentally jump over + the first X gate on qubit 2, which should have been executed. diff --git a/test/terra/backends/aer_simulator/test_control_flow.py b/test/terra/backends/aer_simulator/test_control_flow.py index bb7b35382e..51f2005f96 100644 --- a/test/terra/backends/aer_simulator/test_control_flow.py +++ b/test/terra/backends/aer_simulator/test_control_flow.py @@ -528,3 +528,77 @@ def test_while_loop_last(self, method): result = backend.run(circ, method=method).result() self.assertSuccess(result) + + @data("statevector", "density_matrix", "matrix_product_state") + def test_no_invalid_nested_reordering(self, method): + """Test that the jump/mark system doesn't allow nested conditional marks to jump incorrectly + relative to their outer marks. Regression test of gh-1665.""" + backend = self.backend(method=method) + + circuit = QuantumCircuit(3, 3) + circuit.initialize('010', circuit.qubits) + circuit.measure(0, 0) + circuit.measure(1, 1) + with circuit.if_test((0, True)): + with circuit.if_test((1, False)): + circuit.x(2) + with circuit.if_test((0, False)): + with circuit.if_test((1, True)): + circuit.x(2) + circuit.measure(range(3), range(3)) + + result = backend.run(circuit, method=method, shots=100).result() + self.assertSuccess(result) + self.assertEqual(result.get_counts(), {"110": 100}) + + @data("statevector", "density_matrix", "matrix_product_state") + def test_no_invalid_reordering_if(self, method): + """Test that the jump/mark system doesn't allow an unrelated operation to jump inside a + conditional statement.""" + backend = self.backend(method=method) + + circuit = QuantumCircuit(3, 3) + circuit.measure(1, 1) + # This should never be entered. + with circuit.if_test((1, True)): + circuit.x(0) + circuit.x(2) + # In the topological ordering that `DAGCircuit` uses, this X on qubit 1 will naturally + # appear between the X on 0 and X on 2 once they are inlined, and the jump/mark instructions + # won't act as a full barrier, because the if test doesn't touch qubit 1. In this case, the + # X on 1 will migrate to between the jump and mark, causing it to be skipped along with the + # other X gates. This test ensures that suitable full-width barriers are in place to stop + # that from happening. + circuit.x(1) + + circuit.measure(circuit.qubits, circuit.clbits) + + result = backend.run(circuit, method=method, shots=100).result() + self.assertSuccess(result) + self.assertEqual(result.get_counts(), {"010": 100}) + + @data("statevector", "density_matrix", "matrix_product_state") + def test_no_invalid_reordering_while(self, method): + """Test that the jump/mark system doesn't allow an unrelated operation to jump inside a + conditional statement.""" + backend = self.backend(method=method) + + circuit = QuantumCircuit(3, 3) + circuit.measure(1, 1) + # This should never be entered. + with circuit.while_loop((1, True)): + circuit.x(0) + circuit.x(2) + # In the topological ordering that `DAGCircuit` uses, this X on qubit 1 will naturally + # appear between the X on 0 and X on 2 once they are inlined, and the jump/mark instructions + # won't act as a full barrier, because the if test doesn't touch qubit 1. In this case, the + # X on 1 will migrate to between the jump and mark, causing it to be skipped along with the + # other X gates. This test ensures that suitable full-width barriers are in place to stop + # that from happening. + circuit.x(1) + + circuit.measure(circuit.qubits, circuit.clbits) + + result = backend.run(circuit, method=method, shots=100).result() + self.assertSuccess(result) + self.assertEqual(result.get_counts(), {"010": 100})