Skip to content

Commit

Permalink
Use PyO3 0.22 and rust-numpy 0.22 (#13313)
Browse files Browse the repository at this point in the history
* Use PyO3 0.22 and rust-numpy 0.22

This commit migrates to using PyO3 0.22 and rust-numpy and updates all
the deprecated and changed interfaces in the libraries to their new
syntax. No new or alternative interfaces are used as part of this PR
except for where deprecation warnings pointed to do so.

One thing to note is that in pyo3 0.22 a new feature py-clone
was added to re-enable `Py<T>::clone()`. This was disabled by default in
pyo3 0.22 because of a soundness issue around calling that in situations
when the GIL was not held. [1] Right now qiskit relies on using `Clone` on
`Py` objects because we have container types that own a `PyObject` that
we sometimes clone at the top level so this flag needs to be set. In the
future we can look at adding interfaces to avoid needing this flag in
the future.

Another thing to note is that rust-numpy includes support for ndarray
0.16, however this PR doesn't migrate to it because `ndarray_einsum_beta`
used in the `qiskit_accelerate::unitary_compose` module still depends on
ndarray 0.15 and we can't upgrade until that does.

[1] https://pyo3.rs/v0.22.3/migration#pyclone-is-now-gated-behind-the-py-clone-feature

* Fix MSRV

* Disable py-clone on accelerate
  • Loading branch information
mtreinish authored Oct 11, 2024
1 parent 48d2492 commit 1e2d337
Show file tree
Hide file tree
Showing 26 changed files with 114 additions and 153 deletions.
178 changes: 62 additions & 116 deletions Cargo.lock

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ indexmap.version = "2.6.0"
hashbrown.version = "0.14.5"
num-bigint = "0.4"
num-complex = "0.4"
ndarray = "^0.15.6"
numpy = "0.21.0"
ndarray = "0.15"
numpy = "0.22.0"
smallvec = "1.13"
thiserror = "1.0"
rustworkx-core = "0.15"
Expand All @@ -34,7 +34,7 @@ rayon = "1.10"
# distributions). We only activate that feature when building the C extension module; we still need
# it disabled for Rust-only tests to avoid linker errors with it not being loaded. See
# https://pyo3.rs/main/features#extension-module for more.
pyo3 = { version = "0.21.2", features = ["abi3-py39"] }
pyo3 = { version = "0.22.3", features = ["abi3-py39"] }

# These are our own crates.
qiskit-accelerate = { path = "crates/accelerate" }
Expand Down
2 changes: 1 addition & 1 deletion crates/accelerate/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,4 @@ version = "0.18.22"
features = ["macro"]

[features]
cache_pygates = ["qiskit-circuit/cache_pygates"]
cache_pygates = ["qiskit-circuit/cache_pygates"]
1 change: 1 addition & 0 deletions crates/accelerate/src/barrier_before_final_measurement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use qiskit_circuit::Qubit;
static FINAL_OP_NAMES: [&str; 2] = ["measure", "barrier"];

#[pyfunction]
#[pyo3(signature=(dag, label=None))]
pub fn barrier_before_final_measurements(
py: Python,
dag: &mut DAGCircuit,
Expand Down
1 change: 1 addition & 0 deletions crates/accelerate/src/circuit_library/quantum_volume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ fn random_unitaries(seed: u64, size: usize) -> impl Iterator<Item = Array2<Compl
const UNITARY_PER_SEED: usize = 50;

#[pyfunction]
#[pyo3(signature=(num_qubits, depth, seed=None))]
pub fn quantum_volume(
py: Python,
num_qubits: u32,
Expand Down
1 change: 1 addition & 0 deletions crates/accelerate/src/commutation_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ impl CommutationLibrary {
#[pymethods]
impl CommutationLibrary {
#[new]
#[pyo3(signature=(py_any=None))]
fn new(py_any: Option<Bound<PyAny>>) -> Self {
match py_any {
Some(pyob) => CommutationLibrary {
Expand Down
2 changes: 1 addition & 1 deletion crates/accelerate/src/equivalence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ pub struct GateOper {
}

impl<'py> FromPyObject<'py> for GateOper {
fn extract(ob: &'py PyAny) -> PyResult<Self> {
fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
let op_struct: OperationFromPython = ob.extract()?;
Ok(Self {
operation: op_struct.operation,
Expand Down
3 changes: 2 additions & 1 deletion crates/accelerate/src/error_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub struct ErrorMap {
#[pymethods]
impl ErrorMap {
#[new]
#[pyo3(text_signature = "(/, size=None)")]
#[pyo3(signature=(size=None))]
fn new(size: Option<usize>) -> Self {
match size {
Some(size) => ErrorMap {
Expand Down Expand Up @@ -100,6 +100,7 @@ impl ErrorMap {
Ok(self.error_map.contains_key(&key))
}

#[pyo3(signature=(key, default=None))]
fn get(&self, py: Python, key: [PhysicalQubit; 2], default: Option<PyObject>) -> PyObject {
match self.error_map.get(&key).copied() {
Some(val) => val.to_object(py),
Expand Down
5 changes: 4 additions & 1 deletion crates/accelerate/src/euler_one_qubit_decomposer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub struct OneQubitGateErrorMap {
#[pymethods]
impl OneQubitGateErrorMap {
#[new]
#[pyo3(signature=(num_qubits=None))]
fn new(num_qubits: Option<usize>) -> Self {
OneQubitGateErrorMap {
error_map: match num_qubits {
Expand Down Expand Up @@ -392,6 +393,7 @@ fn circuit_rr(
}

#[pyfunction]
#[pyo3(signature=(target_basis, theta, phi, lam, phase, simplify, atol=None))]
pub fn generate_circuit(
target_basis: &EulerBasis,
theta: f64,
Expand Down Expand Up @@ -673,7 +675,7 @@ impl Default for EulerBasisSet {
}

#[derive(Clone, Debug, Copy, Eq, Hash, PartialEq)]
#[pyclass(module = "qiskit._accelerate.euler_one_qubit_decomposer")]
#[pyclass(module = "qiskit._accelerate.euler_one_qubit_decomposer", eq, eq_int)]
pub enum EulerBasis {
U3 = 0,
U321 = 1,
Expand Down Expand Up @@ -808,6 +810,7 @@ fn compute_error_str(
}

#[pyfunction]
#[pyo3(signature=(circuit, qubit, error_map=None))]
pub fn compute_error_list(
circuit: Vec<PyRef<DAGOpNode>>,
qubit: usize,
Expand Down
6 changes: 5 additions & 1 deletion crates/accelerate/src/nlayout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ macro_rules! qubit_newtype {
}

impl pyo3::FromPyObject<'_> for $id {
fn extract(ob: &PyAny) -> PyResult<Self> {
fn extract_bound(ob: &Bound<'_, PyAny>) -> PyResult<Self> {
Ok(Self(ob.extract()?))
}
}
Expand All @@ -60,6 +60,10 @@ macro_rules! qubit_newtype {
fn get_dtype_bound(py: Python<'_>) -> Bound<'_, numpy::PyArrayDescr> {
u32::get_dtype_bound(py)
}

fn clone_ref(&self, _py: Python<'_>) -> Self {
*self
}
}
};
}
Expand Down
2 changes: 2 additions & 0 deletions crates/accelerate/src/results/marginalization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ fn marginalize<T: std::ops::AddAssign + Copy>(
}

#[pyfunction]
#[pyo3(signature=(counts, indices=None))]
pub fn marginal_counts(
counts: HashMap<String, u64>,
indices: Option<Vec<usize>>,
Expand All @@ -70,6 +71,7 @@ pub fn marginal_counts(
}

#[pyfunction]
#[pyo3(signature=(counts, indices=None))]
pub fn marginal_distribution(
counts: HashMap<String, f64>,
indices: Option<Vec<usize>>,
Expand Down
2 changes: 1 addition & 1 deletion crates/accelerate/src/sabre/heuristic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use pyo3::Python;

/// Affect the dynamic scaling of the weight of node-set-based heuristics (basic and lookahead).
#[pyclass]
#[pyo3(module = "qiskit._accelerate.sabre", frozen)]
#[pyo3(module = "qiskit._accelerate.sabre", frozen, eq)]
#[derive(Clone, Copy, PartialEq, Eq)]
pub enum SetScaling {
/// No dynamic scaling of the weight.
Expand Down
2 changes: 1 addition & 1 deletion crates/accelerate/src/sabre/neighbor_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl std::ops::Index<PhysicalQubit> for NeighborTable {
#[pymethods]
impl NeighborTable {
#[new]
#[pyo3(text_signature = "(/, adjacency_matrix=None)")]
#[pyo3(signature = (adjacency_matrix=None))]
pub fn new(adjacency_matrix: Option<PyReadonlyArray2<f64>>) -> PyResult<Self> {
let run_in_parallel = getenv_use_multiple_threads();
let neighbors = match adjacency_matrix {
Expand Down
1 change: 1 addition & 0 deletions crates/accelerate/src/sabre/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ impl<'a, 'b> RoutingState<'a, 'b> {
/// logical position of the qubit that began in position `i`.
#[pyfunction]
#[allow(clippy::too_many_arguments)]
#[pyo3(signature=(dag, neighbor_table, distance_matrix, heuristic, initial_layout, num_trials, seed=None, run_in_parallel=None))]
pub fn sabre_routing(
py: Python,
dag: &SabreDAG,
Expand Down
2 changes: 1 addition & 1 deletion crates/accelerate/src/stochastic_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ fn swap_trial(
/// will be ``(None, None, max int)``.
#[pyfunction]
#[pyo3(
text_signature = "(num_trials, num_qubits, int_layout, int_qubit_subset, int_gates, cdist, cdist2, edges, /, seed=None)"
signature = (num_trials, num_qubits, int_layout, int_qubit_subset, int_gates, cdist, cdist2, edges, seed=None)
)]
pub fn swap_trials(
num_trials: u64,
Expand Down
6 changes: 3 additions & 3 deletions crates/accelerate/src/target_transpiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ pub(crate) struct NormalOperation {
}

impl<'py> FromPyObject<'py> for NormalOperation {
fn extract(ob: &'py PyAny) -> PyResult<Self> {
fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
let operation: OperationFromPython = ob.extract()?;
Ok(Self {
operation: operation.operation,
params: operation.params,
op_object: ob.into(),
op_object: ob.clone().unbind(),
})
}
}
Expand Down Expand Up @@ -371,7 +371,7 @@ impl Target {
/// properties (InstructionProperties): The properties to set for this instruction
/// Raises:
/// KeyError: If ``instruction`` or ``qarg`` are not in the target
#[pyo3(text_signature = "(instruction, qargs, properties, /,)")]
#[pyo3(signature = (instruction, qargs=None, properties=None))]
fn update_instruction_properties(
&mut self,
instruction: String,
Expand Down
5 changes: 3 additions & 2 deletions crates/accelerate/src/two_qubit_decompose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,8 +407,8 @@ fn compute_unitary(sequence: &TwoQubitSequenceVec, global_phase: f64) -> Array2<

const DEFAULT_FIDELITY: f64 = 1.0 - 1.0e-9;

#[derive(Clone, Debug, Copy)]
#[pyclass(module = "qiskit._accelerate.two_qubit_decompose")]
#[derive(Clone, Debug, Copy, PartialEq, Eq)]
#[pyclass(module = "qiskit._accelerate.two_qubit_decompose", eq)]
pub enum Specialization {
General,
IdEquiv,
Expand Down Expand Up @@ -1030,6 +1030,7 @@ static IPX: GateArray1Q = [[C_ZERO, IM], [IM, C_ZERO]];
#[pymethods]
impl TwoQubitWeylDecomposition {
#[staticmethod]
#[pyo3(signature=(angles, matrices, specialization, default_euler_basis, calculated_fidelity, requested_fidelity=None))]
fn _from_state(
angles: [f64; 4],
matrices: [PyReadonlyArray2<Complex64>; 5],
Expand Down
4 changes: 2 additions & 2 deletions crates/circuit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ itertools.workspace = true

[dependencies.pyo3]
workspace = true
features = ["hashbrown", "indexmap", "num-complex", "num-bigint", "smallvec"]
features = ["hashbrown", "indexmap", "num-complex", "num-bigint", "smallvec", "py-clone"]

[dependencies.hashbrown]
workspace = true
Expand All @@ -38,4 +38,4 @@ workspace = true
features = ["union"]

[features]
cache_pygates = []
cache_pygates = []
1 change: 1 addition & 0 deletions crates/circuit/src/bit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impl PartialEq for BitAsKey {
.bind(py)
.repr()
.unwrap()
.as_any()
.eq(other.bit.bind(py).repr().unwrap())
.unwrap()
})
Expand Down
1 change: 1 addition & 0 deletions crates/circuit/src/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,7 @@ impl CircuitData {
Ok(())
}

#[pyo3(signature = (index=None))]
pub fn pop(&mut self, py: Python<'_>, index: Option<PySequenceIndex>) -> PyResult<PyObject> {
let index = index.unwrap_or(PySequenceIndex::Int(-1));
let native_index = index.with_len(self.data.len())?;
Expand Down
1 change: 1 addition & 0 deletions crates/circuit/src/circuit_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ impl CircuitInstruction {
///
/// Returns:
/// CircuitInstruction: A new instance with the given fields replaced.
#[pyo3(signature=(operation=None, qubits=None, clbits=None, params=None))]
pub fn replace(
&self,
py: Python,
Expand Down
10 changes: 7 additions & 3 deletions crates/circuit/src/dag_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ impl DAGCircuit {
///
/// Raises:
/// Exception: if the gate is of type string and params is None.
#[pyo3(signature=(gate, qubits, schedule, params=None))]
fn add_calibration<'py>(
&mut self,
py: Python<'py>,
Expand Down Expand Up @@ -2144,6 +2145,7 @@ def _format(operand):
///
/// Raises:
/// DAGCircuitError: If the DAG is invalid
#[pyo3(signature=(ignore=None))]
fn idle_wires(&self, py: Python, ignore: Option<&Bound<PyList>>) -> PyResult<Py<PyIterator>> {
let mut result: Vec<PyObject> = Vec::new();
let wires = (0..self.qubit_io_map.len())
Expand Down Expand Up @@ -2650,7 +2652,7 @@ def _format(operand):
///
/// Returns:
/// generator(DAGOpNode, DAGInNode, or DAGOutNode): node in topological order
#[pyo3(name = "topological_nodes")]
#[pyo3(name = "topological_nodes", signature=(key=None))]
fn py_topological_nodes(
&self,
py: Python,
Expand Down Expand Up @@ -2686,7 +2688,7 @@ def _format(operand):
///
/// Returns:
/// generator(DAGOpNode): op node in topological order
#[pyo3(name = "topological_op_nodes")]
#[pyo3(name = "topological_op_nodes", signature=(key=None))]
fn py_topological_op_nodes(
&self,
py: Python,
Expand Down Expand Up @@ -3813,7 +3815,8 @@ def _format(operand):
/// Yield:
/// edge: the edge as a tuple with the format
/// (source node, destination node, edge wire)
fn edges(&self, nodes: Option<Bound<PyAny>>, py: Python) -> PyResult<Py<PyIterator>> {
#[pyo3(signature=(nodes=None))]
fn edges(&self, py: Python, nodes: Option<Bound<PyAny>>) -> PyResult<Py<PyIterator>> {
let get_node_index = |obj: &Bound<PyAny>| -> PyResult<NodeIndex> {
Ok(obj.downcast::<DAGNode>()?.borrow().node.unwrap())
};
Expand Down Expand Up @@ -4717,6 +4720,7 @@ def _format(operand):
module.call_method1("dag_drawer", (slf, scale, filename, style))
}

#[pyo3(signature=(graph_attrs=None, node_attrs=None, edge_attrs=None))]
fn _to_dot<'py>(
&self,
py: Python<'py>,
Expand Down
1 change: 1 addition & 0 deletions crates/circuit/src/dag_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ impl DAGNode {
self.node.map(|node| node.index())
}

#[pyo3(signature=(index=None))]
fn __setstate__(&mut self, index: Option<usize>) {
self.node = index.map(NodeIndex::new);
}
Expand Down
10 changes: 1 addition & 9 deletions crates/circuit/src/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ impl<'a> Operation for OperationRef<'a> {

#[derive(Clone, Debug, Copy, Eq, PartialEq, Hash)]
#[repr(u8)]
#[pyclass(module = "qiskit._accelerate.circuit")]
#[pyclass(module = "qiskit._accelerate.circuit", eq, eq_int)]
pub enum StandardGate {
GlobalPhaseGate = 0,
HGate = 1,
Expand Down Expand Up @@ -720,14 +720,6 @@ impl StandardGate {
self.name()
}

pub fn __eq__(&self, other: &Bound<PyAny>) -> Py<PyAny> {
let py = other.py();
let Ok(other) = other.extract::<Self>() else {
return py.NotImplemented();
};
(*self == other).into_py(py)
}

pub fn __hash__(&self) -> isize {
*self as isize
}
Expand Down
2 changes: 1 addition & 1 deletion crates/circuit/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<'py> PySequenceIndex<'py> {
}
PySequenceIndex::Slice(slice) => {
let indices = slice
.indices(len as ::std::os::raw::c_long)
.indices(len as isize)
.map_err(PySequenceIndexError::from)?;
if indices.step > 0 {
Ok(SequenceIndex::PosRange {
Expand Down
12 changes: 6 additions & 6 deletions crates/qasm2/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ pub struct Bytecode {
}

/// The operations that are represented by the "bytecode" passed to Python.
#[pyclass(module = "qiskit._accelerate.qasm2", frozen)]
#[derive(Clone)]
#[pyclass(module = "qiskit._accelerate.qasm2", frozen, eq)]
#[derive(Clone, Eq, PartialEq)]
pub enum OpCode {
// There is only a `Gate` here, not a `GateInBasis`, because in Python space we don't have the
// same strict typing requirements to satisfy.
Expand Down Expand Up @@ -113,8 +113,8 @@ pub struct ExprCustom {
/// each of these, but this way involves fewer imports in Python, and also serves to split up the
/// option tree at the top level, so we don't have to test every unary operator before testing
/// other operations.
#[pyclass(module = "qiskit._accelerate.qasm2", frozen)]
#[derive(Clone)]
#[pyclass(module = "qiskit._accelerate.qasm2", frozen, eq)]
#[derive(Clone, PartialEq, Eq)]
pub enum UnaryOpCode {
Negate,
Cos,
Expand All @@ -129,8 +129,8 @@ pub enum UnaryOpCode {
/// each of these, but this way involves fewer imports in Python, and also serves to split up the
/// option tree at the top level, so we don't have to test every binary operator before testing
/// other operations.
#[pyclass(module = "qiskit._accelerate.qasm2", frozen)]
#[derive(Clone)]
#[pyclass(module = "qiskit._accelerate.qasm2", frozen, eq)]
#[derive(Clone, PartialEq, Eq)]
pub enum BinaryOpCode {
Add,
Subtract,
Expand Down

0 comments on commit 1e2d337

Please sign in to comment.