diff --git a/crates/circuit/src/circuit_data.rs b/crates/circuit/src/circuit_data.rs index a27a7f85fb3a..6b129c1ce401 100644 --- a/crates/circuit/src/circuit_data.rs +++ b/crates/circuit/src/circuit_data.rs @@ -1,3 +1,4 @@ +// NEW FILE // This code is part of Qiskit. // // (C) Copyright IBM 2023, 2024 @@ -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), @@ -121,6 +126,12 @@ impl From 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.", @@ -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() { @@ -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)], ), @@ -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() { @@ -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)], ), @@ -981,29 +996,6 @@ impl CircuitData { Ok(()) } - pub fn pack(&mut self, py: Python, inst: &CircuitInstruction) -> PyResult { - let qubits = self.qargs_interner.insert_owned( - self.qubits - .map_objects(inst.qubits.extract::>(py)?)? - .collect(), - ); - let clbits = self.cargs_interner.insert_owned( - self.clbits - .map_objects(inst.clbits.extract::>(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 { self.data.iter() @@ -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]; @@ -1461,8 +1459,15 @@ 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; } @@ -1470,9 +1475,16 @@ impl CircuitData { .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( @@ -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 { @@ -1706,21 +1722,21 @@ impl CircuitData { Ok(()) } - pub fn assign_parameters_from_array(&mut self, array: ArrayView1) -> PyResult<()> { + pub fn assign_parameters_from_array( + &mut self, + array: ArrayView1, + ) -> 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( @@ -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> { - // `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() } @@ -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. @@ -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. @@ -2679,18 +2668,18 @@ impl PyCircuitData { } pub fn __setitem__(&mut self, index: PySequenceIndex, value: &Bound) -> PyResult<()> { - fn set_single(slf: &mut CircuitData, index: usize, value: &Bound) -> PyResult<()> { + fn set_single(slf: &mut PyCircuitData, index: usize, value: &Bound) -> PyResult<()> { let py = value.py(); - slf.untrack_instruction_parameters(index)?; - slf.untrack_instruction_blocks(index); - slf.data[index] = slf.pack(py, &value.cast::()?.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::()?.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, @@ -2699,7 +2688,7 @@ impl PyCircuitData { // `list` allows setting a slice with step +1 to an arbitrary length. let values = value.try_iter()?.collect::>>()?; 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 { @@ -2718,7 +2707,7 @@ impl PyCircuitData { let values = value.try_iter()?.collect::>>()?; 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 { @@ -2734,13 +2723,13 @@ impl PyCircuitData { pub fn insert(&mut self, index: isize, value: PyRef) -> 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) -> PyResult<()> { - let packed = self.inner.pack(value.py(), &value.borrow())?; + let packed = self.pack(value.py(), &value.borrow())?; Ok(self.inner.push(packed)?) } @@ -2756,7 +2745,7 @@ impl PyCircuitData { params: &Bound, ) -> 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)>()?; @@ -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()? @@ -3279,6 +3268,31 @@ impl PyCircuitData { } impl PyCircuitData { + pub fn pack(&mut self, py: Python, inst: &CircuitInstruction) -> PyResult { + let qubits = self.inner.qargs_interner.insert_owned( + self.qubits + .map_objects(inst.qubits.extract::>(py)?)? + .collect(), + ); + let clbits = self.inner.cargs_interner.insert_owned( + self.clbits + .map_objects(inst.clbits.extract::>(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. diff --git a/crates/circuit/src/register_data.rs b/crates/circuit/src/register_data.rs index 0b6c2032fa2a..144f542339b1 100644 --- a/crates/circuit/src/register_data.rs +++ b/crates/circuit/src/register_data.rs @@ -17,7 +17,7 @@ use hashbrown::HashMap; use pyo3::prelude::*; use pyo3::types::{IntoPyDict, PyDict, PyList}; -use crate::{bit::Register, circuit_data::CircuitError}; +use crate::{bit::Register, circuit_data::CircuitDataError}; /// Represents the location in which the register is stored within [RegisterData]. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] @@ -103,7 +103,7 @@ where /// it sits will be returned. /// /// __**Note** if `strict` is passed, the insertion will fail. - pub fn add_register(&mut self, register: R, strict: bool) -> PyResult { + pub fn add_register(&mut self, register: R, strict: bool) -> Result { if self .reg_index .try_insert(register.name().to_string(), self.registers.len().into()) @@ -113,10 +113,9 @@ where self.cached_registers.take(); Ok(true) } else if strict { - Err(CircuitError::new_err(format!( - "register name \"{}\" already exists", - register.name() - ))) + Err(CircuitDataError::RegisterNameExists( + register.name().to_string(), + )) } else { Ok(false) } diff --git a/crates/qpy/src/circuit_reader.rs b/crates/qpy/src/circuit_reader.rs index 202704be0b72..5723cad79477 100644 --- a/crates/qpy/src/circuit_reader.rs +++ b/crates/qpy/src/circuit_reader.rs @@ -33,7 +33,7 @@ use qiskit_circuit::bit::{ }; use qiskit_circuit::circuit_data::{CircuitData, PyCircuitData}; use qiskit_circuit::circuit_instruction::OperationFromPython; -use qiskit_circuit::instruction::Parameters; +use qiskit_circuit::instruction::{Parameters, create_py_op}; use qiskit_circuit::interner::Interned; use qiskit_circuit::operations::ArrayType; use qiskit_circuit::operations::PauliBased; @@ -942,13 +942,19 @@ fn unpack_custom_instruction( None => gate_class_name, }; } + let params = qpy_data + .circuit_data + .unpack_blocks_to_circuit_parameters(base_gate.params.as_deref()); + let py_base_gate = create_py_op( + py, + base_gate.op.view(), + params, + base_gate.label.as_deref().map(String::as_str), + )?; let kwargs = PyDict::new(py); kwargs.set_item(intern!(py, "num_ctrl_qubits"), instruction.num_ctrl_qubits)?; kwargs.set_item(intern!(py, "ctrl_state"), instruction.ctrl_state)?; - kwargs.set_item( - intern!(py, "base_gate"), - qpy_data.circuit_data.unpack_py_op(py, &base_gate)?, - )?; + kwargs.set_item(intern!(py, "base_gate"), py_base_gate)?; let controlled_gate_object = imports::CONTROLLED_GATE.get_bound(py).call( (&gate_class_name, custom_instruction.num_qubits, py_params), @@ -968,11 +974,17 @@ fn unpack_custom_instruction( .0; let base_gate = unpack_instruction(&packed_base_gate, custom_instructions_map, qpy_data)?; - let kwargs = PyDict::new(py); - kwargs.set_item( - intern!(py, "base_op"), - qpy_data.circuit_data.unpack_py_op(py, &base_gate)?, + let params = qpy_data + .circuit_data + .unpack_blocks_to_circuit_parameters(base_gate.params.as_deref()); + let py_base_gate = create_py_op( + py, + base_gate.op.view(), + params, + base_gate.label.as_deref().map(String::as_str), )?; + let kwargs = PyDict::new(py); + kwargs.set_item(intern!(py, "base_op"), py_base_gate)?; kwargs.set_item(intern!(py, "modifiers"), py_params)?; imports::ANNOTATED_OPERATION .get_bound(py) diff --git a/crates/qpy/src/circuit_writer.rs b/crates/qpy/src/circuit_writer.rs index 2a941492ad26..97740d2b7180 100644 --- a/crates/qpy/src/circuit_writer.rs +++ b/crates/qpy/src/circuit_writer.rs @@ -1152,7 +1152,9 @@ fn pack_custom_instruction( // But we still want to serialize it like a regular instruction, so we need to convert it to a PackedInstruction. // To avoid changing the original CircuitData we use a hack where it is packed using a dummy circuit data. // TODO: Hopefully we'll change all this in a future version of QPY. - let mut dummy_circuit_data = CircuitData::new(None, None, Param::Float(0.0))?; + let mut dummy_circuit_data = PyCircuitData { + inner: CircuitData::new(None, None, Param::Float(0.0))?, + }; let packed_instruction = dummy_circuit_data.pack(py, &instruction)?; base_gate_raw = serialize(&pack_instruction( &packed_instruction,