Skip to content

Commit

Permalink
Avoid operator creation in transpiler
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jakelishman committed Jul 26, 2024
1 parent edf0b53 commit 991263a
Show file tree
Hide file tree
Showing 17 changed files with 182 additions and 100 deletions.
17 changes: 15 additions & 2 deletions crates/circuit/src/circuit_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
22 changes: 17 additions & 5 deletions crates/circuit/src/dag_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PyObject> {
let matrix = self.instruction.op().matrix(&self.instruction.params);
Expand Down Expand Up @@ -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<String>) {
match self.instruction.extra_attrs.as_mut() {
Expand Down
2 changes: 2 additions & 0 deletions crates/circuit/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
3 changes: 2 additions & 1 deletion crates/circuit/src/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2016,6 +2016,7 @@ pub struct PyInstruction {
pub clbits: u32,
pub params: u32,
pub op_name: String,
pub control_flow: bool,
pub instruction: PyObject,
}

Expand All @@ -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<Array2<Complex64>> {
None
Expand Down
2 changes: 2 additions & 0 deletions crates/circuit/src/packed_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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()),
Expand Down
4 changes: 2 additions & 2 deletions qiskit/circuit/commutation_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
85 changes: 49 additions & 36 deletions qiskit/dagcircuit/dagcircuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)}")

Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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))
)


Expand Down
6 changes: 3 additions & 3 deletions qiskit/transpiler/passes/basis/basis_translator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand All @@ -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):
Expand Down Expand Up @@ -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]
Expand Down
16 changes: 13 additions & 3 deletions qiskit/transpiler/passes/layout/apply_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 = {
Expand Down
6 changes: 3 additions & 3 deletions qiskit/transpiler/passes/layout/vf2_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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))
Expand Down
Loading

0 comments on commit 991263a

Please sign in to comment.