Avoid Python op creation in commutative cancellation

This commit updates the commutative cancellation and commutation
analysis transpiler pass. It builds off of Qiskit#12692 to adjust access
patterns in the python transpiler path to avoid eagerly creating a
Python space operation object. The goal of this PR is to mitigate the
performance regression on these passes introduced by the extra
conversion cost of Qiskit#12459.

crates/circuit/src/circuit_instruction.rs

@@ -15,7 +15,7 @@ use std::cell::RefCell;
 
 use numpy::IntoPyArray;
 use pyo3::basic::CompareOp;
-use pyo3::exceptions::{PyDeprecationWarning, PyValueError};
+use pyo3::exceptions::{PyDeprecationWarning, PyTypeError, PyValueError};
 use pyo3::prelude::*;
 use pyo3::types::{IntoPyDict, PyList, PyTuple, PyType};
 use pyo3::{intern, IntoPy, PyObject, PyResult};
@@ -432,6 +432,41 @@ impl CircuitInstruction {
         matrix.map(|mat| mat.into_pyarray_bound(py).into())
     }
 
+    fn to_matrix(&self, py: Python) -> Option<PyObject> {
+        self.matrix(py)
+    }
+
+    fn __array__(
+        &self,
+        py: Python,
+        dtype: Option<PyObject>,
+        copy: Option<bool>,
+    ) -> PyResult<PyObject> {
+        if copy == Some(false) {
+            return Err(PyValueError::new_err(
+                "A copy is needed to return an array from this object.",
+            ));
+        }
+        let res =
+            match self.to_matrix(py) {
+                Some(res) => res,
+                None => return Err(PyTypeError::new_err(
+                    "ParameterExpression with unbound parameters cannot be represented as an array",
+                )),
+            };
+        Ok(match dtype {
+            Some(dtype) => {
+                let numpy_mod = py.import_bound("numpy")?;
+                let args = (res,);
+                let kwargs = [("dtype", dtype)].into_py_dict_bound(py);
+                numpy_mod
+                    .call_method("asarray", args, Some(&kwargs))?
+                    .into()
+            }
+            None => res,
+        })
+    }
+
     #[getter]
     fn label(&self) -> Option<&str> {
         self.extra_attrs
@@ -460,6 +495,12 @@ impl CircuitInstruction {
             .and_then(|attrs| attrs.unit.as_deref())
     }
 
+    pub fn is_parameterized(&self) -> bool {
+        self.params
+            .iter()
+            .any(|x| matches!(x, Param::ParameterExpression(_)))
+    }
+
     /// Creates a shallow copy with the given fields replaced.
     ///
     /// Returns:

crates/circuit/src/dag_node.rs

@@ -14,10 +14,12 @@ use crate::circuit_instruction::{
     convert_py_to_operation_type, operation_type_to_py, CircuitInstruction,
     ExtraInstructionAttributes,
 };
+use crate::imports::QUANTUM_CIRCUIT;
 use crate::operations::Operation;
 use numpy::IntoPyArray;
+use pyo3::exceptions::{PyTypeError, PyValueError};
 use pyo3::prelude::*;
-use pyo3::types::{PyDict, PyList, PySequence, PyString, PyTuple};
+use pyo3::types::{IntoPyDict, PyDict, PyList, PySequence, PyString, PyTuple};
 use pyo3::{intern, IntoPy, PyObject, PyResult};
 use smallvec::smallvec;
 
@@ -184,6 +186,16 @@ impl DAGOpNode {
         Ok(())
     }
 
+    #[getter]
+    fn num_qubits(&self) -> u32 {
+        self.instruction.operation.num_qubits()
+    }
+
+    #[getter]
+    fn num_clbits(&self) -> u32 {
+        self.instruction.operation.num_clbits()
+    }
+
     #[getter]
     fn get_qargs(&self, py: Python) -> Py<PyTuple> {
         self.instruction.qubits.clone_ref(py)
@@ -215,12 +227,51 @@ impl DAGOpNode {
         self.instruction.params.to_object(py)
     }
 
+    pub fn is_parameterized(&self) -> bool {
+        self.instruction.is_parameterized()
+    }
+
     #[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())
     }
 
+    fn to_matrix(&self, py: Python) -> Option<PyObject> {
+        self.matrix(py)
+    }
+
+    fn __array__(
+        &self,
+        py: Python,
+        dtype: Option<PyObject>,
+        copy: Option<bool>,
+    ) -> PyResult<PyObject> {
+        if copy == Some(false) {
+            return Err(PyValueError::new_err(
+                "A copy is needed to return an array from this object.",
+            ));
+        }
+        let res =
+            match self.to_matrix(py) {
+                Some(res) => res,
+                None => return Err(PyTypeError::new_err(
+                    "ParameterExpression with unbound parameters cannot be represented as an array",
+                )),
+            };
+        Ok(match dtype {
+            Some(dtype) => {
+                let numpy_mod = py.import_bound("numpy")?;
+                let args = (res,);
+                let kwargs = [("dtype", dtype)].into_py_dict_bound(py);
+                numpy_mod
+                    .call_method("asarray", args, Some(&kwargs))?
+                    .into()
+            }
+            None => res,
+        })
+    }
+
     #[getter]
     fn label(&self) -> Option<&str> {
         self.instruction
@@ -281,6 +332,21 @@ impl DAGOpNode {
         }
     }
 
+    #[getter]
+    fn definition<'py>(&self, py: Python<'py>) -> PyResult<Option<Bound<'py, PyAny>>> {
+        let definition = self
+            .instruction
+            .operation
+            .definition(&self.instruction.params);
+        definition
+            .map(|data| {
+                QUANTUM_CIRCUIT
+                    .get_bound(py)
+                    .call_method1(intern!(py, "_from_circuit_data"), (data,))
+            })
+            .transpose()
+    }
+
     /// Sets the Instruction name corresponding to the op for this node
     #[setter]
     fn set_name(&mut self, py: Python, new_name: PyObject) -> PyResult<()> {

crates/circuit/src/operations.rs

@@ -709,8 +709,38 @@ impl Operation for StandardGate {
                     .expect("Unexpected Qiskit python bug"),
                 )
             }),
