From faaf06cd293b5042bb80dbb58fea81923fe6e5c6 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Thu, 13 Oct 2022 11:32:49 +0100 Subject: [PATCH] Fix StochasticSwap routing for if blocks with no else If a control-flow block is not required to be entered (such as an "if" with no "else"), then it can't affect the layout in case the branch isn't taken. This failure would have been clearer in a more control-flow-graph view of the circuit, but we don't yet have that, which is how this bug snuck through. The case of "exit layout need not match entry layout" is really the special case for routing, which this commit makes a bit clearer. --- .../passes/routing/stochastic_swap.py | 158 +++++------------- .../python/transpiler/test_stochastic_swap.py | 37 +++- 2 files changed, 82 insertions(+), 113 deletions(-) diff --git a/qiskit/transpiler/passes/routing/stochastic_swap.py b/qiskit/transpiler/passes/routing/stochastic_swap.py index ea0c8d206740..13fd6afb1184 100644 --- a/qiskit/transpiler/passes/routing/stochastic_swap.py +++ b/qiskit/transpiler/passes/routing/stochastic_swap.py @@ -370,57 +370,9 @@ def _controlflow_layer_update(self, dagcircuit_output, layer_dag, current_layout TranspilerError: if layer_dag does not contain a recognized ControlFlowOp. """ - cf_opnode = layer_dag.op_nodes()[0] - if isinstance(cf_opnode.op, IfElseOp): - new_op, new_qargs, new_layout = self._route_control_flow_multiblock( - cf_opnode, current_layout, root_dag - ) - elif isinstance(cf_opnode.op, (ForLoopOp, WhileLoopOp)): - new_op, new_qargs, new_layout = self._route_control_flow_looping( - cf_opnode, current_layout, root_dag - ) - else: - raise TranspilerError(f"unsupported control flow operation: {cf_opnode}") - if not self.fake_run: - dagcircuit_output.apply_operation_back(new_op, new_qargs, cf_opnode.cargs) - return new_layout - - def _new_seed(self): - """Get a seed for a new RNG instance.""" - return self.rng.integers(0x7FFF_FFFF_FFFF_FFFF) - - def _recursive_pass(self, initial_layout): - """Get a new instance of this class to handle a recursive call for a control-flow block. - - Each pass starts with its own new seed, determined deterministically from our own.""" - return self.__class__( - self.coupling_map, - # This doesn't cause an exponential explosion of the trials because we only generate a - # recursive pass instance for control-flow operations, while the trial multiplicity is - # only for non-control-flow layers. - trials=self.trials, - seed=self._new_seed(), - fake_run=self.fake_run, - initial_layout=initial_layout, - ) - - def _route_control_flow_multiblock(self, node, current_layout, root_dag): - """Route control flow instructions which contain multiple blocks (e.g. :class:`.IfElseOp`). - Since each control flow block may yield a different layout, this function applies swaps to - the shorter depth blocks to make all final layouts match. - - Args: - node (DAGOpNode): A DAG node whose operation is a :class:`.ControlFlowOp` that contains - more than one block, such as :class:`.IfElseOp`. - current_layout (Layout): The current layout at the start of the instruction. - root_dag (DAGCircuit): root dag of compilation - - Returns: - ControlFlowOp: routed control flow operation. - List[Qubit]: the new physical-qubit arguments that the output `ControlFlowOp` should be - applied to. This might be wider than the input node if internal routing was needed. - Layout: the new layout after the control-flow operation is applied. - """ + node = layer_dag.op_nodes()[0] + if not isinstance(node.op, (IfElseOp, ForLoopOp, WhileLoopOp)): + raise TranspilerError(f"unsupported control flow operation: {node}") # For each block, expand it up be the full width of the containing DAG so we can be certain # that it is routable, then route it within that. When we recombine later, we'll reduce all # these blocks down to remove any qubits that are idle. @@ -433,79 +385,61 @@ def _route_control_flow_multiblock(self, node, current_layout, root_dag): block_dags.append(inner_pass.run(full_dag_block)) block_layouts.append(inner_pass.property_set["final_layout"].copy()) - # Add swaps to the end of each block to make sure they all have the same layout at the end. - # As a heuristic we choose the final layout of the deepest block to be the target for - # everyone. Adding these swaps can cause fewer wires to be idle than we expect (if we have - # to swap across unused qubits), so we track that at this point too. - deepest_index = np.argmax([block.depth(recurse=True) for block in block_dags]) - final_layout = block_layouts[deepest_index] + # Determine what layout we need to go towards. For some blocks (such as `for`), we must + # guarantee that the final layout is the same as the initial or the loop won't work. For an + # `if` with an `else`, we don't need that as long as the two branches are the same. We have + # to be careful with `if` _without_ an else, though - the `if` needs to restore the layout + # in case it isn't taken; we can't have two different virtual layouts. + if not (isinstance(node.op, IfElseOp) and len(node.op.blocks) == 2): + final_layout = current_layout + else: + # We heuristically just choose to use the layout of whatever the deepest block is, to + # avoid extending the total depth by too much. + final_layout = max( + zip(block_layouts, block_dags), key=lambda x: x[1].depth(recurse=True) + )[0] if self.fake_run: - return None, None, final_layout + return final_layout + + # Add swaps to the end of each block to make sure they all have the same layout at the end. + # Adding these swaps can cause fewer wires to be idle than we expect (if we have to swap + # across unused qubits), so we track that at this point too. idle_qubits = set(root_dag.qubits) - for i, updated_dag_block in enumerate(block_dags): - if i != deepest_index: - swap_dag, swap_qubits = get_swap_map_dag( - root_dag, - self.coupling_map, - block_layouts[i], - final_layout, - seed=self._new_seed(), - ) - if swap_dag.depth(): - updated_dag_block.compose(swap_dag, qubits=swap_qubits) + for layout, updated_dag_block in zip(block_layouts, block_dags): + swap_dag, swap_qubits = get_swap_map_dag( + root_dag, self.coupling_map, layout, final_layout, seed=self._new_seed() + ) + if swap_dag.size(recurse=False): + updated_dag_block.compose(swap_dag, qubits=swap_qubits) idle_qubits &= set(updated_dag_block.idle_wires()) # Now for each block, expand it to be full width over all active wires (all blocks of a # control-flow operation need to have equal input wires), and convert it to circuit form. block_circuits = [] - for i, updated_dag_block in enumerate(block_dags): + for updated_dag_block in block_dags: updated_dag_block.remove_qubits(*idle_qubits) block_circuits.append(dag_to_circuit(updated_dag_block)) - return node.op.replace_blocks(block_circuits), block_circuits[0].qubits, final_layout - def _route_control_flow_looping(self, node, current_layout, root_dag): - """Route a control-flow operation that represents a loop, such as :class:`.ForOpLoop` or - :class:`.WhileOpLoop`. Importantly, these operations have a single block inside, and the - final layout of the block needs to match the initial layout so the loop can continue. + new_op = node.op.replace_blocks(block_circuits) + new_qargs = block_circuits[0].qubits + dagcircuit_output.apply_operation_back(new_op, new_qargs, node.cargs) + return final_layout - Args: - node (DAGOpNode): A DAG node whose operation is a :class:`.ControlFlowOp` that - represents a loop with a single block, such as :class:`.ForLoopOp`. - current_layout (Layout): The current layout at the start of the instruction. - root_dag (DAGCircuit): root dag of compilation + def _new_seed(self): + """Get a seed for a new RNG instance.""" + return self.rng.integers(0x7FFF_FFFF_FFFF_FFFF) - Returns: - ControlFlowOp: routed control flow operation. - List[Qubit]: the new physical-qubit arguments that the output `ControlFlowOp` should be - applied to. This might be wider than the input node if internal routing was needed. - Layout: the new layout after the control-flow operation is applied. - """ - if self.fake_run: - return None, None, current_layout - # Temporarily expand to full width, and route within that. - inner_pass = self._recursive_pass(current_layout) - full_dag_block = root_dag.copy_empty_like() - full_dag_block.compose(circuit_to_dag(node.op.blocks[0]), qubits=node.qargs) - updated_dag_block = inner_pass.run(full_dag_block) - - # Ensure that the layout at the end of the block is returned to being the layout at the - # start of the block again, so the loop works. - swap_dag, swap_qubits = get_swap_map_dag( - root_dag, + def _recursive_pass(self, initial_layout): + """Get a new instance of this class to handle a recursive call for a control-flow block. + + Each pass starts with its own new seed, determined deterministically from our own.""" + return self.__class__( self.coupling_map, - inner_pass.property_set["final_layout"], - current_layout, + # This doesn't cause an exponential explosion of the trials because we only generate a + # recursive pass instance for control-flow operations, while the trial multiplicity is + # only for non-control-flow layers. + trials=self.trials, seed=self._new_seed(), - ) - if swap_dag.depth(): - updated_dag_block.compose(swap_dag, qubits=swap_qubits) - - # Contract the routed block back down to only operate on the qubits that it actually needs. - idle_qubits = set(root_dag.qubits) & set(updated_dag_block.idle_wires()) - updated_dag_block.remove_qubits(*idle_qubits) - updated_circ_block = dag_to_circuit(updated_dag_block) - return ( - node.op.replace_blocks([updated_circ_block]), - updated_dag_block.qubits, - current_layout, + fake_run=self.fake_run, + initial_layout=initial_layout, ) diff --git a/test/python/transpiler/test_stochastic_swap.py b/test/python/transpiler/test_stochastic_swap.py index 839cf69a63a4..da37210a8c9a 100644 --- a/test/python/transpiler/test_stochastic_swap.py +++ b/test/python/transpiler/test_stochastic_swap.py @@ -18,7 +18,7 @@ from ddt import ddt, data from qiskit.transpiler.passes import StochasticSwap -from qiskit.transpiler import CouplingMap, PassManager +from qiskit.transpiler import CouplingMap, PassManager, Layout from qiskit.transpiler.exceptions import TranspilerError from qiskit.converters import circuit_to_dag, dag_to_circuit from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit @@ -1173,6 +1173,41 @@ def test_multiple_ops_per_layer(self): expected.for_loop((0,), None, efor_body, [3, 4, 5], []) self.assertEqual(cqc, expected) + def test_if_no_else_restores_layout(self): + """Test that an if block with no else branch restores the initial layout. If there is an + else branch, we don't need to guarantee this.""" + qc = QuantumCircuit(8, 1) + with qc.if_test((qc.clbits[0], False)): + # Just some arbitrary gates with no perfect layout. + qc.cx(3, 5) + qc.cx(4, 6) + qc.cx(1, 4) + qc.cx(7, 4) + qc.cx(0, 5) + qc.cx(7, 3) + qc.cx(1, 3) + qc.cx(5, 2) + qc.cx(6, 7) + qc.cx(3, 2) + qc.cx(6, 2) + qc.cx(2, 0) + qc.cx(7, 6) + coupling = CouplingMap.from_line(8) + pass_ = StochasticSwap(coupling, seed=2022_10_13) + transpiled = pass_(qc) + + # Check the pass claims to have done things right. + initial_layout = Layout.generate_trivial_layout(*qc.qubits) + self.assertEqual(initial_layout, pass_.property_set["final_layout"]) + + # Check that pass really did do it right. + inner_block = transpiled.data[0].operation.blocks[0] + running_layout = initial_layout.copy() + for instruction in inner_block: + if instruction.operation.name == "swap": + running_layout.swap(*instruction.qubits) + self.assertEqual(initial_layout, running_layout) + @ddt class TestStochasticSwapRandomCircuitValidOutput(QiskitTestCase):