Skip to content

Commit

Permalink
Avoid ExtraInstructionAttributes allocation on unit="dt" (Qiskit#…
Browse files Browse the repository at this point in the history
…13078)

The default value for `Instruction.unit` is `"dt"`.  Previously, the
`OperationFromPython` extraction logic would only suppress allocation of
the extra instruction attributes if all the contained fields were
`None`, but `None` is not actually a valid value of `Instruction.unit`
(which must be a string).  This meant that `OperationFromPython` would
always allocate and store extra attributes, even for the default cases.
This did not affect standard gates appended using their corresponding
`QuantumCircuit` methods (since no Python-space extraction is
performed in that case), but did affect standard calls to `append`, or
anything else that entered from Python space.

This drastically reduces the memory usage of circuits built by
`append`-like methods. Ignoring the inefficiency factor of the
heap-allocation implementation, this saves 66 bytes plus
small-allocation overhead for 2-byte heap allocations (another 14 bytes
on macOS, but will vary depending on the allocator) per standard
instruction, which is on the order of 40% memory-usage reduction.
  • Loading branch information
jakelishman committed Sep 3, 2024
1 parent dde09f9 commit d3b8b91
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
37 changes: 33 additions & 4 deletions crates/circuit/src/circuit_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use numpy::IntoPyArray;
use pyo3::basic::CompareOp;
use pyo3::exceptions::{PyDeprecationWarning, PyTypeError};
use pyo3::prelude::*;
use pyo3::types::{PyList, PyTuple, PyType};
use pyo3::types::{PyList, PyString, PyTuple, PyType};
use pyo3::{intern, IntoPy, PyObject, PyResult};

use smallvec::SmallVec;
Expand Down Expand Up @@ -63,6 +63,20 @@ impl ExtraInstructionAttributes {
None
}
}

/// Get the Python-space version of the stored `unit`. This evalutes the Python-space default
/// (`"dt"`) value if we're storing a `None`.
pub fn py_unit(&self, py: Python) -> Py<PyString> {
self.unit
.as_deref()
.map(|unit| <&str as IntoPy<Py<PyString>>>::into_py(unit, py))
.unwrap_or_else(|| Self::default_unit(py).clone().unbind())
}

/// Get the Python-space default value for the `unit` field.
pub fn default_unit(py: Python) -> &Bound<PyString> {
intern!(py, "dt")
}
}

/// A single instruction in a :class:`.QuantumCircuit`, comprised of the :attr:`operation` and
Expand Down Expand Up @@ -267,10 +281,17 @@ impl CircuitInstruction {
}

#[getter]
fn unit(&self) -> Option<&str> {
fn unit(&self, py: Python) -> Py<PyString> {
// Python space uses `"dt"` as the default, whereas we simply don't store the extra
// attributes at all if they're none.
self.extra_attrs
.as_ref()
.and_then(|attrs| attrs.unit.as_deref())
.map(|attrs| attrs.py_unit(py))
.unwrap_or_else(|| {
ExtraInstructionAttributes::default_unit(py)
.clone()
.unbind()
})
}

/// Is the :class:`.Operation` contained in this instruction a Qiskit standard gate?
Expand Down Expand Up @@ -524,10 +545,18 @@ impl<'py> FromPyObject<'py> for OperationFromPython {
.map(|params| params.unwrap_or_default())
};
let extract_extra = || -> PyResult<_> {
let unit = {
// We accept Python-space `None` or `"dt"` as both meaning the default `"dt"`.
let raw_unit = ob.getattr(intern!(py, "unit"))?;
(!(raw_unit.is_none()
|| raw_unit.eq(ExtraInstructionAttributes::default_unit(py))?))
.then(|| raw_unit.extract::<String>())
.transpose()?
};
Ok(ExtraInstructionAttributes::new(
ob.getattr(intern!(py, "label"))?.extract()?,
ob.getattr(intern!(py, "duration"))?.extract()?,
ob.getattr(intern!(py, "unit"))?.extract()?,
unit,
ob.getattr(intern!(py, "condition"))?.extract()?,
)
.map(Box::from))
Expand Down
2 changes: 1 addition & 1 deletion crates/circuit/src/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ impl StandardGate {
if let Some(extra) = extra_attrs {
let kwargs = [
("label", extra.label.to_object(py)),
("unit", extra.unit.to_object(py)),
("unit", extra.py_unit(py).into_any()),
("duration", extra.duration.to_object(py)),
]
.into_py_dict_bound(py);
Expand Down

0 comments on commit d3b8b91

Please sign in to comment.