-            Self::RXGate => todo!("Add when we have R"),
-            Self::RYGate => todo!("Add when we have R"),
+            Self::RXGate => Python::with_gil(|py| -> Option<CircuitData> {
+                let theta = &params[0];
+                Some(
+                    CircuitData::from_standard_gates(
+                        py,
+                        1,
+                        [(
+                            Self::RGate,
+                            smallvec![theta.clone(), FLOAT_ZERO],
+                            smallvec![Qubit(0)],
+                        )],
+                        FLOAT_ZERO,
+                    )
+                    .expect("Unexpected Qiskit python bug"),
+                )
+            }),
+            Self::RYGate => Python::with_gil(|py| -> Option<CircuitData> {
+                let theta = &params[0];
+                Some(
+                    CircuitData::from_standard_gates(
+                        py,
+                        1,
+                        [(
+                            Self::RGate,
+                            smallvec![theta.clone(), Param::Float(PI / 2.0)],
+                            smallvec![Qubit(0)],
+                        )],
+                        FLOAT_ZERO,
+                    )
+                    .expect("Unexpected Qiskit python bug"),
+                )
+            }),
             Self::RZGate => Python::with_gil(|py| -> Option<CircuitData> {
                 let theta = &params[0];
                 Some(

qiskit/circuit/commutation_checker.py

@@ -21,6 +21,7 @@
 from qiskit.circuit.operation import Operation
 from qiskit.circuit.controlflow import CONTROL_FLOW_OP_NAMES
 from qiskit.quantum_info.operators import Operator
+from qiskit._accelerate.circuit import StandardGate
 
 _skipped_op_names = {"measure", "reset", "delay", "initialize"}
 _no_cache_op_names = {"annotated"}
@@ -57,6 +58,23 @@ def __init__(self, standard_gate_commutations: dict = None, cache_max_entries: i
         self._cache_miss = 0
         self._cache_hit = 0
 
+    def commute_nodes(
+        self,
+        op1,
+        op2,
+        max_num_qubits: int = 3,
+    ) -> bool:
+        """Checks if two DAGOpNodes commute."""
+        qargs1 = op1.qargs
+        cargs1 = op2.cargs
+        if not isinstance(op1._raw_op, StandardGate):
+            op1 = op1.op
+        qargs2 = op2.qargs
+        cargs2 = op2.cargs
+        if not isinstance(op2._raw_op, StandardGate):
+            op2 = op2.op
+        return self.commute(op1, qargs1, cargs1, op2, qargs2, cargs2, max_num_qubits)
+
     def commute(
         self,
         op1: Operation,

qiskit/quantum_info/operators/operator.py

@@ -112,6 +112,7 @@ def __init__(
             # a complex numpy matrix.
             self._data = np.asarray(data.to_matrix(), dtype=complex)
         else:
+            print(data)
             raise QiskitError("Invalid input data format for Operator")
 
         super().__init__(

qiskit/transpiler/passes/optimization/commutation_analysis.py

@@ -72,14 +72,7 @@ def run(self, dag):
                         does_commute = (
                             isinstance(current_gate, DAGOpNode)
                             and isinstance(prev_gate, DAGOpNode)
-                            and self.comm_checker.commute(
-                                current_gate.op,
-                                current_gate.qargs,
-                                current_gate.cargs,
-                                prev_gate.op,
-                                prev_gate.qargs,
-                                prev_gate.cargs,
-                            )
+                            and self.comm_checker.commute_nodes(current_gate, prev_gate)
                         )
                         if not does_commute:
                             break

qiskit/transpiler/passes/optimization/commutative_cancellation.py

@@ -24,7 +24,7 @@
 from qiskit.circuit.library.standard_gates.rx import RXGate
 from qiskit.circuit.library.standard_gates.p import PhaseGate
 from qiskit.circuit.library.standard_gates.rz import RZGate
-from qiskit.circuit import ControlFlowOp
+from qiskit.circuit.controlflow import CONTROL_FLOW_OP_NAMES
 
 
 _CUTOFF_PRECISION = 1e-5
@@ -138,14 +138,14 @@ def run(self, dag):
                 total_phase = 0.0
                 for current_node in run:
                     if (
-                        getattr(current_node.op, "condition", None) is not None
+                        current_node.condition is not None
                         or len(current_node.qargs) != 1
                         or current_node.qargs[0] != run_qarg
                     ):
                         raise RuntimeError("internal error")
 
                     if current_node.name in ["p", "u1", "rz", "rx"]:
-                        current_angle = float(current_node.op.params[0])
+                        current_angle = float(current_node.params[0])
                     elif current_node.name in ["z", "x"]:
                         current_angle = np.pi
                     elif current_node.name == "t":
@@ -159,8 +159,8 @@ def run(self, dag):
 
                     # Compose gates
                     total_angle = current_angle + total_angle
-                    if current_node.op.definition:
-                        total_phase += current_node.op.definition.global_phase
+                    if current_node.definition:
+                        total_phase += current_node.definition.global_phase
 
                 # Replace the data of the first node in the run
                 if cancel_set_key[0] == "z_rotation":
@@ -200,7 +200,9 @@ def _handle_control_flow_ops(self, dag):
         """
 
         pass_manager = PassManager([CommutationAnalysis(), self])
-        for node in dag.op_nodes(ControlFlowOp):
+        for node in dag.op_nodes():
+            if node.name not in CONTROL_FLOW_OP_NAMES:
+                continue
             mapped_blocks = []
             for block in node.op.blocks:
                 new_circ = pass_manager.run(block)