From 991263a8db1bbf1e65ec65751046faef2ad0ab58 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Fri, 26 Jul 2024 15:25:07 +0100 Subject: [PATCH 1/2] Avoid operator creation in transpiler This removes very nearly all of the use of `DAGOpNode.op` in the default transpiler paths. The sole exception is in `InverseCancellation`, which currently would involve some quite awkward gymnastics for little near-term benefit. The pass should move fully to Rust soon, making it not worth the effort. Most of the tricks here involve using the knowledge that most operations will involve only Rust-space standard gates, and that these cannot be control-flow operations. --- crates/circuit/src/circuit_instruction.rs | 17 +++- crates/circuit/src/dag_node.rs | 22 +++-- crates/circuit/src/imports.rs | 2 + crates/circuit/src/operations.rs | 3 +- crates/circuit/src/packed_instruction.rs | 2 + qiskit/circuit/commutation_checker.py | 4 +- qiskit/dagcircuit/dagcircuit.py | 85 +++++++++++-------- .../passes/basis/basis_translator.py | 6 +- .../transpiler/passes/layout/apply_layout.py | 16 +++- qiskit/transpiler/passes/layout/vf2_utils.py | 6 +- .../transpiler/passes/routing/sabre_swap.py | 31 +++---- .../passes/synthesis/high_level_synthesis.py | 56 +++++++----- qiskit/transpiler/passes/utils/check_map.py | 3 +- .../utils/convert_conditions_to_if_ops.py | 5 +- qiskit/transpiler/passes/utils/gates_basis.py | 3 +- .../transpiler/preset_passmanagers/common.py | 5 +- .../avoid-op-creation-804c0bed6c408911.yaml | 16 ++++ 17 files changed, 182 insertions(+), 100 deletions(-) create mode 100644 releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml diff --git a/crates/circuit/src/circuit_instruction.rs b/crates/circuit/src/circuit_instruction.rs index 3ab0fe6279f7..9cc763cbd35c 100644 --- a/crates/circuit/src/circuit_instruction.rs +++ b/crates/circuit/src/circuit_instruction.rs @@ -22,7 +22,7 @@ use pyo3::{intern, IntoPy, PyObject, PyResult}; use smallvec::SmallVec; -use crate::imports::{GATE, INSTRUCTION, OPERATION, WARNINGS_WARN}; +use crate::imports::{CONTROL_FLOW_OP, GATE, INSTRUCTION, OPERATION, WARNINGS_WARN}; use crate::operations::{ Operation, OperationRef, Param, PyGate, PyInstruction, PyOperation, StandardGate, }; @@ -266,11 +266,23 @@ impl CircuitInstruction { .and_then(|attrs| attrs.unit.as_deref()) } - #[getter] + /// Is the :class:`.Operation` contained in this instruction a Qiskit standard gate? pub fn is_standard_gate(&self) -> bool { self.operation.try_standard_gate().is_some() } + /// Is the :class:`.Operation` contained in this node a directive? + pub fn is_directive(&self) -> bool { + self.op().directive() + } + + /// Is the :class:`.Operation` contained in this instruction a control-flow operation (i.e. an + /// instance of :class:`.ControlFlowOp`)? + pub fn is_control_flow(&self) -> bool { + self.op().control_flow() + } + + /// Does this instruction contain any :class:`.ParameterExpression` parameters? pub fn is_parameterized(&self) -> bool { self.params .iter() @@ -557,6 +569,7 @@ impl<'py> FromPyObject<'py> for OperationFromPython { clbits: ob.getattr(intern!(py, "num_clbits"))?.extract()?, params: params.len() as u32, op_name: ob.getattr(intern!(py, "name"))?.extract()?, + control_flow: ob.is_instance(CONTROL_FLOW_OP.get_bound(py))?, instruction: ob.into_py(py), }); return Ok(OperationFromPython { diff --git a/crates/circuit/src/dag_node.rs b/crates/circuit/src/dag_node.rs index db9f6f650174..4b6c1b7c0bf0 100644 --- a/crates/circuit/src/dag_node.rs +++ b/crates/circuit/src/dag_node.rs @@ -291,10 +291,6 @@ impl DAGOpNode { self.instruction.params = val; } - pub fn is_parameterized(&self) -> bool { - self.instruction.is_parameterized() - } - #[getter] fn matrix(&self, py: Python) -> Option { let matrix = self.instruction.op().matrix(&self.instruction.params); @@ -333,11 +329,27 @@ impl DAGOpNode { .and_then(|attrs| attrs.unit.as_deref()) } - #[getter] + /// Is the :class:`.Operation` contained in this node a Qiskit standard gate? pub fn is_standard_gate(&self) -> bool { self.instruction.is_standard_gate() } + /// Is the :class:`.Operation` contained in this node a directive? + pub fn is_directive(&self) -> bool { + self.instruction.is_directive() + } + + /// Is the :class:`.Operation` contained in this node a control-flow operation (i.e. an instance + /// of :class:`.ControlFlowOp`)? + pub fn is_control_flow(&self) -> bool { + self.instruction.is_control_flow() + } + + /// Does this node contain any :class:`.ParameterExpression` parameters? + pub fn is_parameterized(&self) -> bool { + self.instruction.is_parameterized() + } + #[setter] fn set_label(&mut self, val: Option) { match self.instruction.extra_attrs.as_mut() { diff --git a/crates/circuit/src/imports.rs b/crates/circuit/src/imports.rs index 153b66392083..7d17aba2440f 100644 --- a/crates/circuit/src/imports.rs +++ b/crates/circuit/src/imports.rs @@ -61,6 +61,8 @@ pub static OPERATION: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.opera pub static INSTRUCTION: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.instruction", "Instruction"); pub static GATE: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.gate", "Gate"); +pub static CONTROL_FLOW_OP: ImportOnceCell = + ImportOnceCell::new("qiskit.circuit.controlflow", "ControlFlowOp"); pub static QUBIT: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.quantumregister", "Qubit"); pub static CLBIT: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.classicalregister", "Clbit"); pub static PARAMETER_EXPRESSION: ImportOnceCell = diff --git a/crates/circuit/src/operations.rs b/crates/circuit/src/operations.rs index 8d3cfdf7f007..caf905a8c036 100644 --- a/crates/circuit/src/operations.rs +++ b/crates/circuit/src/operations.rs @@ -2016,6 +2016,7 @@ pub struct PyInstruction { pub clbits: u32, pub params: u32, pub op_name: String, + pub control_flow: bool, pub instruction: PyObject, } @@ -2033,7 +2034,7 @@ impl Operation for PyInstruction { self.params } fn control_flow(&self) -> bool { - false + self.control_flow } fn matrix(&self, _params: &[Param]) -> Option> { None diff --git a/crates/circuit/src/packed_instruction.rs b/crates/circuit/src/packed_instruction.rs index c909ca3d1b56..ac8795d664c1 100644 --- a/crates/circuit/src/packed_instruction.rs +++ b/crates/circuit/src/packed_instruction.rs @@ -283,6 +283,7 @@ impl PackedOperation { qubits: instruction.qubits, clbits: instruction.clbits, params: instruction.params, + control_flow: instruction.control_flow, op_name: instruction.op_name.clone(), } .into()), @@ -316,6 +317,7 @@ impl PackedOperation { qubits: instruction.qubits, clbits: instruction.clbits, params: instruction.params, + control_flow: instruction.control_flow, op_name: instruction.op_name.clone(), }) .into()), diff --git a/qiskit/circuit/commutation_checker.py b/qiskit/circuit/commutation_checker.py index 5c1fb5586cb7..34cc66e9f1d6 100644 --- a/qiskit/circuit/commutation_checker.py +++ b/qiskit/circuit/commutation_checker.py @@ -66,11 +66,11 @@ def commute_nodes( """Checks if two DAGOpNodes commute.""" qargs1 = op1.qargs cargs1 = op2.cargs - if not op1.is_standard_gate: + if not op1.is_standard_gate(): op1 = op1.op qargs2 = op2.qargs cargs2 = op2.cargs - if not op2.is_standard_gate: + if not op2.is_standard_gate(): op2 = op2.op return self.commute(op1, qargs1, cargs1, op2, qargs2, cargs2, max_num_qubits) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 28a1c16002fa..53cbc6f8f7f1 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -974,15 +974,26 @@ def _reject_new_register(reg): elif isinstance(nd, DAGOpNode): m_qargs = [edge_map.get(x, x) for x in nd.qargs] m_cargs = [edge_map.get(x, x) for x in nd.cargs] - op = nd.op.copy() - if (condition := getattr(op, "condition", None)) is not None: - if not isinstance(op, ControlFlowOp): - op = op.c_if(*variable_mapper.map_condition(condition, allow_reorder=True)) + inst = nd._to_circuit_instruction(deepcopy=True) + m_op = None + if inst.condition is not None: + if inst.is_control_flow(): + m_op = inst.operation + m_op.condition = variable_mapper.map_condition( + inst.condition, allow_reorder=True + ) else: - op.condition = variable_mapper.map_condition(condition, allow_reorder=True) - elif isinstance(op, SwitchCaseOp): - op.target = variable_mapper.map_target(op.target) - dag.apply_operation_back(op, m_qargs, m_cargs, check=False) + m_op = inst.operation.c_if( + *variable_mapper.map_condition(inst.condition, allow_reorder=True) + ) + elif inst.is_control_flow() and isinstance(inst.operation, SwitchCaseOp): + m_op = inst.operation + m_op.target = variable_mapper.map_target(m_op.target) + if m_op is None: + inst = inst.replace(qubits=m_qargs, clbits=m_cargs) + else: + inst = inst.replace(operation=m_op, qubits=m_qargs, clbits=m_cargs) + dag._apply_op_node_back(DAGOpNode.from_instruction(inst), check=False) else: raise DAGCircuitError(f"bad node type {type(nd)}") @@ -1460,11 +1471,7 @@ def substitute_node_with_dag(self, node, input_dag, wires=None, propagate_condit reverse_wire_map = {b: a for a, b in wire_map.items()} # It doesn't make sense to try and propagate a condition from a control-flow op; a # replacement for the control-flow op should implement the operation completely. - if ( - propagate_condition - and not isinstance(node.op, ControlFlowOp) - and (op_condition := getattr(node.op, "condition", None)) is not None - ): + if propagate_condition and not node.is_control_flow() and node.condition is not None: in_dag = input_dag.copy_empty_like() # The remapping of `condition` below is still using the old code that assumes a 2-tuple. # This is because this remapping code only makes sense in the case of non-control-flow @@ -1473,7 +1480,7 @@ def substitute_node_with_dag(self, node, input_dag, wires=None, propagate_condit # in favour of the new-style conditional blocks. The extra logic in here to add # additional wires into the map as necessary would hugely complicate matters if we tried # to abstract it out into the `VariableMapper` used elsewhere. - target, value = op_condition + target, value = node.condition if isinstance(target, Clbit): new_target = reverse_wire_map.get(target, Clbit()) if new_target not in wire_map: @@ -1593,25 +1600,31 @@ def edge_weight_map(wire): for old_node_index, new_node_index in node_map.items(): # update node attributes old_node = in_dag._multi_graph[old_node_index] - if isinstance(old_node.op, SwitchCaseOp): + m_op = None + if not old_node.is_standard_gate() and isinstance(old_node.op, SwitchCaseOp): m_op = SwitchCaseOp( variable_mapper.map_target(old_node.op.target), old_node.op.cases_specifier(), label=old_node.op.label, ) - elif getattr(old_node.op, "condition", None) is not None: + elif old_node.condition is not None: m_op = old_node.op - if not isinstance(old_node.op, ControlFlowOp): + if old_node.is_control_flow(): + m_op.condition = variable_mapper.map_condition(m_op.condition) + else: new_condition = variable_mapper.map_condition(m_op.condition) if new_condition is not None: m_op = m_op.c_if(*new_condition) - else: - m_op.condition = variable_mapper.map_condition(m_op.condition) - else: - m_op = old_node.op m_qargs = [wire_map[x] for x in old_node.qargs] m_cargs = [wire_map[x] for x in old_node.cargs] - new_node = DAGOpNode(m_op, qargs=m_qargs, cargs=m_cargs, dag=self) + old_instruction = old_node._to_circuit_instruction() + if m_op is None: + new_instruction = old_instruction.replace(qubits=m_qargs, clbits=m_cargs) + else: + new_instruction = old_instruction.replace( + operation=m_op, qubits=m_qargs, clbits=m_cargs + ) + new_node = DAGOpNode.from_instruction(new_instruction) new_node._node_id = new_node_index self._multi_graph[new_node_index] = new_node self._increment_op(new_node.name) @@ -1840,11 +1853,18 @@ def op_nodes(self, op=None, include_directives=True): list[DAGOpNode]: the list of node ids containing the given op. """ nodes = [] + filter_is_nonstandard = getattr(op, "_standard_gate", None) is None for node in self._multi_graph.nodes(): if isinstance(node, DAGOpNode): - if not include_directives and getattr(node.op, "_directive", False): + if not include_directives and node.is_directive(): continue - if op is None or isinstance(node.op, op): + if op is None or ( + # This middle catch is to avoid Python-space operation creation for most uses of + # `op`; we're usually just looking for control-flow ops, and standard gates + # aren't control-flow ops. + not (filter_is_nonstandard and node.is_standard_gate()) + and isinstance(node.op, op) + ): nodes.append(node) return nodes @@ -1864,7 +1884,7 @@ def named_nodes(self, *names): """Get the set of "op" nodes with the given name.""" named_nodes = [] for node in self._multi_graph.nodes(): - if isinstance(node, DAGOpNode) and node.op.name in names: + if isinstance(node, DAGOpNode) and node.name in names: named_nodes.append(node) return named_nodes @@ -2070,14 +2090,11 @@ def layers(self, *, vars_mode: _VarsMode = "captures"): new_layer = self.copy_empty_like(vars_mode=vars_mode) for node in op_nodes: - # this creates new DAGOpNodes in the new_layer - new_layer.apply_operation_back(node.op, node.qargs, node.cargs, check=False) + new_layer._apply_op_node_back(node, check=False) # The quantum registers that have an operation in this layer. support_list = [ - op_node.qargs - for op_node in new_layer.op_nodes() - if not getattr(op_node.op, "_directive", False) + op_node.qargs for op_node in new_layer.op_nodes() if not op_node.is_directive() ] yield {"graph": new_layer, "partition": support_list} @@ -2129,11 +2146,7 @@ def collect_runs(self, namelist): """ def filter_fn(node): - return ( - isinstance(node, DAGOpNode) - and node.op.name in namelist - and getattr(node.op, "condition", None) is None - ) + return isinstance(node, DAGOpNode) and node.name in namelist and node.condition is None group_list = rx.collect_runs(self._multi_graph, filter_fn) return {tuple(x) for x in group_list} @@ -2366,7 +2379,7 @@ def _may_have_additional_wires(node) -> bool: # # If updating this, you most likely also need to update `_additional_wires`. return node.condition is not None or ( - not node.is_standard_gate and isinstance(node.op, (ControlFlowOp, Store)) + not node.is_standard_gate() and isinstance(node.op, (ControlFlowOp, Store)) ) diff --git a/qiskit/transpiler/passes/basis/basis_translator.py b/qiskit/transpiler/passes/basis/basis_translator.py index 30b25b271755..3dbdfd5eaad4 100644 --- a/qiskit/transpiler/passes/basis/basis_translator.py +++ b/qiskit/transpiler/passes/basis/basis_translator.py @@ -313,7 +313,7 @@ def _replace_node(self, dag, node, instr_map): inner_node._to_circuit_instruction(), dag=bound_target_dag, ) - if not new_node.is_standard_gate: + if not new_node.is_standard_gate(): new_node.op = new_node.op.copy() if any(isinstance(x, ParameterExpression) for x in inner_node.params): new_params = [] @@ -332,7 +332,7 @@ def _replace_node(self, dag, node, instr_map): new_value = new_value.numeric() new_params.append(new_value) new_node.params = new_params - if not new_node.is_standard_gate: + if not new_node.is_standard_gate(): new_node.op.params = new_params bound_target_dag._apply_op_node_back(new_node) if isinstance(target_dag.global_phase, ParameterExpression): @@ -516,7 +516,7 @@ def edge_cost(self, edge_data): cost_tot = 0 for instruction in edge_data.rule.circuit: - key = Key(name=instruction.operation.name, num_qubits=len(instruction.qubits)) + key = Key(name=instruction.name, num_qubits=len(instruction.qubits)) cost_tot += self._opt_cost_map[key] return cost_tot - self._opt_cost_map[edge_data.source] diff --git a/qiskit/transpiler/passes/layout/apply_layout.py b/qiskit/transpiler/passes/layout/apply_layout.py index 9cbedcef5ab3..629dc32061fc 100644 --- a/qiskit/transpiler/passes/layout/apply_layout.py +++ b/qiskit/transpiler/passes/layout/apply_layout.py @@ -13,7 +13,7 @@ """Transform a circuit with virtual qubits into a circuit with physical qubits.""" from qiskit.circuit import QuantumRegister -from qiskit.dagcircuit import DAGCircuit +from qiskit.dagcircuit import DAGCircuit, DAGOpNode from qiskit.transpiler.basepasses import TransformationPass from qiskit.transpiler.exceptions import TranspilerError from qiskit.transpiler.layout import Layout @@ -79,7 +79,12 @@ def run(self, dag): virtual_physical_map = layout.get_virtual_bits() for node in dag.topological_op_nodes(): qargs = [q[virtual_physical_map[qarg]] for qarg in node.qargs] - new_dag.apply_operation_back(node.op, qargs, node.cargs, check=False) + new_dag._apply_op_node_back( + DAGOpNode.from_instruction( + node._to_circuit_instruction().replace(qubits=qargs) + ), + check=False, + ) else: # First build a new layout object going from: # old virtual -> old physical -> new virtual -> new physical @@ -99,7 +104,12 @@ def run(self, dag): # Apply new layout to the circuit for node in dag.topological_op_nodes(): qargs = [q[new_virtual_to_physical[qarg]] for qarg in node.qargs] - new_dag.apply_operation_back(node.op, qargs, node.cargs, check=False) + new_dag._apply_op_node_back( + DAGOpNode.from_instruction( + node._to_circuit_instruction().replace(qubits=qargs) + ), + check=False, + ) self.property_set["layout"] = full_layout if (final_layout := self.property_set["final_layout"]) is not None: final_layout_mapping = { diff --git a/qiskit/transpiler/passes/layout/vf2_utils.py b/qiskit/transpiler/passes/layout/vf2_utils.py index c5d420127f88..037ccc37155d 100644 --- a/qiskit/transpiler/passes/layout/vf2_utils.py +++ b/qiskit/transpiler/passes/layout/vf2_utils.py @@ -19,7 +19,7 @@ import numpy as np from rustworkx import PyDiGraph, PyGraph, connected_components -from qiskit.circuit import ControlFlowOp, ForLoopOp +from qiskit.circuit import ForLoopOp from qiskit.converters import circuit_to_dag from qiskit._accelerate import vf2_layout from qiskit._accelerate.nlayout import NLayout @@ -37,7 +37,7 @@ class MultiQEncountered(Exception): def _visit(dag, weight, wire_map): for node in dag.op_nodes(include_directives=False): - if isinstance(node.op, ControlFlowOp): + if node.is_control_flow(): if isinstance(node.op, ForLoopOp): inner_weight = len(node.op.params[0]) * weight else: @@ -57,7 +57,7 @@ def _visit(dag, weight, wire_map): im_graph_node_map[qargs[0]] = im_graph.add_node(weights) reverse_im_graph_node_map[im_graph_node_map[qargs[0]]] = qargs[0] else: - im_graph[im_graph_node_map[qargs[0]]][node.op.name] += weight + im_graph[im_graph_node_map[qargs[0]]][node.name] += weight if len_args == 2: if qargs[0] not in im_graph_node_map: im_graph_node_map[qargs[0]] = im_graph.add_node(defaultdict(int)) diff --git a/qiskit/transpiler/passes/routing/sabre_swap.py b/qiskit/transpiler/passes/routing/sabre_swap.py index acb23f39ab09..e3aa723b65b2 100644 --- a/qiskit/transpiler/passes/routing/sabre_swap.py +++ b/qiskit/transpiler/passes/routing/sabre_swap.py @@ -18,7 +18,7 @@ import rustworkx -from qiskit.circuit import SwitchCaseOp, ControlFlowOp, Clbit, ClassicalRegister +from qiskit.circuit import SwitchCaseOp, Clbit, ClassicalRegister from qiskit.circuit.library.standard_gates import SwapGate from qiskit.circuit.controlflow import condition_resources, node_resources from qiskit.converters import dag_to_circuit @@ -28,7 +28,7 @@ from qiskit.transpiler.layout import Layout from qiskit.transpiler.target import Target from qiskit.transpiler.passes.layout import disjoint_utils -from qiskit.dagcircuit import DAGCircuit +from qiskit.dagcircuit import DAGCircuit, DAGOpNode from qiskit.utils.parallel import CPU_COUNT from qiskit._accelerate.sabre import ( @@ -289,9 +289,9 @@ def process_dag(block_dag, wire_map): node_blocks = {} for node in block_dag.topological_op_nodes(): cargs_bits = set(node.cargs) - if node.op.condition is not None: - cargs_bits.update(condition_resources(node.op.condition).clbits) - if isinstance(node.op, SwitchCaseOp): + if node.condition is not None: + cargs_bits.update(condition_resources(node.condition).clbits) + if node.is_control_flow() and isinstance(node.op, SwitchCaseOp): target = node.op.target if isinstance(target, Clbit): cargs_bits.add(target) @@ -300,7 +300,7 @@ def process_dag(block_dag, wire_map): else: # Expr cargs_bits.update(node_resources(target).clbits) cargs = {block_dag.find_bit(x).index for x in cargs_bits} - if isinstance(node.op, ControlFlowOp): + if node.is_control_flow(): node_blocks[node._node_id] = [ recurse( block, @@ -313,7 +313,7 @@ def process_dag(block_dag, wire_map): node._node_id, [wire_map[x] for x in node.qargs], cargs, - getattr(node.op, "_directive", False), + node.is_directive(), ) ) return SabreDAG(num_physical_qubits, block_dag.num_clbits(), dag_list, node_blocks) @@ -383,14 +383,15 @@ def recurse(dest_dag, source_dag, result, root_logical_map, layout): node = source_dag._multi_graph[node_id] if node_id in swap_map: apply_swaps(dest_dag, swap_map[node_id], layout) - if not isinstance(node.op, ControlFlowOp): - dest_dag.apply_operation_back( - node.op, - [ - physical_qubits[layout.virtual_to_physical(root_logical_map[q])] - for q in node.qargs - ], - node.cargs, + if not node.is_control_flow(): + qubits = [ + physical_qubits[layout.virtual_to_physical(root_logical_map[q])] + for q in node.qargs + ] + dest_dag._apply_op_node_back( + DAGOpNode.from_instruction( + node._to_circuit_instruction().replace(qubits=qubits) + ), check=False, ) continue diff --git a/qiskit/transpiler/passes/synthesis/high_level_synthesis.py b/qiskit/transpiler/passes/synthesis/high_level_synthesis.py index f1de2b8f2136..d8ca3e35c524 100644 --- a/qiskit/transpiler/passes/synthesis/high_level_synthesis.py +++ b/qiskit/transpiler/passes/synthesis/high_level_synthesis.py @@ -168,7 +168,7 @@ from qiskit.converters import circuit_to_dag, dag_to_circuit from qiskit.transpiler.basepasses import TransformationPass from qiskit.circuit.quantumcircuit import QuantumCircuit -from qiskit.circuit import ControlFlowOp, ControlledGate, EquivalenceLibrary +from qiskit.circuit import ControlledGate, EquivalenceLibrary from qiskit.circuit.library import LinearFunction from qiskit.transpiler.passes.utils import control_flow from qiskit.transpiler.target import Target @@ -415,11 +415,11 @@ def run(self, dag: DAGCircuit) -> DAGCircuit: dag_op_nodes = dag.op_nodes() for node in dag_op_nodes: - if isinstance(node.op, ControlFlowOp): + if node.is_control_flow(): node.op = control_flow.map_blocks(self.run, node.op) continue - if getattr(node.op, "_directive", False): + if node.is_directive(): continue if dag.has_calibration_for(node) or len(node.qargs) < self._min_qubits: @@ -429,6 +429,9 @@ def run(self, dag: DAGCircuit) -> DAGCircuit: [dag.find_bit(x).index for x in node.qargs] if self._use_qubit_indices else None ) + if self._definitely_skip_node(node): + continue + decomposition, modified = self._recursively_handle_op(node.op, qubits) if not modified: @@ -445,6 +448,17 @@ def run(self, dag: DAGCircuit) -> DAGCircuit: return dag + def _definitely_skip_node(self, node) -> bool: + """Fast-path determination of whether a node can certainly be skipped (i.e. nothing will + attempt to synthesise it). + + This is tightly coupled to `_recursively_handle_op`; it exists as a temporary measure to + avoid Python-space `Operation` creation from a `DAGOpNode` if we wouldn't do anything to the + node (which is _most_ nodes).""" + # In general, Rust-space standard gates aren't going to need synthesising (but check to be + # sure). + return node.is_standard_gate() and not self._methods_to_try(node.name) + def _recursively_handle_op( self, op: Operation, qubits: Optional[List] = None ) -> Tuple[Union[QuantumCircuit, DAGCircuit, Operation], bool]: @@ -472,6 +486,9 @@ def _recursively_handle_op( an annotated operation. """ + # WARNING: if adding new things in here, ensure that `_definitely_skip_node` is also + # up-to-date. + # Try to apply plugin mechanism decomposition = self._synthesize_op_using_plugins(op, qubits) if decomposition is not None: @@ -521,6 +538,22 @@ def _recursively_handle_op( dag = self.run(dag) return dag, True + def _methods_to_try(self, name: str): + """Get a sequence of methods to try for a given op name.""" + if (methods := self.hls_config.methods.get(name)) is not None: + # the operation's name appears in the user-provided config, + # we use the list of methods provided by the user + return methods + if ( + self.hls_config.use_default_on_unspecified + and "default" in self.hls_plugin_manager.method_names(name) + ): + # the operation's name does not appear in the user-specified config, + # we use the "default" method when instructed to do so and the "default" + # method is available + return ["default"] + return [] + def _synthesize_op_using_plugins( self, op: Operation, qubits: List ) -> Union[QuantumCircuit, None]: @@ -531,25 +564,10 @@ def _synthesize_op_using_plugins( """ hls_plugin_manager = self.hls_plugin_manager - if op.name in self.hls_config.methods.keys(): - # the operation's name appears in the user-provided config, - # we use the list of methods provided by the user - methods = self.hls_config.methods[op.name] - elif ( - self.hls_config.use_default_on_unspecified - and "default" in hls_plugin_manager.method_names(op.name) - ): - # the operation's name does not appear in the user-specified config, - # we use the "default" method when instructed to do so and the "default" - # method is available - methods = ["default"] - else: - methods = [] - best_decomposition = None best_score = np.inf - for method in methods: + for method in self._methods_to_try(op.name): # There are two ways to specify a synthesis method. The more explicit # way is to specify it as a tuple consisting of a synthesis algorithm and a # list of additional arguments, e.g., diff --git a/qiskit/transpiler/passes/utils/check_map.py b/qiskit/transpiler/passes/utils/check_map.py index 437718ec27b4..bd78c65de5f4 100644 --- a/qiskit/transpiler/passes/utils/check_map.py +++ b/qiskit/transpiler/passes/utils/check_map.py @@ -14,7 +14,6 @@ from qiskit.transpiler.basepasses import AnalysisPass from qiskit.transpiler.target import Target -from qiskit.circuit.controlflow import ControlFlowOp from qiskit.converters import circuit_to_dag @@ -73,7 +72,7 @@ def run(self, dag): def _recurse(self, dag, wire_map) -> bool: for node in dag.op_nodes(include_directives=False): - if isinstance(node.op, ControlFlowOp): + if node.is_control_flow(): for block in node.op.blocks: inner_wire_map = { inner: wire_map[outer] for inner, outer in zip(block.qubits, node.qargs) diff --git a/qiskit/transpiler/passes/utils/convert_conditions_to_if_ops.py b/qiskit/transpiler/passes/utils/convert_conditions_to_if_ops.py index 46d91f7d6ce0..a73f9690ae0e 100644 --- a/qiskit/transpiler/passes/utils/convert_conditions_to_if_ops.py +++ b/qiskit/transpiler/passes/utils/convert_conditions_to_if_ops.py @@ -17,7 +17,6 @@ CircuitInstruction, ClassicalRegister, Clbit, - ControlFlowOp, IfElseOp, QuantumCircuit, ) @@ -37,7 +36,7 @@ def _run_inner(self, dag): was modified and ``False`` if not.""" modified = False for node in dag.op_nodes(): - if isinstance(node.op, ControlFlowOp): + if node.is_control_flow(): modified_blocks = False new_dags = [] for block in node.op.blocks: @@ -51,7 +50,7 @@ def _run_inner(self, dag): node.op.replace_blocks(dag_to_circuit(block) for block in new_dags), inplace=True, ) - elif getattr(node.op, "condition", None) is None: + elif node.condition is None: continue else: target, value = node.op.condition diff --git a/qiskit/transpiler/passes/utils/gates_basis.py b/qiskit/transpiler/passes/utils/gates_basis.py index b1f004cc0df3..16a68c3e533e 100644 --- a/qiskit/transpiler/passes/utils/gates_basis.py +++ b/qiskit/transpiler/passes/utils/gates_basis.py @@ -12,7 +12,6 @@ """Check if all gates in the DAGCircuit are in the specified basis gates.""" -from qiskit.circuit import ControlFlowOp from qiskit.converters import circuit_to_dag from qiskit.transpiler.basepasses import AnalysisPass @@ -55,7 +54,7 @@ def _visit_target(dag, wire_map): return True # Control-flow ops still need to be supported, so don't skip them in the # previous checks. - if isinstance(gate.op, ControlFlowOp): + if gate.is_control_flow(): for block in gate.op.blocks: inner_wire_map = { inner: wire_map[outer] diff --git a/qiskit/transpiler/preset_passmanagers/common.py b/qiskit/transpiler/preset_passmanagers/common.py index ec479f9e006b..b0e9eae57b03 100644 --- a/qiskit/transpiler/preset_passmanagers/common.py +++ b/qiskit/transpiler/preset_passmanagers/common.py @@ -364,10 +364,7 @@ def _swap_condition(property_set): routing.append(ConditionalController(ApplyLayout(), condition=_apply_post_layout_condition)) def filter_fn(node): - return ( - getattr(node.op, "label", None) - != "qiskit.transpiler.internal.routing.protection.barrier" - ) + return node.label != "qiskit.transpiler.internal.routing.protection.barrier" routing.append([FilterOpNodes(filter_fn)]) diff --git a/releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml b/releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml new file mode 100644 index 000000000000..79fcdb26f143 --- /dev/null +++ b/releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml @@ -0,0 +1,16 @@ +--- +features_circuits: + - | + :class:`.CircuitInstruction` and :class:`.DAGOpNode` each have new methods to query various + properties of their internal :class:`.Operation`, without necessarily needing to access it. + These methods are: + + * :meth:`.CircuitInstruction.is_standard_gate` and :meth:`.DAGOpNode.is_standard_gate`, + * :meth:`.CircuitInstruction.is_directive` and :meth:`.DAGOpNode.is_directive`, + * :meth:`.CircuitInstruction.is_control_flow` and :meth:`.DAGOpNode.is_control_flow`, and + * :meth:`.CircuitInstruction.is_parameterized` and :meth:`.DAGOpNode.is_parameterized`. + + If applicable, using any of these methods is significantly faster than querying + :attr:`.CircuitInstruction.operation` or :attr:`.DAGOpNode.op` directly, especially if the + instruction or node represents a Qiskit standard gate. This is because the standard gates are + stored natively in Rust, and their Python representation is only created when requested. From f3a10a293e49f238054169fb25d7f5661f185aef Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Mon, 29 Jul 2024 18:26:18 +0100 Subject: [PATCH 2/2] Fix `HighLevelSynthesis` fast path --- crates/circuit/src/circuit_instruction.rs | 17 ++++- crates/circuit/src/dag_node.rs | 5 ++ .../passes/synthesis/high_level_synthesis.py | 64 +++++++++++++------ .../avoid-op-creation-804c0bed6c408911.yaml | 1 + 4 files changed, 67 insertions(+), 20 deletions(-) diff --git a/crates/circuit/src/circuit_instruction.rs b/crates/circuit/src/circuit_instruction.rs index 9cc763cbd35c..d44051745a89 100644 --- a/crates/circuit/src/circuit_instruction.rs +++ b/crates/circuit/src/circuit_instruction.rs @@ -22,7 +22,9 @@ use pyo3::{intern, IntoPy, PyObject, PyResult}; use smallvec::SmallVec; -use crate::imports::{CONTROL_FLOW_OP, GATE, INSTRUCTION, OPERATION, WARNINGS_WARN}; +use crate::imports::{ + CONTROLLED_GATE, CONTROL_FLOW_OP, GATE, INSTRUCTION, OPERATION, WARNINGS_WARN, +}; use crate::operations::{ Operation, OperationRef, Param, PyGate, PyInstruction, PyOperation, StandardGate, }; @@ -271,6 +273,19 @@ impl CircuitInstruction { self.operation.try_standard_gate().is_some() } + /// Is the :class:`.Operation` contained in this instruction a subclass of + /// :class:`.ControlledGate`? + pub fn is_controlled_gate(&self, py: Python) -> PyResult { + match self.operation.view() { + OperationRef::Standard(standard) => Ok(standard.num_ctrl_qubits() != 0), + OperationRef::Gate(gate) => gate + .gate + .bind(py) + .is_instance(CONTROLLED_GATE.get_bound(py)), + _ => Ok(false), + } + } + /// Is the :class:`.Operation` contained in this node a directive? pub fn is_directive(&self) -> bool { self.op().directive() diff --git a/crates/circuit/src/dag_node.rs b/crates/circuit/src/dag_node.rs index 4b6c1b7c0bf0..73983c35e2e8 100644 --- a/crates/circuit/src/dag_node.rs +++ b/crates/circuit/src/dag_node.rs @@ -334,6 +334,11 @@ impl DAGOpNode { self.instruction.is_standard_gate() } + /// Is the :class:`.Operation` contained in this node a subclass of :class:`.ControlledGate`? + pub fn is_controlled_gate(&self, py: Python) -> PyResult { + self.instruction.is_controlled_gate(py) + } + /// Is the :class:`.Operation` contained in this node a directive? pub fn is_directive(&self) -> bool { self.instruction.is_directive() diff --git a/qiskit/transpiler/passes/synthesis/high_level_synthesis.py b/qiskit/transpiler/passes/synthesis/high_level_synthesis.py index d8ca3e35c524..6974b1cce06c 100644 --- a/qiskit/transpiler/passes/synthesis/high_level_synthesis.py +++ b/qiskit/transpiler/passes/synthesis/high_level_synthesis.py @@ -159,7 +159,10 @@ QFTSynthesisLine """ -from typing import Optional, Union, List, Tuple, Callable +from __future__ import annotations + +import typing +from typing import Optional, Union, List, Tuple, Callable, Sequence import numpy as np import rustworkx as rx @@ -168,7 +171,7 @@ from qiskit.converters import circuit_to_dag, dag_to_circuit from qiskit.transpiler.basepasses import TransformationPass from qiskit.circuit.quantumcircuit import QuantumCircuit -from qiskit.circuit import ControlledGate, EquivalenceLibrary +from qiskit.circuit import ControlledGate, EquivalenceLibrary, equivalence from qiskit.circuit.library import LinearFunction from qiskit.transpiler.passes.utils import control_flow from qiskit.transpiler.target import Target @@ -210,6 +213,9 @@ from .plugin import HighLevelSynthesisPluginManager, HighLevelSynthesisPlugin +if typing.TYPE_CHECKING: + from qiskit.dagcircuit import DAGOpNode + class HLSConfig: """The high-level-synthesis config allows to specify a list of "methods" used by @@ -396,6 +402,8 @@ def __init__( if not self._top_level_only and (self._target is None or self._target.num_qubits is None): basic_insts = {"measure", "reset", "barrier", "snapshot", "delay", "store"} self._device_insts = basic_insts | set(self._basis_gates) + else: + self._device_insts = set() def run(self, dag: DAGCircuit) -> DAGCircuit: """Run the HighLevelSynthesis pass on `dag`. @@ -429,7 +437,7 @@ def run(self, dag: DAGCircuit) -> DAGCircuit: [dag.find_bit(x).index for x in node.qargs] if self._use_qubit_indices else None ) - if self._definitely_skip_node(node): + if self._definitely_skip_node(node, qubits): continue decomposition, modified = self._recursively_handle_op(node.op, qubits) @@ -448,16 +456,42 @@ def run(self, dag: DAGCircuit) -> DAGCircuit: return dag - def _definitely_skip_node(self, node) -> bool: + def _definitely_skip_node(self, node: DAGOpNode, qubits: Sequence[int] | None) -> bool: """Fast-path determination of whether a node can certainly be skipped (i.e. nothing will - attempt to synthesise it). + attempt to synthesise it) without accessing its Python-space `Operation`. This is tightly coupled to `_recursively_handle_op`; it exists as a temporary measure to avoid Python-space `Operation` creation from a `DAGOpNode` if we wouldn't do anything to the node (which is _most_ nodes).""" - # In general, Rust-space standard gates aren't going to need synthesising (but check to be - # sure). - return node.is_standard_gate() and not self._methods_to_try(node.name) + return ( + # The fast path is just for Rust-space standard gates (which excludes + # `AnnotatedOperation`). + node.is_standard_gate() + # If it's a controlled gate, we might choose to do funny things to it. + and not node.is_controlled_gate() + # If there are plugins to try, they need to be tried. + and not self._methods_to_try(node.name) + # If all the above constraints hold, and it's already supported or the basis translator + # can handle it, we'll leave it be. + and ( + self._instruction_supported(node.name, qubits) + # This uses unfortunately private details of `EquivalenceLibrary`, but so does the + # `BasisTranslator`, and this is supposed to just be temporary til this is moved + # into Rust space. + or ( + self._equiv_lib is not None + and equivalence.Key(name=node.name, num_qubits=node.num_qubits) + in self._equiv_lib._key_to_node_index + ) + ) + ) + + def _instruction_supported(self, name: str, qubits: Sequence[int]) -> bool: + qubits = tuple(qubits) if qubits is not None else None + # include path for when target exists but target.num_qubits is None (BasicSimulator) + if self._target is None or self._target.num_qubits is None: + return name in self._device_insts + return self._target.instruction_supported(operation_name=name, qargs=qubits) def _recursively_handle_op( self, op: Operation, qubits: Optional[List] = None @@ -507,17 +541,9 @@ def _recursively_handle_op( # or is in equivalence library controlled_gate_open_ctrl = isinstance(op, ControlledGate) and op._open_ctrl if not controlled_gate_open_ctrl: - qargs = tuple(qubits) if qubits is not None else None - # include path for when target exists but target.num_qubits is None (BasicSimulator) - inst_supported = ( - self._target.instruction_supported( - operation_name=op.name, - qargs=qargs, - ) - if self._target is not None and self._target.num_qubits is not None - else op.name in self._device_insts - ) - if inst_supported or (self._equiv_lib is not None and self._equiv_lib.has_entry(op)): + if self._instruction_supported(op.name, qubits) or ( + self._equiv_lib is not None and self._equiv_lib.has_entry(op) + ): return op, False try: diff --git a/releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml b/releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml index 79fcdb26f143..c7f63a45f8cc 100644 --- a/releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml +++ b/releasenotes/notes/avoid-op-creation-804c0bed6c408911.yaml @@ -6,6 +6,7 @@ features_circuits: These methods are: * :meth:`.CircuitInstruction.is_standard_gate` and :meth:`.DAGOpNode.is_standard_gate`, + * :meth:`.CircuitInstruction.is_controlled_gate` and :meth:`.DAGOpNode.is_controlled_gate`, * :meth:`.CircuitInstruction.is_directive` and :meth:`.DAGOpNode.is_directive`, * :meth:`.CircuitInstruction.is_control_flow` and :meth:`.DAGOpNode.is_control_flow`, and * :meth:`.CircuitInstruction.is_parameterized` and :meth:`.DAGOpNode.is_parameterized`.