Skip to content

Better separation of CircuitData and PyCircuitData #16095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
gadial wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Better separation of CircuitData and PyCircuitData #16095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0c49e7c
Move pack() from CircuitData to PyCircuitData
gadial Apr 28, 2026
f02030d
Removed `unpack_py_op` from `CircuitData`, refactored how `assign_par…
gadial Apr 29, 2026
ec80658
Remove PyErr from non python facing methods
gadial Apr 29, 2026
ad45579
revert the `assign_parameters_inner` changes due to many complications
gadial Apr 29, 2026
cc4c4a3
Merge branch 'main' into py_circuit_data_params_handling
gadial Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 106 additions & 92 deletions crates/circuit/src/circuit_data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// NEW FILE
// This code is part of Qiskit.
//
// (C) Copyright IBM 2023, 2024
Expand Down Expand Up @@ -81,6 +82,10 @@ pub enum CircuitDataError {
AbsentObject(object_registry::AbsentObject),
#[error(transparent)]
AddObjectRegistry(object_registry::AddError),
#[error("register name \"{0}\" already exists")]
RegisterNameExists(String),
#[error("{0} at index {1} exceeds circuit capacity.")]
BitExceedsCapacity(String, usize),
// Explicitly an error returned from calling Python
#[error(transparent)]
ErrorFromPython(#[from] PyErr),
Expand Down Expand Up @@ -121,6 +126,12 @@ impl From<CircuitDataError> for PyErr {
CircuitDataError::AddObjectRegistry(e) => e.into(),
CircuitDataError::ErrorFromPython(e) => e,
CircuitDataError::ParameterTableError(e) => e.into(),
CircuitDataError::RegisterNameExists(name) => {
CircuitError::new_err(format!("register name {name} already exists"))
}
CircuitDataError::BitExceedsCapacity(bit_type, bit_index) => CircuitError::new_err(
format!("{bit_type} at index {bit_index} exceds circuit capacity."),
),
CircuitDataError::VarStretchContainerError(e) => CircuitError::new_err(e.to_string()),
CircuitDataError::ParameterSliceLenMismatch => PyValueError::new_err(
"Mismatching number of values and parameters. For partial binding please pass a mapping of {parameter: value} pairs.",
Expand Down Expand Up @@ -402,7 +413,11 @@ impl CircuitData {
///
/// Args:
/// bit (:class:`.QuantumRegister`): The register to add.
pub fn add_qreg(&mut self, register: QuantumRegister, strict: bool) -> PyResult<()> {
pub fn add_qreg(
&mut self,
register: QuantumRegister,
strict: bool,
) -> Result<(), CircuitDataError> {
self.qregs.add_register(register.clone(), strict)?;

for (index, bit) in register.bits().enumerate() {
Expand All @@ -420,9 +435,7 @@ impl CircuitData {
bit,
BitLocations::new(
bit_idx.try_into().map_err(|_| {
CircuitError::new_err(format!(
"Qubit at index {bit_idx} exceeds circuit capacity."
))
CircuitDataError::BitExceedsCapacity("Qubit".to_string(), bit_idx)
})?,
[(register.clone(), index)],
),
Expand All @@ -436,7 +449,11 @@ impl CircuitData {
///
/// Args:
/// bit (:class:`.ClassicalRegister`): The register to add.
pub fn add_creg(&mut self, register: ClassicalRegister, strict: bool) -> PyResult<()> {
pub fn add_creg(
&mut self,
register: ClassicalRegister,
strict: bool,
) -> Result<(), CircuitDataError> {
self.cregs.add_register(register.clone(), strict)?;

for (index, bit) in register.bits().enumerate() {
Expand All @@ -454,9 +471,7 @@ impl CircuitData {
bit,
BitLocations::new(
bit_idx.try_into().map_err(|_| {
CircuitError::new_err(format!(
"Clbit at index {bit_idx} exceeds circuit capacity."
))
CircuitDataError::BitExceedsCapacity("Clbit".to_string(), bit_idx)
})?,
[(register.clone(), index)],
),
Expand Down Expand Up @@ -981,29 +996,6 @@ impl CircuitData {
Ok(())
}

pub fn pack(&mut self, py: Python, inst: &CircuitInstruction) -> PyResult<PackedInstruction> {
let qubits = self.qargs_interner.insert_owned(
self.qubits
.map_objects(inst.qubits.extract::<Vec<ShareableQubit>>(py)?)?
.collect(),
);
let clbits = self.cargs_interner.insert_owned(
self.clbits
.map_objects(inst.clbits.extract::<Vec<ShareableClbit>>(py)?)?
.collect(),
);
let params = self.extract_blocks_from_circuit_parameters(inst.params.as_ref());
Ok(PackedInstruction {
op: inst.operation.clone(),
qubits,
clbits,
params,
label: inst.label.clone(),
#[cfg(feature = "cache_pygates")]
py_op: inst.py_op.clone(),
})
}

/// Returns an iterator over all the instructions present in the circuit.
pub fn iter(&self) -> impl ExactSizeIterator<Item = &PackedInstruction> {
self.data.iter()
Expand Down Expand Up @@ -1344,17 +1336,23 @@ impl CircuitData {
// Rust that accept a `Param::ParameterExpression` which aren't standard
// gates. Technically `StandardInstruction::Delay` could, but in
// practice that's not a common path, and it's only supported for
// backwards compatibility from before Stretch was introduced. If we did
// backwards compatibility from before Stretch was introduced.
// it in rust without Python that's a mistake and this attach() call
// will panic and point out the error of your ways when this comment is
// read.
Python::attach(|py| -> Result<_, CircuitDataError> {
let validate_parameter_attr = intern!(py, "validate_parameter");
let assign_parameters_attr = intern!(py, "assign_parameters");

let op = self
.unpack_py_op(py, &self.data[instruction])?
.into_bound(py);
let params = self.unpack_blocks_to_circuit_parameters(
self.data[instruction].params.as_deref(),
);
let op = instruction::create_py_op(
py,
self.data[instruction].op.view(),
params,
self.data[instruction].label.as_deref().map(String::as_str),
)?
.into_bound(py);
let previous = &mut self.data[instruction];
// All "user" operations (e.g. PyOperation) use Parameters::Param.
let previous_param = &previous.params_view()[parameter];
Expand Down Expand Up @@ -1461,18 +1459,32 @@ impl CircuitData {
// they might be an `AnnotatedOperation`, which is one of our special built-ins.
// This should be handled more completely in the user-customisation interface by a
// delegating method, but that's not the data model we currently have.
let py_op = self.unpack_py_op(py, instruction)?;
let py_op = py_op.bind(py);
let params = self
.unpack_blocks_to_circuit_parameters(instruction.params.as_deref());
let py_op = instruction::create_py_op(
py,
instruction.op.view(),
params,
instruction.label.as_deref().map(String::as_str),
)?
.into_bound(py);
if !py_op.is_instance(ANNOTATED_OPERATION.get_bound(py))? {
continue;
}
py_op
.getattr(intern!(py, "base_op"))?
.getattr(_definition_attr)?
} else {
self.unpack_py_op(py, instruction)?
.bind(py)
.getattr(_definition_attr)?
let params = self
.unpack_blocks_to_circuit_parameters(instruction.params.as_deref());
instruction::create_py_op(
py,
instruction.op.view(),
params,
instruction.label.as_deref().map(String::as_str),
)?
.into_bound(py)
.getattr(_definition_attr)?
};
if !definition_cache.is_none() {
definition_cache.call_method(
Expand Down Expand Up @@ -1647,7 +1659,11 @@ impl CircuitData {
.any(|inst| inst.op.try_control_flow().is_some())
}

pub fn insert(&mut self, mut index: isize, packed: PackedInstruction) -> PyResult<()> {
pub fn insert(
&mut self,
mut index: isize,
packed: PackedInstruction,
) -> Result<(), CircuitDataError> {
// `list.insert` has special-case extra clamping logic for its index argument.
let index = {
if index < 0 {
Expand Down Expand Up @@ -1706,21 +1722,21 @@ impl CircuitData {
Ok(())
}

pub fn assign_parameters_from_array(&mut self, array: ArrayView1<f64>) -> PyResult<()> {
pub fn assign_parameters_from_array(
&mut self,
array: ArrayView1<f64>,
) -> Result<(), CircuitDataError> {
if array.len() != self.param_table.num_parameters() {
return Err(PyValueError::new_err(concat!(
"Mismatching number of values and parameters. For partial binding ",
"please pass a dictionary of {parameter: value} pairs."
)));
return Err(CircuitDataError::ParameterSliceLenMismatch);
}
let mut old_table = std::mem::take(&mut self.param_table);
Ok(self.assign_parameters_inner(
self.assign_parameters_inner(
array
.iter()
.map(|value| Param::Float(*value))
.zip(old_table.drain_ordered())
.map(|(value, (obj, uses))| (obj, value, uses)),
)?)
)
}

pub fn assign_parameters_from_slice(
Expand Down Expand Up @@ -1804,33 +1820,6 @@ impl CircuitData {
.count()
}

// TODO: in the long run, `CircuitData` should no rely on this method
pub fn unpack_py_op(&self, py: Python, instr: &PackedInstruction) -> PyResult<Py<PyAny>> {
// `OnceLock::get_or_init` and the non-stabilised `get_or_try_init`, which would otherwise
// be nice here are both non-reentrant. This is a problem if the init yields control to the
// Python interpreter as this one does, since that can allow CPython to freeze the thread
// and for another to attempt the initialisation.
#[cfg(feature = "cache_pygates")]
{
if let Some(ob) = instr.py_op.get() {
return Ok(ob.clone_ref(py));
}
}
let params = self.unpack_blocks_to_circuit_parameters(instr.params.as_deref());
let out = instruction::create_py_op(
py,
instr.op.view(),
params,
instr.label.as_deref().map(String::as_str),
)?;
#[cfg(feature = "cache_pygates")]
// The unpacking operation can cause a thread pause and concurrency, since it can call
// interpreted Python code for a standard gate, so we need to take care that some other
// Python thread might have populated the cache before we do.
let _ = instr.py_op.set(out.clone_ref(py));
Ok(out)
}

pub fn len(&self) -> usize {
self.data.len()
}
Expand Down Expand Up @@ -2462,7 +2451,7 @@ impl PyCircuitData {
/// bit (:class:`.QuantumRegister`): The register to add.
#[pyo3(signature = (register, *,strict = true))]
pub fn add_qreg(&mut self, register: QuantumRegister, strict: bool) -> PyResult<()> {
self.inner.add_qreg(register, strict)
Ok(self.inner.add_qreg(register, strict)?)
}

/// Registers a :class:`.ClassicalRegister` instance.
Expand All @@ -2471,7 +2460,7 @@ impl PyCircuitData {
/// bit (:class:`.ClassicalRegister`): The register to add.
#[pyo3(signature = (register, *,strict = true))]
pub fn add_creg(&mut self, register: ClassicalRegister, strict: bool) -> PyResult<()> {
self.inner.add_creg(register, strict)
Ok(self.inner.add_creg(register, strict)?)
}

/// Registers a :class:`.Clbit` instance.
Expand Down Expand Up @@ -2679,18 +2668,18 @@ impl PyCircuitData {
}

pub fn __setitem__(&mut self, index: PySequenceIndex, value: &Bound<PyAny>) -> PyResult<()> {
fn set_single(slf: &mut CircuitData, index: usize, value: &Bound<PyAny>) -> PyResult<()> {
fn set_single(slf: &mut PyCircuitData, index: usize, value: &Bound<PyAny>) -> PyResult<()> {
let py = value.py();
slf.untrack_instruction_parameters(index)?;
slf.untrack_instruction_blocks(index);
slf.data[index] = slf.pack(py, &value.cast::<CircuitInstruction>()?.borrow())?;
slf.track_instruction_blocks(index);
slf.track_instruction_parameters(index)?;
slf.inner.untrack_instruction_parameters(index)?;
slf.inner.untrack_instruction_blocks(index);
slf.inner.data[index] = slf.pack(py, &value.cast::<CircuitInstruction>()?.borrow())?;
slf.inner.track_instruction_blocks(index);
slf.inner.track_instruction_parameters(index)?;
Ok(())
}

match index.with_len(self.inner.data.len())? {
SequenceIndex::Int(index) => set_single(&mut self.inner, index, value),
SequenceIndex::Int(index) => set_single(self, index, value),
indices @ SequenceIndex::PosRange {
start,
stop,
Expand All @@ -2699,7 +2688,7 @@ impl PyCircuitData {
// `list` allows setting a slice with step +1 to an arbitrary length.
let values = value.try_iter()?.collect::<PyResult<Vec<_>>>()?;
for (index, value) in indices.iter().zip(values.iter()) {
set_single(&mut self.inner, index, value)?;
set_single(self, index, value)?;
}
if indices.len() > values.len() {
self.inner.delitem(SequenceIndex::PosRange {
Expand All @@ -2718,7 +2707,7 @@ impl PyCircuitData {
let values = value.try_iter()?.collect::<PyResult<Vec<_>>>()?;
if indices.len() == values.len() {
for (index, value) in indices.iter().zip(values.iter()) {
set_single(&mut self.inner, index, value)?;
set_single(self, index, value)?;
}
Ok(())
} else {
Expand All @@ -2734,13 +2723,13 @@ impl PyCircuitData {

pub fn insert(&mut self, index: isize, value: PyRef<CircuitInstruction>) -> PyResult<()> {
let py = value.py();
let packed = self.inner.pack(py, &value)?;
self.inner.insert(index, packed)
let packed = self.pack(py, &value)?;
Ok(self.inner.insert(index, packed)?)
}

/// Primary entry point for appending an instruction from Python space.
pub fn append(&mut self, value: &Bound<CircuitInstruction>) -> PyResult<()> {
let packed = self.inner.pack(value.py(), &value.borrow())?;
let packed = self.pack(value.py(), &value.borrow())?;
Ok(self.inner.push(packed)?)
}

Expand All @@ -2756,7 +2745,7 @@ impl PyCircuitData {
params: &Bound<PyList>,
) -> PyResult<()> {
let instruction_index = self.len();
let packed = self.inner.pack(value.py(), &value.borrow())?;
let packed = self.pack(value.py(), &value.borrow())?;
self.inner.data.push(packed);
for item in params.iter() {
let (parameter_index, parameters) = item.extract::<(u32, Bound<PyAny>)>()?;
Expand Down Expand Up @@ -2859,7 +2848,7 @@ impl PyCircuitData {
// Fast path for Numpy arrays; in this case we can easily handle them without copying
// the data across into a Rust-space `Vec` first.
let array = readonly.as_array();
self.inner.assign_parameters_from_array(array)
Ok(self.inner.assign_parameters_from_array(array)?)
} else {
let values = sequence
.try_iter()?
Expand Down Expand Up @@ -3279,6 +3268,31 @@ impl PyCircuitData {
}

impl PyCircuitData {
pub fn pack(&mut self, py: Python, inst: &CircuitInstruction) -> PyResult<PackedInstruction> {
let qubits = self.inner.qargs_interner.insert_owned(
self.qubits
.map_objects(inst.qubits.extract::<Vec<ShareableQubit>>(py)?)?
.collect(),
);
let clbits = self.inner.cargs_interner.insert_owned(
self.clbits
.map_objects(inst.clbits.extract::<Vec<ShareableClbit>>(py)?)?
.collect(),
);
let params = self
.inner
.extract_blocks_from_circuit_parameters(inst.params.as_ref());
Ok(PackedInstruction {
op: inst.operation.clone(),
qubits,
clbits,
params,
label: inst.label.clone(),
#[cfg(feature = "cache_pygates")]
py_op: inst.py_op.clone(),
})
}

/// Build a reference to the Python-space operation object (the `Gate`, etc) packed into an
/// instruction. This may construct the reference if the `Instruction` is a standard
/// gate or instruction with no already stored operation.
Expand Down
Loading