Skip to content

Commit

Permalink
Enable avoiding Python operation creation in transpiler (Qiskit#12692)
Browse files Browse the repository at this point in the history
* Avoid Python operation creation in transpiler

Since Qiskit#12459 accessing `node.op` in the transpiler eagerly creates a
Python object on access. This is because we now are no longer storing a
Python object internally and we need to rebuild the object to return the
python object as expected by the api. This is causing a significant
performance regression because of the extra overhead. The longer term
goal is to move as much of the performance critical passes to operate in
rust which will eliminate this overhead. But in the meantime we can
mitigate the performance overhead by changing the Python access patterns
to avoid the operation object creation. This commit adds some new getter
methods to DAGOpNode to give access to the inner rust data so that we
can avoid the extra overhead. As a proof of concept this updates the
unitary synthesis pass in isolation. Doing this fixes the regression
caused by Qiskit#12459 for that pass. We can continue this migration for
everything else in follow up PRs. This commit is mostly to establish the
pattern and add the python space access methods.

* Remove unused import

* Add path to avoid StandardGate conversion in circuit_to_dag

* Add fast path through dag_to_circuit
  • Loading branch information
mtreinish authored and Procatv committed Aug 1, 2024
1 parent 06e809c commit 8bcfca3
Show file tree
Hide file tree
Showing 10 changed files with 313 additions and 67 deletions.
11 changes: 4 additions & 7 deletions crates/circuit/src/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::circuit_instruction::{
convert_py_to_operation_type, CircuitInstruction, ExtraInstructionAttributes, OperationInput,
PackedInstruction,
};
use crate::imports::{BUILTIN_LIST, QUBIT};
use crate::imports::{BUILTIN_LIST, DEEPCOPY, QUBIT};
use crate::interner::{IndexedInterner, Interner, InternerKey};
use crate::operations::{Operation, OperationType, Param, StandardGate};
use crate::parameter_table::{ParamEntry, ParamTable, GLOBAL_PHASE_INDEX};
Expand Down Expand Up @@ -488,20 +488,17 @@ impl CircuitData {
res.param_table.clone_from(&self.param_table);

if deepcopy {
let deepcopy = py
.import_bound(intern!(py, "copy"))?
.getattr(intern!(py, "deepcopy"))?;
for inst in &mut res.data {
match &mut inst.op {
OperationType::Standard(_) => {}
OperationType::Gate(ref mut op) => {
op.gate = deepcopy.call1((&op.gate,))?.unbind();
op.gate = DEEPCOPY.get_bound(py).call1((&op.gate,))?.unbind();
}
OperationType::Instruction(ref mut op) => {
op.instruction = deepcopy.call1((&op.instruction,))?.unbind();
op.instruction = DEEPCOPY.get_bound(py).call1((&op.instruction,))?.unbind();
}
OperationType::Operation(ref mut op) => {
op.operation = deepcopy.call1((&op.operation,))?.unbind();
op.operation = DEEPCOPY.get_bound(py).call1((&op.operation,))?.unbind();
}
};
#[cfg(feature = "cache_pygates")]
Expand Down
55 changes: 54 additions & 1 deletion crates/circuit/src/circuit_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#[cfg(feature = "cache_pygates")]
use std::cell::RefCell;

use numpy::IntoPyArray;
use pyo3::basic::CompareOp;
use pyo3::exceptions::{PyDeprecationWarning, PyValueError};
use pyo3::prelude::*;
Expand All @@ -25,7 +26,9 @@ use crate::imports::{
SINGLETON_CONTROLLED_GATE, SINGLETON_GATE, WARNINGS_WARN,
};
use crate::interner::Index;
use crate::operations::{OperationType, Param, PyGate, PyInstruction, PyOperation, StandardGate};
use crate::operations::{
Operation, OperationType, Param, PyGate, PyInstruction, PyOperation, StandardGate,
};

/// These are extra mutable attributes for a circuit instruction's state. In general we don't
/// typically deal with this in rust space and the majority of the time they're not used in Python
Expand Down Expand Up @@ -407,6 +410,56 @@ impl CircuitInstruction {
})
}

#[getter]
fn _raw_op(&self, py: Python) -> PyObject {
self.operation.clone().into_py(py)
}

/// Returns the Instruction name corresponding to the op for this node
#[getter]
fn get_name(&self, py: Python) -> PyObject {
self.operation.name().to_object(py)
}

#[getter]
fn get_params(&self, py: Python) -> PyObject {
self.params.to_object(py)
}

#[getter]
fn matrix(&self, py: Python) -> Option<PyObject> {
let matrix = self.operation.matrix(&self.params);
matrix.map(|mat| mat.into_pyarray_bound(py).into())
}

#[getter]
fn label(&self) -> Option<&str> {
self.extra_attrs
.as_ref()
.and_then(|attrs| attrs.label.as_deref())
}

#[getter]
fn condition(&self, py: Python) -> Option<PyObject> {
self.extra_attrs
.as_ref()
.and_then(|attrs| attrs.condition.as_ref().map(|x| x.clone_ref(py)))
}

#[getter]
fn duration(&self, py: Python) -> Option<PyObject> {
self.extra_attrs
.as_ref()
.and_then(|attrs| attrs.duration.as_ref().map(|x| x.clone_ref(py)))
}

#[getter]
fn unit(&self) -> Option<&str> {
self.extra_attrs
.as_ref()
.and_then(|attrs| attrs.unit.as_deref())
}

/// Creates a shallow copy with the given fields replaced.
///
/// Returns:
Expand Down
119 changes: 93 additions & 26 deletions crates/circuit/src/dag_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ use crate::circuit_instruction::{
ExtraInstructionAttributes,
};
use crate::operations::Operation;
use numpy::IntoPyArray;
use pyo3::prelude::*;
use pyo3::types::{PyDict, PyList, PySequence, PyString, PyTuple};
use pyo3::{intern, PyObject, PyResult};
use pyo3::{intern, IntoPy, PyObject, PyResult};
use smallvec::smallvec;

/// Parent class for DAGOpNode, DAGInNode, and DAGOutNode.
#[pyclass(module = "qiskit._accelerate.circuit", subclass)]
Expand Down Expand Up @@ -70,12 +72,19 @@ pub struct DAGOpNode {

#[pymethods]
impl DAGOpNode {
#[allow(clippy::too_many_arguments)]
#[new]
#[pyo3(signature = (op, qargs=None, cargs=None, params=smallvec![], label=None, duration=None, unit=None, condition=None, dag=None))]
fn new(
py: Python,
op: PyObject,
op: crate::circuit_instruction::OperationInput,
qargs: Option<&Bound<PySequence>>,
cargs: Option<&Bound<PySequence>>,
params: smallvec::SmallVec<[crate::operations::Param; 3]>,
label: Option<String>,
duration: Option<PyObject>,
unit: Option<String>,
condition: Option<PyObject>,
dag: Option<&Bound<PyAny>>,
) -> PyResult<(Self, DAGNode)> {
let qargs =
Expand Down Expand Up @@ -110,34 +119,16 @@ impl DAGOpNode {
}
None => qargs.str()?.into_any(),
};
let res = convert_py_to_operation_type(py, op.clone_ref(py))?;

let extra_attrs = if res.label.is_some()
|| res.duration.is_some()
|| res.unit.is_some()
|| res.condition.is_some()
{
Some(Box::new(ExtraInstructionAttributes {
label: res.label,
duration: res.duration,
unit: res.unit,
condition: res.condition,
}))
} else {
None
};
let mut instruction = CircuitInstruction::py_new(
py, op, None, None, params, label, duration, unit, condition,
)?;
instruction.qubits = qargs.into();
instruction.clbits = cargs.into();

Ok((
DAGOpNode {
instruction: CircuitInstruction {
operation: res.operation,
qubits: qargs.unbind(),
clbits: cargs.unbind(),
params: res.params,
extra_attrs,
#[cfg(feature = "cache_pygates")]
py_op: Some(op),
},
instruction,
sort_key: sort_key.unbind(),
},
DAGNode { _node_id: -1 },
Expand Down Expand Up @@ -219,6 +210,77 @@ impl DAGOpNode {
self.instruction.operation.name().to_object(py)
}

#[getter]
fn get_params(&self, py: Python) -> PyObject {
self.instruction.params.to_object(py)
}

#[getter]
fn matrix(&self, py: Python) -> Option<PyObject> {
let matrix = self.instruction.operation.matrix(&self.instruction.params);
matrix.map(|mat| mat.into_pyarray_bound(py).into())
}

#[getter]
fn label(&self) -> Option<&str> {
self.instruction
.extra_attrs
.as_ref()
.and_then(|attrs| attrs.label.as_deref())
}

#[getter]
fn condition(&self, py: Python) -> Option<PyObject> {
self.instruction
.extra_attrs
.as_ref()
.and_then(|attrs| attrs.condition.as_ref().map(|x| x.clone_ref(py)))
}

#[getter]
fn duration(&self, py: Python) -> Option<PyObject> {
self.instruction
.extra_attrs
.as_ref()
.and_then(|attrs| attrs.duration.as_ref().map(|x| x.clone_ref(py)))
}

#[getter]
fn unit(&self) -> Option<&str> {
self.instruction
.extra_attrs
.as_ref()
.and_then(|attrs| attrs.unit.as_deref())
}

#[setter]
fn set_label(&mut self, val: Option<String>) {
match self.instruction.extra_attrs.as_mut() {
Some(attrs) => attrs.label = val,
None => {
if val.is_some() {
self.instruction.extra_attrs = Some(Box::new(
crate::circuit_instruction::ExtraInstructionAttributes {
label: val,
duration: None,
unit: None,
condition: None,
},
))
}
}
};
if let Some(attrs) = &self.instruction.extra_attrs {
if attrs.label.is_none()
&& attrs.duration.is_none()
&& attrs.unit.is_none()
&& attrs.condition.is_none()
{
self.instruction.extra_attrs = None;
}
}
}

/// Sets the Instruction name corresponding to the op for this node
#[setter]
fn set_name(&mut self, py: Python, new_name: PyObject) -> PyResult<()> {
Expand All @@ -229,6 +291,11 @@ impl DAGOpNode {
Ok(())
}

#[getter]
fn _raw_op(&self, py: Python) -> PyObject {
self.instruction.operation.clone().into_py(py)
}

/// Returns a representation of the DAGOpNode
fn __repr__(&self, py: Python) -> PyResult<String> {
Ok(format!(
Expand Down
1 change: 1 addition & 0 deletions crates/circuit/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub static SINGLETON_GATE: ImportOnceCell =
ImportOnceCell::new("qiskit.circuit.singleton", "SingletonGate");
pub static SINGLETON_CONTROLLED_GATE: ImportOnceCell =
ImportOnceCell::new("qiskit.circuit.singleton", "SingletonControlledGate");
pub static DEEPCOPY: ImportOnceCell = ImportOnceCell::new("copy", "deepcopy");

pub static WARNINGS_WARN: ImportOnceCell = ImportOnceCell::new("warnings", "warn");

Expand Down
43 changes: 42 additions & 1 deletion crates/circuit/src/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use std::f64::consts::PI;

use crate::circuit_data::CircuitData;
use crate::imports::{PARAMETER_EXPRESSION, QUANTUM_CIRCUIT};
use crate::imports::{DEEPCOPY, PARAMETER_EXPRESSION, QUANTUM_CIRCUIT};
use crate::{gate_matrix, Qubit};

use ndarray::{aview2, Array2};
Expand All @@ -35,6 +35,17 @@ pub enum OperationType {
Operation(PyOperation),
}

impl IntoPy<PyObject> for OperationType {
fn into_py(self, py: Python) -> PyObject {
match self {
Self::Standard(gate) => gate.into_py(py),
Self::Instruction(inst) => inst.into_py(py),
Self::Gate(gate) => gate.into_py(py),
Self::Operation(op) => op.into_py(py),
}
}
}

impl Operation for OperationType {
fn name(&self) -> &str {
match self {
Expand Down Expand Up @@ -1418,6 +1429,16 @@ impl PyInstruction {
instruction,
}
}

fn __deepcopy__(&self, py: Python, _memo: PyObject) -> PyResult<Self> {
Ok(PyInstruction {
qubits: self.qubits,
clbits: self.clbits,
params: self.params,
op_name: self.op_name.clone(),
instruction: DEEPCOPY.get_bound(py).call1((&self.instruction,))?.unbind(),
})
}
}

impl Operation for PyInstruction {
Expand Down Expand Up @@ -1497,6 +1518,16 @@ impl PyGate {
gate,
}
}

fn __deepcopy__(&self, py: Python, _memo: PyObject) -> PyResult<Self> {
Ok(PyGate {
qubits: self.qubits,
clbits: self.clbits,
params: self.params,
op_name: self.op_name.clone(),
gate: DEEPCOPY.get_bound(py).call1((&self.gate,))?.unbind(),
})
}
}

impl Operation for PyGate {
Expand Down Expand Up @@ -1589,6 +1620,16 @@ impl PyOperation {
operation,
}
}

fn __deepcopy__(&self, py: Python, _memo: PyObject) -> PyResult<Self> {
Ok(PyOperation {
qubits: self.qubits,
clbits: self.clbits,
params: self.params,
op_name: self.op_name.clone(),
operation: DEEPCOPY.get_bound(py).call1((&self.operation,))?.unbind(),
})
}
}

impl Operation for PyOperation {
Expand Down
25 changes: 20 additions & 5 deletions qiskit/converters/circuit_to_dag.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"""Helper function for converting a circuit to a dag"""
import copy

from qiskit.dagcircuit.dagcircuit import DAGCircuit
from qiskit.dagcircuit.dagcircuit import DAGCircuit, DAGOpNode
from qiskit._accelerate.circuit import StandardGate


def circuit_to_dag(circuit, copy_operations=True, *, qubit_order=None, clbit_order=None):
Expand Down Expand Up @@ -93,10 +94,24 @@ def circuit_to_dag(circuit, copy_operations=True, *, qubit_order=None, clbit_ord
dagcircuit.add_creg(register)

for instruction in circuit.data:
op = instruction.operation
if copy_operations:
op = copy.deepcopy(op)
dagcircuit.apply_operation_back(op, instruction.qubits, instruction.clbits, check=False)
if not isinstance(instruction._raw_op, StandardGate):
op = instruction.operation
if copy_operations:
op = copy.deepcopy(op)
dagcircuit.apply_operation_back(op, instruction.qubits, instruction.clbits, check=False)
else:
node = DAGOpNode(
instruction._raw_op,
qargs=instruction.qubits,
cargs=instruction.clbits,
params=instruction.params,
label=instruction.label,
duration=instruction.duration,
unit=instruction.unit,
condition=instruction.condition,
dag=dagcircuit,
)
dagcircuit._apply_op_node_back(node)

dagcircuit.duration = circuit.duration
dagcircuit.unit = circuit.unit
Expand Down
Loading

0 comments on commit 8bcfca3

Please sign in to comment.