Skip to content

Commit

Permalink
Move QuantumCircuit.assign_parameters to Rust (#12794) (#12878)
Browse files Browse the repository at this point in the history
* Move `QuantumCircuit.assign_parameters` to Rust

This is (as far as I could tell), the last really major performance
regression in our asv suite compared to 1.1.0, so with this commit, we
should be at _not worse_ for important utility-scale benchmarks.

This largely rewrites `ParamTable` (renamed back to `ParameterTable`
because I kept getting confused with `Param`) to have more Rust-friendly
interfaces available, so that `assign_parameters` can then use them.

This represents a 2-3x speedup in `assign_parameters` performance over
1.1.0, when binding simple `Parameter` instances.  Approximately 75% of
the time is now spent in Python-space `Parameter.assign` and
`ParameterExpression.numeric` calls; almost all of this could be removed
were we to move `Parameter` and `ParameterExpression` to have their data
exposed directly to Rust space.  The percentage of time spent in Python
space only increases if the expressions to be bound are actual
`ParameterExpression`s and not just `Parameter`.

Most changes in the test suite are because of the use of internal-only
methods that changed with the new `ParameterTable`.  The only
discrepancy is a bug in `test_pauli_feature_map`, which was trying to
assign using a set.

* Add unit test of parameter insertion

This catches a bug that was present in the parent commit, but this PR
fixes.

* Update crates/circuit/src/imports.rs

* Fix assignment to `AnnotatedOperation`

* Rename `CircuitData::num_params` to match normal terminology

* Fix typos and 🇺🇸

* Fix lint

(cherry picked from commit a68de4f)

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
  • Loading branch information
mergify[bot] and jakelishman authored Aug 1, 2024
1 parent dc25aa6 commit 9120b8d
Show file tree
Hide file tree
Showing 12 changed files with 921 additions and 579 deletions.
676 changes: 402 additions & 274 deletions crates/circuit/src/circuit_data.rs

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions crates/circuit/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,12 @@ pub static SINGLETON_CONTROLLED_GATE: ImportOnceCell =
ImportOnceCell::new("qiskit.circuit.singleton", "SingletonControlledGate");
pub static CONTROLLED_GATE: ImportOnceCell =
ImportOnceCell::new("qiskit.circuit", "ControlledGate");
pub static ANNOTATED_OPERATION: ImportOnceCell =
ImportOnceCell::new("qiskit.circuit", "AnnotatedOperation");
pub static DEEPCOPY: ImportOnceCell = ImportOnceCell::new("copy", "deepcopy");
pub static QI_OPERATOR: ImportOnceCell = ImportOnceCell::new("qiskit.quantum_info", "Operator");
pub static WARNINGS_WARN: ImportOnceCell = ImportOnceCell::new("warnings", "warn");
pub static UUID: ImportOnceCell = ImportOnceCell::new("uuid", "UUID");

/// A mapping from the enum variant in crate::operations::StandardGate to the python
/// module path and class name to import it. This is used to populate the conversion table
Expand Down
66 changes: 54 additions & 12 deletions crates/circuit/src/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use smallvec::smallvec;
use numpy::IntoPyArray;
use numpy::PyReadonlyArray2;
use pyo3::prelude::*;
use pyo3::types::{IntoPyDict, PyTuple};
use pyo3::types::{IntoPyDict, PyFloat, PyIterator, PyTuple};
use pyo3::{intern, IntoPy, Python};

#[derive(Clone, Debug)]
Expand All @@ -37,17 +37,13 @@ pub enum Param {

impl<'py> FromPyObject<'py> for Param {
fn extract_bound(b: &Bound<'py, PyAny>) -> Result<Self, PyErr> {
Ok(
if b.is_instance(PARAMETER_EXPRESSION.get_bound(b.py()))?
|| b.is_instance(QUANTUM_CIRCUIT.get_bound(b.py()))?
{
Param::ParameterExpression(b.clone().unbind())
} else if let Ok(val) = b.extract::<f64>() {
Param::Float(val)
} else {
Param::Obj(b.clone().unbind())
},
)
Ok(if b.is_instance(PARAMETER_EXPRESSION.get_bound(b.py()))? {
Param::ParameterExpression(b.clone().unbind())
} else if let Ok(val) = b.extract::<f64>() {
Param::Float(val)
} else {
Param::Obj(b.clone().unbind())
})
}
}

Expand All @@ -71,6 +67,52 @@ impl ToPyObject for Param {
}
}

impl Param {
/// Get an iterator over any Python-space `Parameter` instances tracked within this `Param`.
pub fn iter_parameters<'py>(&self, py: Python<'py>) -> PyResult<ParamParameterIter<'py>> {
let parameters_attr = intern!(py, "parameters");
match self {
Param::Float(_) => Ok(ParamParameterIter(None)),
Param::ParameterExpression(expr) => Ok(ParamParameterIter(Some(
expr.bind(py).getattr(parameters_attr)?.iter()?,
))),
Param::Obj(obj) => {
let obj = obj.bind(py);
if obj.is_instance(QUANTUM_CIRCUIT.get_bound(py))? {
Ok(ParamParameterIter(Some(
obj.getattr(parameters_attr)?.iter()?,
)))
} else {
Ok(ParamParameterIter(None))
}
}
}
}

/// Extract from a Python object without numeric coercion to float. The default conversion will
/// coerce integers into floats, but in things like `assign_parameters`, this is not always
/// desirable.
pub fn extract_no_coerce(ob: &Bound<PyAny>) -> PyResult<Self> {
Ok(if ob.is_instance_of::<PyFloat>() {
Param::Float(ob.extract()?)
} else if ob.is_instance(PARAMETER_EXPRESSION.get_bound(ob.py()))? {
Param::ParameterExpression(ob.clone().unbind())
} else {
Param::Obj(ob.clone().unbind())
})
}
}

/// Struct to provide iteration over Python-space `Parameter` instances within a `Param`.
pub struct ParamParameterIter<'py>(Option<Bound<'py, PyIterator>>);
impl<'py> Iterator for ParamParameterIter<'py> {
type Item = PyResult<Bound<'py, PyAny>>;

fn next(&mut self) -> Option<Self::Item> {
self.0.as_mut().and_then(|iter| iter.next())
}
}

/// Trait for generic circuit operations these define the common attributes
/// needed for something to be addable to the circuit struct
pub trait Operation {
Expand Down
Loading

0 comments on commit 9120b8d

Please sign in to comment.