Don't add __array__ to DAGOpNode or CircuitInstruction

Qiskit · Jul 3, 2024 · 5f94bf0 · 5f94bf0
1 parent db72c30
commit 5f94bf0
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 74 deletions.
diff --git a/crates/circuit/src/circuit_instruction.rs b/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, PyTypeError, PyValueError};
+use pyo3::exceptions::{PyDeprecationWarning, PyValueError};
 use pyo3::prelude::*;
 use pyo3::types::{IntoPyDict, PyList, PyTuple, PyType};
 use pyo3::{intern, IntoPy, PyObject, PyResult};
@@ -432,41 +432,6 @@ 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

diff --git a/crates/circuit/src/dag_node.rs b/crates/circuit/src/dag_node.rs
@@ -17,9 +17,8 @@ use crate::circuit_instruction::{
 use crate::imports::QUANTUM_CIRCUIT;
 use crate::operations::Operation;
 use numpy::IntoPyArray;
-use pyo3::exceptions::{PyTypeError, PyValueError};
 use pyo3::prelude::*;
-use pyo3::types::{IntoPyDict, PyDict, PyList, PySequence, PyString, PyTuple};
+use pyo3::types::{PyDict, PyList, PySequence, PyString, PyTuple};
 use pyo3::{intern, IntoPy, PyObject, PyResult, ToPyObject};
 use smallvec::smallvec;
 
@@ -281,41 +280,6 @@ impl DAGOpNode {
         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

diff --git a/qiskit/circuit/commutation_checker.py b/qiskit/circuit/commutation_checker.py
@@ -273,9 +273,15 @@ def is_commutation_skipped(op, qargs, max_num_qubits):
     if getattr(op, "is_parameterized", False) and op.is_parameterized():
         return True
 
+    from qiskit.dagcircuit.dagnode import DAGOpNode
+
     # we can proceed if op has defined: to_operator, to_matrix and __array__, or if its definition can be
     # recursively resolved by operations that have a matrix. We check this by constructing an Operator.
-    if (hasattr(op, "to_matrix") and hasattr(op, "__array__")) or hasattr(op, "to_operator"):
+    if (
+        isinstance(op, DAGOpNode)
+        or (hasattr(op, "to_matrix") and hasattr(op, "__array__"))
+        or hasattr(op, "to_operator")
+    ):
         return False
 
     return False
@@ -427,6 +433,15 @@ def _commute_matmul(
     first_qarg = tuple(qarg[q] for q in first_qargs)
     second_qarg = tuple(qarg[q] for q in second_qargs)
 
+    from qiskit.dagcircuit.dagnode import DAGOpNode
+
+    # If we have a DAGOpNode here we've received a StandardGate definition from
+    # rust and we can manually pull the matrix to use for the Operators
+    if isinstance(first_ops, DAGOpNode):
+        first_ops = first_ops.matrix
+    if isinstance(second_op, DAGOpNode):
+        second_op = second_op.matrix
+
     # try to generate an Operator out of op, if this succeeds we can determine commutativity, otherwise
     # return false
     try: