From 9120b8d90824fb7bfd03e1868558bbdca2b492f3 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 1 Aug 2024 12:59:38 +0000 Subject: [PATCH] Move `QuantumCircuit.assign_parameters` to Rust (#12794) (#12878) * 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 :us: * Fix lint (cherry picked from commit a68de4f9ff4bcf1716d29d21b39db42ba55b1aca) Co-authored-by: Jake Lishman --- crates/circuit/src/circuit_data.rs | 676 +++++++++++------- crates/circuit/src/imports.rs | 3 + crates/circuit/src/operations.rs | 66 +- crates/circuit/src/parameter_table.rs | 451 +++++++++--- qiskit/circuit/_utils.py | 13 - qiskit/circuit/quantumcircuit.py | 164 +---- qiskit/qasm3/exporter.py | 4 +- .../circuit/library/test_blueprintcircuit.py | 6 +- .../circuit/library/test_pauli_feature_map.py | 6 +- test/python/circuit/test_circuit_data.py | 4 +- .../python/circuit/test_circuit_operations.py | 2 +- test/python/circuit/test_parameters.py | 105 ++- 12 files changed, 921 insertions(+), 579 deletions(-) diff --git a/crates/circuit/src/circuit_data.rs b/crates/circuit/src/circuit_data.rs index a325ca4e1d5..a6b1ef84a43 100644 --- a/crates/circuit/src/circuit_data.rs +++ b/crates/circuit/src/circuit_data.rs @@ -15,22 +15,26 @@ use std::cell::RefCell; use crate::bit_data::BitData; use crate::circuit_instruction::{CircuitInstruction, OperationFromPython}; -use crate::imports::{BUILTIN_LIST, QUBIT}; +use crate::imports::{ANNOTATED_OPERATION, QUANTUM_CIRCUIT, QUBIT}; use crate::interner::{IndexedInterner, Interner, InternerKey}; -use crate::operations::{Operation, Param, StandardGate}; +use crate::operations::{Operation, OperationRef, Param, StandardGate}; use crate::packed_instruction::PackedInstruction; -use crate::parameter_table::{ParamEntry, ParamTable, GLOBAL_PHASE_INDEX}; +use crate::parameter_table::{ParameterTable, ParameterTableError, ParameterUse, ParameterUuid}; use crate::slice::{PySequenceIndex, SequenceIndex}; use crate::{Clbit, Qubit}; -use pyo3::exceptions::{PyIndexError, PyValueError}; +use numpy::PyReadonlyArray1; +use pyo3::exceptions::{PyRuntimeError, PyTypeError, PyValueError}; use pyo3::prelude::*; -use pyo3::types::{PyDict, PyList, PySet, PyTuple, PyType}; -use pyo3::{intern, PyTraverseError, PyVisit}; +use pyo3::pybacked::PyBackedStr; +use pyo3::types::{IntoPyDict, PyDict, PyList, PySet, PyTuple, PyType}; +use pyo3::{import_exception, intern, PyTraverseError, PyVisit}; use hashbrown::{HashMap, HashSet}; use smallvec::SmallVec; +import_exception!(qiskit.circuit.exceptions, CircuitError); + /// A container for :class:`.QuantumCircuit` instruction listings that stores /// :class:`.CircuitInstruction` instances in a packed form by interning /// their :attr:`~.CircuitInstruction.qubits` and @@ -94,7 +98,7 @@ pub struct CircuitData { qubits: BitData, /// Clbits registered in the circuit. clbits: BitData, - param_table: ParamTable, + param_table: ParameterTable, #[pyo3(get)] global_phase: Param, } @@ -133,7 +137,7 @@ impl CircuitData { cargs_interner: IndexedInterner::new(), qubits: BitData::new(py, "qubits".to_string()), clbits: BitData::new(py, "clbits".to_string()), - param_table: ParamTable::new(), + param_table: ParameterTable::new(), global_phase, }; if num_qubits > 0 { @@ -160,172 +164,73 @@ impl CircuitData { #[cfg(feature = "cache_pygates")] py_op: RefCell::new(None), }); + res.track_instruction_parameters(py, res.data.len() - 1)?; } Ok(res) } - fn handle_manual_params( + /// Add the entries from the `PackedInstruction` at the given index to the internal parameter + /// table. + fn track_instruction_parameters( &mut self, py: Python, - inst_index: usize, - params: &[(usize, Vec)], - ) -> PyResult { - let mut new_param = false; - let mut atomic_parameters: HashMap = HashMap::new(); - for (param_index, raw_param_objs) in params { - raw_param_objs.iter().for_each(|x| { - atomic_parameters.insert( - x.getattr(py, intern!(py, "_uuid")) - .expect("Not a parameter") - .getattr(py, intern!(py, "int")) - .expect("Not a uuid") - .extract::(py) - .unwrap(), - x.clone_ref(py), - ); - }); - for (param_uuid, param_obj) in atomic_parameters.iter() { - match self.param_table.table.get_mut(param_uuid) { - Some(entry) => entry.add(inst_index, *param_index), - None => { - new_param = true; - let new_entry = ParamEntry::new(inst_index, *param_index); - self.param_table - .insert(py, param_obj.clone_ref(py), new_entry)?; - } - }; + instruction_index: usize, + ) -> PyResult<()> { + for (index, param) in self.data[instruction_index] + .params_view() + .iter() + .enumerate() + { + let usage = ParameterUse::Index { + instruction: instruction_index, + parameter: index as u32, + }; + for param_ob in param.iter_parameters(py)? { + self.param_table.track(¶m_ob?, Some(usage))?; } - atomic_parameters.clear() } - Ok(new_param) + Ok(()) } - /// Add an instruction's entries to the parameter table - fn update_param_table( + /// Remove the entries from the `PackedInstruction` at the given index from the internal + /// parameter table. + fn untrack_instruction_parameters( &mut self, py: Python, - inst_index: usize, - params: Option)>>, - ) -> PyResult { - if let Some(params) = params { - return self.handle_manual_params(py, inst_index, ¶ms); - } - // Update the parameter table - let mut new_param = false; - let inst_params = self.data[inst_index].params_view(); - if !inst_params.is_empty() { - let params: Vec<(usize, PyObject)> = inst_params - .iter() - .enumerate() - .filter_map(|(idx, x)| match x { - Param::ParameterExpression(param_obj) => Some((idx, param_obj.clone_ref(py))), - _ => None, - }) - .collect(); - if !params.is_empty() { - let list_builtin = BUILTIN_LIST.get_bound(py); - let mut atomic_parameters: HashMap = HashMap::new(); - for (param_index, param) in ¶ms { - let temp: PyObject = param.getattr(py, intern!(py, "parameters"))?; - let raw_param_objs: Vec = list_builtin.call1((temp,))?.extract()?; - raw_param_objs.iter().for_each(|x| { - atomic_parameters.insert( - x.getattr(py, intern!(py, "_uuid")) - .expect("Not a parameter") - .getattr(py, intern!(py, "int")) - .expect("Not a uuid") - .extract(py) - .unwrap(), - x.clone_ref(py), - ); - }); - for (param_uuid, param_obj) in &atomic_parameters { - match self.param_table.table.get_mut(param_uuid) { - Some(entry) => entry.add(inst_index, *param_index), - None => { - new_param = true; - let new_entry = ParamEntry::new(inst_index, *param_index); - self.param_table - .insert(py, param_obj.clone_ref(py), new_entry)?; - } - }; - } - atomic_parameters.clear(); - } - } - } - Ok(new_param) - } - - /// Remove an index's entries from the parameter table. - fn remove_from_parameter_table(&mut self, py: Python, inst_index: usize) -> PyResult<()> { - let list_builtin = BUILTIN_LIST.get_bound(py); - if inst_index == GLOBAL_PHASE_INDEX { - if let Param::ParameterExpression(global_phase) = &self.global_phase { - let temp: PyObject = global_phase.getattr(py, intern!(py, "parameters"))?; - let raw_param_objs: Vec = list_builtin.call1((temp,))?.extract()?; - for (param_index, param_obj) in raw_param_objs.iter().enumerate() { - let uuid: u128 = param_obj - .getattr(py, intern!(py, "_uuid"))? - .getattr(py, intern!(py, "int"))? - .extract(py)?; - let name: String = param_obj.getattr(py, intern!(py, "name"))?.extract(py)?; - self.param_table - .discard_references(uuid, inst_index, param_index, name); - } - } - } else if !self.data[inst_index].params_view().is_empty() { - let params: Vec<(usize, PyObject)> = self.data[inst_index] - .params_view() - .iter() - .enumerate() - .filter_map(|(idx, x)| match x { - Param::ParameterExpression(param_obj) => Some((idx, param_obj.clone_ref(py))), - _ => None, - }) - .collect(); - if !params.is_empty() { - for (param_index, param) in ¶ms { - let temp: PyObject = param.getattr(py, intern!(py, "parameters"))?; - let raw_param_objs: Vec = list_builtin.call1((temp,))?.extract()?; - let mut atomic_parameters: HashSet<(u128, String)> = - HashSet::with_capacity(params.len()); - for x in raw_param_objs { - let uuid = x - .getattr(py, intern!(py, "_uuid"))? - .getattr(py, intern!(py, "int"))? - .extract(py)?; - let name = x.getattr(py, intern!(py, "name"))?.extract(py)?; - atomic_parameters.insert((uuid, name)); - } - for (uuid, name) in atomic_parameters { - self.param_table - .discard_references(uuid, inst_index, *param_index, name); - } - } + instruction_index: usize, + ) -> PyResult<()> { + for (index, param) in self.data[instruction_index] + .params_view() + .iter() + .enumerate() + { + let usage = ParameterUse::Index { + instruction: instruction_index, + parameter: index as u32, + }; + for param_ob in param.iter_parameters(py)? { + self.param_table.untrack(¶m_ob?, usage)?; } } Ok(()) } + /// Retrack the entire `ParameterTable`. + /// + /// This is necessary each time an insertion or removal occurs on `self.data` other than in the + /// last position. fn reindex_parameter_table(&mut self, py: Python) -> PyResult<()> { self.param_table.clear(); for inst_index in 0..self.data.len() { - self.update_param_table(py, inst_index, None)?; + self.track_instruction_parameters(py, inst_index)?; + } + for param_ob in self.global_phase.iter_parameters(py)? { + self.param_table + .track(¶m_ob?, Some(ParameterUse::GlobalPhase))?; } - // Technically we could keep the global phase entry directly if it exists, but we're - // the incremental cost is minimal after reindexing everything. - self.global_phase(py, self.global_phase.clone())?; Ok(()) } - - pub fn append_inner(&mut self, py: Python, value: &CircuitInstruction) -> PyResult { - let packed = self.pack(py, value)?; - let new_index = self.data.len(); - self.data.push(packed); - self.update_param_table(py, new_index, None) - } } #[pymethods] @@ -346,10 +251,10 @@ impl CircuitData { cargs_interner: IndexedInterner::new(), qubits: BitData::new(py, "qubits".to_string()), clbits: BitData::new(py, "clbits".to_string()), - param_table: ParamTable::new(), + param_table: ParameterTable::new(), global_phase: Param::Float(0.), }; - self_.global_phase(py, global_phase)?; + self_.set_global_phase(py, global_phase)?; if let Some(qubits) = qubits { for bit in qubits.iter()? { self_.add_qubit(py, &bit?, true)?; @@ -430,6 +335,32 @@ impl CircuitData { self.clbits.len() } + /// Return the number of unbound compile-time symbolic parameters tracked by the circuit. + pub fn num_parameters(&self) -> usize { + self.param_table.num_parameters() + } + + /// Get a (cached) sorted list of the Python-space `Parameter` instances tracked by this circuit + /// data's parameter table. + #[getter] + pub fn get_parameters<'py>(&mut self, py: Python<'py>) -> Bound<'py, PyList> { + self.param_table.py_parameters(py) + } + + pub fn unsorted_parameters<'py>(&self, py: Python<'py>) -> PyResult> { + self.param_table.py_parameters_unsorted(py) + } + + fn _raw_parameter_table_entry(&self, param: Bound) -> PyResult> { + self.param_table._py_raw_entry(param) + } + + pub fn get_parameter_by_name(&self, py: Python, name: PyBackedStr) -> Option> { + self.param_table + .py_parameter_by_name(&name) + .map(|ob| ob.clone_ref(py)) + } + /// Return the width of the circuit. This is the number of qubits plus the /// number of clbits. /// @@ -723,22 +654,12 @@ impl CircuitData { self.delitem(py, index.with_len(self.data.len())?) } - pub fn setitem_no_param_table_update( - &mut self, - py: Python, - index: usize, - value: &CircuitInstruction, - ) -> PyResult<()> { - self.data[index] = self.pack(py, value)?; - Ok(()) - } - pub fn __setitem__(&mut self, index: PySequenceIndex, value: &Bound) -> PyResult<()> { fn set_single(slf: &mut CircuitData, index: usize, value: &Bound) -> PyResult<()> { let py = value.py(); + slf.untrack_instruction_parameters(py, index)?; slf.data[index] = slf.pack(py, &value.downcast::()?.borrow())?; - slf.remove_from_parameter_table(py, index)?; - slf.update_param_table(py, index, None)?; + slf.track_instruction_parameters(py, index)?; Ok(()) } @@ -808,7 +729,7 @@ impl CircuitData { let packed = self.pack(py, &value)?; self.data.insert(index, packed); if index == self.data.len() - 1 { - self.update_param_table(py, index, None)?; + self.track_instruction_parameters(py, index)?; } else { self.reindex_parameter_table(py)?; } @@ -823,16 +744,40 @@ impl CircuitData { Ok(item) } - pub fn append( - &mut self, - py: Python<'_>, - value: &Bound, - params: Option)>>, - ) -> PyResult { + /// Primary entry point for appending an instruction from Python space. + pub fn append(&mut self, value: &Bound) -> PyResult<()> { + let py = value.py(); let new_index = self.data.len(); let packed = self.pack(py, &value.borrow())?; self.data.push(packed); - self.update_param_table(py, new_index, params) + self.track_instruction_parameters(py, new_index) + } + + /// Backup entry point for appending an instruction from Python space, in the unusual case that + /// one of the instruction parameters contains a cyclical reference to the circuit itself. + /// + /// In this case, the `params` field should be a list of `(index, parameters)` tuples, where the + /// index is into the instruction's `params` attribute, and `parameters` is a Python iterable + /// of `Parameter` objects. + pub fn append_manual_params( + &mut self, + value: &Bound, + params: &Bound, + ) -> PyResult<()> { + let instruction_index = self.data.len(); + let packed = self.pack(value.py(), &value.borrow())?; + self.data.push(packed); + for item in params.iter() { + let (parameter_index, parameters) = item.extract::<(u32, Bound)>()?; + let usage = ParameterUse::Index { + instruction: instruction_index, + parameter: parameter_index, + }; + for param in parameters.iter()? { + self.param_table.track(¶m?, Some(usage))?; + } + } + Ok(()) } pub fn extend(&mut self, py: Python<'_>, itr: &Bound) -> PyResult<()> { @@ -879,20 +824,63 @@ impl CircuitData { #[cfg(feature = "cache_pygates")] py_op: inst.py_op.clone(), }); - self.update_param_table(py, new_index, None)?; + self.track_instruction_parameters(py, new_index)?; } return Ok(()); } for v in itr.iter()? { - self.append_inner(py, &v?.downcast()?.borrow())?; + self.append(v?.downcast()?)?; } Ok(()) } - pub fn clear(&mut self, _py: Python<'_>) -> PyResult<()> { + /// Assign all the circuit parameters, given a sequence-like input of `Param` instances. + fn assign_parameters_sequence(&mut self, sequence: Bound) -> PyResult<()> { + if sequence.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." + ))); + } + let mut old_table = std::mem::take(&mut self.param_table); + if let Ok(readonly) = sequence.extract::>() { + // 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.assign_parameters_inner( + sequence.py(), + array + .iter() + .zip(old_table.drain_ordered()) + .map(|(value, (param_ob, uses))| (param_ob, Param::Float(*value), uses)), + ) + } else { + let values = sequence.extract::>()?; + self.assign_parameters_inner( + sequence.py(), + values + .into_iter() + .zip(old_table.drain_ordered()) + .map(|(value, (param_ob, uses))| (param_ob, value.0, uses)), + ) + } + } + + /// Assign all uses of the circuit parameters as keys `mapping` to their corresponding values. + fn assign_parameters_mapping(&mut self, mapping: Bound) -> PyResult<()> { + let py = mapping.py(); + let mut items = Vec::new(); + for item in mapping.call_method0("items")?.iter()? { + let (param_ob, value) = item?.extract::<(Py, AssignParam)>()?; + let uuid = ParameterUuid::from_parameter(param_ob.bind(py))?; + items.push((param_ob, value.0, self.param_table.pop(uuid)?)); + } + self.assign_parameters_inner(py, items) + } + + pub fn clear(&mut self) { std::mem::take(&mut self.data); self.param_table.clear(); - Ok(()) } // Marks this pyclass as NOT hashable. @@ -939,6 +927,7 @@ impl CircuitData { // references they contain are to the bits in these lists! visit.call(self.qubits.cached())?; visit.call(self.clbits.cached())?; + self.param_table.py_gc_traverse(&visit)?; Ok(()) } @@ -947,117 +936,47 @@ impl CircuitData { self.data.clear(); self.qubits.dispose(); self.clbits.dispose(); + self.param_table.clear(); } + /// Set the global phase of the circuit. + /// + /// This method assumes that the parameter table is either fully consistent, or contains zero + /// entries for the global phase, regardless of what value is currently stored there. It's not + /// uncommon for subclasses and other parts of Qiskit to have filled in the global phase field + /// by copies or other means, before making the parameter table consistent. #[setter] - pub fn global_phase(&mut self, py: Python, angle: Param) -> PyResult<()> { - let list_builtin = BUILTIN_LIST.get_bound(py); - self.remove_from_parameter_table(py, GLOBAL_PHASE_INDEX)?; + pub fn set_global_phase(&mut self, py: Python, angle: Param) -> PyResult<()> { + if let Param::ParameterExpression(expr) = &self.global_phase { + for param_ob in expr.bind(py).getattr(intern!(py, "parameters"))?.iter()? { + match self.param_table.remove_use( + ParameterUuid::from_parameter(¶m_ob?)?, + ParameterUse::GlobalPhase, + ) { + Ok(_) + | Err(ParameterTableError::ParameterNotTracked(_)) + | Err(ParameterTableError::UsageNotTracked(_)) => (), + // Any errors added later might want propagating. + } + } + } match angle { Param::Float(angle) => { self.global_phase = Param::Float(angle.rem_euclid(2. * std::f64::consts::PI)); + Ok(()) } - Param::ParameterExpression(angle) => { - let temp: PyObject = angle.getattr(py, intern!(py, "parameters"))?; - let raw_param_objs: Vec = list_builtin.call1((temp,))?.extract()?; - - for (param_index, param_obj) in raw_param_objs.into_iter().enumerate() { - let param_uuid: u128 = param_obj - .getattr(py, intern!(py, "_uuid"))? - .getattr(py, intern!(py, "int"))? - .extract(py)?; - match self.param_table.table.get_mut(¶m_uuid) { - Some(entry) => entry.add(GLOBAL_PHASE_INDEX, param_index), - None => { - let new_entry = ParamEntry::new(GLOBAL_PHASE_INDEX, param_index); - self.param_table.insert(py, param_obj, new_entry)?; - } - }; + Param::ParameterExpression(_) => { + for param_ob in angle.iter_parameters(py)? { + self.param_table + .track(¶m_ob?, Some(ParameterUse::GlobalPhase))?; } - self.global_phase = Param::ParameterExpression(angle); - } - Param::Obj(_) => return Err(PyValueError::new_err("Invalid type for global phase")), - }; - Ok(()) - } - - /// Get the global_phase sentinel value - #[classattr] - pub const fn global_phase_param_index() -> usize { - GLOBAL_PHASE_INDEX - } - - // Below are functions to interact with the parameter table. These methods - // are done to avoid needing to deal with shared references and provide - // an entry point via python through an owned CircuitData object. - pub fn num_params(&self) -> usize { - self.param_table.table.len() - } - - pub fn get_param_from_name(&self, py: Python, name: String) -> Option { - self.param_table.get_param_from_name(py, name) - } - - pub fn get_params_unsorted(&self, py: Python) -> PyResult> { - Ok(PySet::new_bound(py, self.param_table.uuid_map.values())?.unbind()) - } - - pub fn pop_param(&mut self, py: Python, uuid: u128, name: &str, default: PyObject) -> PyObject { - match self.param_table.pop(uuid, name) { - Some(res) => res.into_py(py), - None => default.clone_ref(py), - } - } - - pub fn _get_param(&self, py: Python, uuid: u128) -> PyObject { - self.param_table.table[&uuid].clone().into_py(py) - } - - pub fn contains_param(&self, uuid: u128) -> bool { - self.param_table.table.contains_key(&uuid) - } - - pub fn add_new_parameter( - &mut self, - py: Python, - param: PyObject, - inst_index: usize, - param_index: usize, - ) -> PyResult<()> { - self.param_table.insert( - py, - param.clone_ref(py), - ParamEntry::new(inst_index, param_index), - )?; - Ok(()) - } - - pub fn update_parameter_entry( - &mut self, - uuid: u128, - inst_index: usize, - param_index: usize, - ) -> PyResult<()> { - match self.param_table.table.get_mut(&uuid) { - Some(entry) => { - entry.add(inst_index, param_index); + self.global_phase = angle; Ok(()) } - None => Err(PyIndexError::new_err(format!( - "Invalid parameter uuid: {:?}", - uuid - ))), + Param::Obj(_) => Err(PyTypeError::new_err("invalid type for global phase")), } } - pub fn _get_entry_count(&self, py: Python, param_obj: PyObject) -> PyResult { - let uuid: u128 = param_obj - .getattr(py, intern!(py, "_uuid"))? - .getattr(py, intern!(py, "int"))? - .extract(py)?; - Ok(self.param_table.table[&uuid].index_ids.len()) - } - pub fn num_nonlocal_gates(&self) -> usize { self.data .iter() @@ -1105,4 +1024,213 @@ impl CircuitData { pub fn iter(&self) -> impl Iterator { self.data.iter() } + + fn assign_parameters_inner(&mut self, py: Python, iter: I) -> PyResult<()> + where + I: IntoIterator, Param, HashSet)>, + { + let inconsistent = + || PyRuntimeError::new_err("internal error: circuit parameter table is inconsistent"); + + let assign_attr = intern!(py, "assign"); + let assign_parameters_attr = intern!(py, "assign_parameters"); + let _definition_attr = intern!(py, "_definition"); + let numeric_attr = intern!(py, "numeric"); + let parameters_attr = intern!(py, "parameters"); + let params_attr = intern!(py, "params"); + let validate_parameter_attr = intern!(py, "validate_parameter"); + + // Bind a single `Parameter` into a Python-space `ParameterExpression`. + let bind_expr = |expr: Borrowed, + param_ob: &Py, + value: &Param, + coerce: bool| + -> PyResult { + let new_expr = expr.call_method1(assign_attr, (param_ob, value.to_object(py)))?; + if new_expr.getattr(parameters_attr)?.len()? == 0 { + let out = new_expr.call_method0(numeric_attr)?; + if coerce { + out.extract() + } else { + Param::extract_no_coerce(&out) + } + } else { + Ok(Param::ParameterExpression(new_expr.unbind())) + } + }; + + let mut user_operations = HashMap::new(); + let mut uuids = Vec::new(); + for (param_ob, value, uses) in iter { + debug_assert!(!uses.is_empty()); + uuids.clear(); + for inner_param_ob in value.iter_parameters(py)? { + uuids.push(self.param_table.track(&inner_param_ob?, None)?) + } + for usage in uses { + match usage { + ParameterUse::GlobalPhase => { + let Param::ParameterExpression(expr) = &self.global_phase else { + return Err(inconsistent()); + }; + self.set_global_phase( + py, + bind_expr(expr.bind_borrowed(py), ¶m_ob, &value, true)?, + )?; + } + ParameterUse::Index { + instruction, + parameter, + } => { + let parameter = parameter as usize; + let previous = &mut self.data[instruction]; + if let Some(standard) = previous.standard_gate() { + let params = previous.params_mut(); + let Param::ParameterExpression(expr) = ¶ms[parameter] else { + return Err(inconsistent()); + }; + params[parameter] = + match bind_expr(expr.bind_borrowed(py), ¶m_ob, &value, true)? { + Param::Obj(obj) => { + return Err(CircuitError::new_err(format!( + "bad type after binding for gate '{}': '{}'", + standard.name(), + obj.bind(py).repr()?, + ))) + } + param => param, + }; + for uuid in uuids.iter() { + self.param_table.add_use(*uuid, usage)? + } + #[cfg(feature = "cache_pygates")] + { + // Standard gates can all rebuild their definitions, so if the + // cached py_op exists, just clear out any existing cache. + if let Some(borrowed) = previous.py_op.borrow().as_ref() { + borrowed.bind(py).setattr("_definition", py.None())? + } + } + } else { + // Track user operations we've seen so we can rebind their definitions. + // Strictly this can add the same binding pair more than once, if an + // instruction has the same `Parameter` in several of its `params`, but + // we're going to turn that into a `dict` anyway, so it doesn't matter. + user_operations + .entry(instruction) + .or_insert_with(Vec::new) + .push((param_ob.clone_ref(py), value.clone())); + + let op = previous.unpack_py_op(py)?.into_bound(py); + let previous_param = &previous.params_view()[parameter]; + let new_param = match previous_param { + Param::Float(_) => return Err(inconsistent()), + Param::ParameterExpression(expr) => { + // For user gates, we don't coerce floats to integers in `Param` + // so that users can use them if they choose. + let new_param = bind_expr( + expr.bind_borrowed(py), + ¶m_ob, + &value, + false, + )?; + // Historically, `assign_parameters` called `validate_parameter` + // only when a `ParameterExpression` became fully bound. Some + // "generalised" (or user) gates fail without this, though + // arguably, that's them indicating they shouldn't be allowed to + // be parametric. + // + // Our `bind_expr` coercion means that a non-parametric + // `ParameterExperssion` after binding would have been coerced + // to a numeric quantity already, so the match here is + // definitely parameterized. + match new_param { + Param::ParameterExpression(_) => new_param, + new_param => Param::extract_no_coerce(&op.call_method1( + validate_parameter_attr, + (new_param,), + )?)?, + } + } + Param::Obj(obj) => { + let obj = obj.bind_borrowed(py); + if !obj.is_instance(QUANTUM_CIRCUIT.get_bound(py))? { + return Err(inconsistent()); + } + Param::extract_no_coerce( + &obj.call_method( + assign_parameters_attr, + ([(¶m_ob, &value)].into_py_dict_bound(py),), + Some( + &[("inplace", false), ("flat_input", true)] + .into_py_dict_bound(py), + ), + )?, + )? + } + }; + op.getattr(params_attr)?.set_item(parameter, new_param)?; + let mut new_op = op.extract::()?; + previous.op = new_op.operation; + previous.params_mut().swap_with_slice(&mut new_op.params); + previous.extra_attrs = new_op.extra_attrs; + #[cfg(feature = "cache_pygates")] + { + *previous.py_op.borrow_mut() = Some(op.into_py(py)); + } + for uuid in uuids.iter() { + self.param_table.add_use(*uuid, usage)? + } + } + } + } + } + } + + let assign_kwargs = (!user_operations.is_empty()).then(|| { + [("inplace", true), ("flat_input", true), ("strict", false)].into_py_dict_bound(py) + }); + for (instruction, bindings) in user_operations { + // We only put non-standard gates in `user_operations`, so we're not risking creating a + // previously non-existent Python object. + let instruction = &self.data[instruction]; + let definition_cache = if matches!(instruction.op.view(), OperationRef::Operation(_)) { + // `Operation` instances don't have a `definition` as part of their interfaces, but + // 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 = instruction.unpack_py_op(py)?; + let py_op = py_op.bind(py); + if !py_op.is_instance(ANNOTATED_OPERATION.get_bound(py))? { + continue; + } + py_op + .getattr(intern!(py, "base_op"))? + .getattr(_definition_attr)? + } else { + instruction + .unpack_py_op(py)? + .bind(py) + .getattr(_definition_attr)? + }; + if !definition_cache.is_none() { + definition_cache.call_method( + assign_parameters_attr, + (bindings.into_py_dict_bound(py),), + assign_kwargs.as_ref(), + )?; + } + } + Ok(()) + } +} + +/// Helper struct for `assign_parameters` to allow use of `Param::extract_no_coerce` in +/// PyO3-provided `FromPyObject` implementations on containers. +#[repr(transparent)] +struct AssignParam(Param); +impl<'py> FromPyObject<'py> for AssignParam { + fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult { + Ok(Self(Param::extract_no_coerce(ob)?)) + } } diff --git a/crates/circuit/src/imports.rs b/crates/circuit/src/imports.rs index 7d17aba2440..241292d76c6 100644 --- a/crates/circuit/src/imports.rs +++ b/crates/circuit/src/imports.rs @@ -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 diff --git a/crates/circuit/src/operations.rs b/crates/circuit/src/operations.rs index caf905a8c03..7134662e1ac 100644 --- a/crates/circuit/src/operations.rs +++ b/crates/circuit/src/operations.rs @@ -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)] @@ -37,17 +37,13 @@ pub enum Param { impl<'py> FromPyObject<'py> for Param { fn extract_bound(b: &Bound<'py, PyAny>) -> Result { - 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::() { - 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::() { + Param::Float(val) + } else { + Param::Obj(b.clone().unbind()) + }) } } @@ -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> { + 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) -> PyResult { + Ok(if ob.is_instance_of::() { + 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>); +impl<'py> Iterator for ParamParameterIter<'py> { + type Item = PyResult>; + + fn next(&mut self) -> Option { + 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 { diff --git a/crates/circuit/src/parameter_table.rs b/crates/circuit/src/parameter_table.rs index 9e5b3124539..8825fbd7177 100644 --- a/crates/circuit/src/parameter_table.rs +++ b/crates/circuit/src/parameter_table.rs @@ -10,164 +10,395 @@ // copyright notice, and modified files need to carry a notice indicating // that they have been altered from the originals. +use hashbrown::hash_map::Entry; +use hashbrown::{HashMap, HashSet}; +use thiserror::Error; + +use pyo3::exceptions::PyTypeError; use pyo3::prelude::*; -use pyo3::{import_exception, intern, PyObject}; +use pyo3::pybacked::PyBackedStr; +use pyo3::types::{PyList, PySet}; +use pyo3::{import_exception, intern, PyTraverseError, PyVisit}; -import_exception!(qiskit.circuit.exceptions, CircuitError); +use crate::imports::UUID; -use hashbrown::{HashMap, HashSet}; +import_exception!(qiskit.circuit, CircuitError); + +#[derive(Error, Debug)] +pub enum ParameterTableError { + #[error("parameter '{0:?}' is not tracked in the table")] + ParameterNotTracked(ParameterUuid), + #[error("usage {0:?} is not tracked by the table")] + UsageNotTracked(ParameterUse), +} +impl From for PyErr { + fn from(value: ParameterTableError) -> PyErr { + CircuitError::new_err(value.to_string()) + } +} + +/// A single use of a symbolic parameter. +#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)] +pub enum ParameterUse { + Index { instruction: usize, parameter: u32 }, + GlobalPhase, +} + +/// Rust-space extra information that a `ParameterVectorElement` has. This is used most heavily +/// during sorting; vector elements are sorted by their parent name, and index within that. +#[derive(Clone, Debug)] +struct VectorElement { + vector_uuid: VectorUuid, + index: usize, +} + +/// Tracked data tied to each parameter's UUID in the table. +#[derive(Clone, Debug)] +pub struct ParameterInfo { + uses: HashSet, + name: PyBackedStr, + element: Option, + object: Py, +} -/// The index value in a `ParamEntry` that indicates the global phase. -pub const GLOBAL_PHASE_INDEX: usize = usize::MAX; +/// Rust-space information on a Python `ParameterVector` and its uses in the table. +#[derive(Clone, Debug)] +struct VectorInfo { + name: PyBackedStr, + /// Number of elements of the vector tracked within the parameter table. + refcount: usize, +} -#[pyclass(freelist = 20, module = "qiskit._accelerate.circuit")] -pub(crate) struct ParamEntryKeys { - keys: Vec<(usize, usize)>, - iter_pos: usize, +/// Type-safe UUID for a symbolic parameter. This does not track the name of the `Parameter`; it +/// can't be used alone to reconstruct a Python instance. That tracking remains only withing the +/// `ParameterTable`. +#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)] +pub struct ParameterUuid(u128); +impl ParameterUuid { + /// Extract a UUID from a Python-space `Parameter` object. This assumes that the object is known + /// to be a parameter. + pub fn from_parameter(ob: &Bound) -> PyResult { + ob.getattr(intern!(ob.py(), "_uuid"))?.extract() + } } -#[pymethods] -impl ParamEntryKeys { - fn __iter__(slf: PyRef) -> Py { - slf.into() +/// This implementation of `FromPyObject` is for the UUID itself, which is what the `ParameterUuid` +/// struct actually encapsulates. +impl<'py> FromPyObject<'py> for ParameterUuid { + fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult { + if ob.is_exact_instance(UUID.get_bound(ob.py())) { + ob.getattr(intern!(ob.py(), "int"))?.extract().map(Self) + } else { + Err(PyTypeError::new_err("not a UUID")) + } } +} - fn __next__(mut slf: PyRefMut) -> Option<(usize, usize)> { - if slf.iter_pos < slf.keys.len() { - let res = Some(slf.keys[slf.iter_pos]); - slf.iter_pos += 1; - res +/// Type-safe UUID for a parameter vector. This is just used internally for tracking. +#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)] +struct VectorUuid(u128); +impl VectorUuid { + /// Extract a UUID from a Python-space `ParameterVector` object. This assumes that the object is + /// the correct type. + fn from_vector(ob: &Bound) -> PyResult { + ob.getattr(intern!(ob.py(), "_root_uuid"))?.extract() + } +} +impl<'py> FromPyObject<'py> for VectorUuid { + fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult { + if ob.is_exact_instance(UUID.get_bound(ob.py())) { + ob.getattr(intern!(ob.py(), "int"))?.extract().map(Self) } else { - None + Err(PyTypeError::new_err("not a UUID")) } } } -#[derive(Clone, Debug)] -#[pyclass(freelist = 20, module = "qiskit._accelerate.circuit")] -pub(crate) struct ParamEntry { - /// Mapping of tuple of instruction index (in CircuitData) and parameter index to the actual - /// parameter object - pub index_ids: HashSet<(usize, usize)>, +#[derive(Clone, Default, Debug)] +pub struct ParameterTable { + /// Mapping of the parameter key (its UUID) to the information on it tracked by this table. + by_uuid: HashMap, + /// Mapping of the parameter names to the UUID that represents them. Since we always get + /// parameters in as Python-space objects, we use the string object extracted from Python space. + by_name: HashMap, + /// Additional information on any `ParameterVector` instances that have elements in the circuit. + vectors: HashMap, + /// Sort order of the parameters. This is lexicographical for most parameters, except elements + /// of a `ParameterVector` are sorted within the vector by numerical index. We calculate this + /// on demand and cache it; an empty `order` implies it is not currently calculated. We don't + /// use `Option` so we can re-use the allocation for partial parameter bindings. + /// + /// Any method that adds or a removes a parameter is responsible for invalidating this cache. + order: Vec, + /// Cache of a Python-space list of the parameter objects, in order. We only generate this + /// specifically when asked. + /// + /// Any method that adds or a removes a parameter is responsible for invalidating this cache. + py_parameters: Option>, } -impl ParamEntry { - pub fn add(&mut self, inst_index: usize, param_index: usize) { - self.index_ids.insert((inst_index, param_index)); +impl ParameterTable { + pub fn new() -> Self { + Default::default() } - pub fn discard(&mut self, inst_index: usize, param_index: usize) { - self.index_ids.remove(&(inst_index, param_index)); + /// Get the number of parameters tracked by the table. + pub fn num_parameters(&self) -> usize { + self.by_uuid.len() } -} -#[pymethods] -impl ParamEntry { - #[new] - pub fn new(inst_index: usize, param_index: usize) -> Self { - ParamEntry { - index_ids: HashSet::from([(inst_index, param_index)]), + /// Add a new usage of a parameter coming in from Python space, optionally adding a first usage + /// to it. + /// + /// The no-use form is useful when doing parameter assignments from Rust space, where the + /// replacement is itself parametric; the replacement can be extracted once, then subsequent + /// lookups and updates done without interaction with Python. + pub fn track( + &mut self, + param_ob: &Bound, + usage: Option, + ) -> PyResult { + let py = param_ob.py(); + let uuid = ParameterUuid::from_parameter(param_ob)?; + match self.by_uuid.entry(uuid) { + Entry::Occupied(mut entry) => { + if let Some(usage) = usage { + entry.get_mut().uses.insert(usage); + } + } + Entry::Vacant(entry) => { + let py_name_attr = intern!(py, "name"); + let name = param_ob.getattr(py_name_attr)?.extract::()?; + if self.by_name.contains_key(&name) { + return Err(CircuitError::new_err(format!( + "name conflict adding parameter '{}'", + &name + ))); + } + let element = if let Ok(vector) = param_ob.getattr(intern!(py, "vector")) { + let vector_uuid = VectorUuid::from_vector(&vector)?; + match self.vectors.entry(vector_uuid) { + Entry::Occupied(mut entry) => entry.get_mut().refcount += 1, + Entry::Vacant(entry) => { + entry.insert(VectorInfo { + name: vector.getattr(py_name_attr)?.extract()?, + refcount: 1, + }); + } + } + Some(VectorElement { + vector_uuid, + index: param_ob.getattr(intern!(py, "index"))?.extract()?, + }) + } else { + None + }; + self.by_name.insert(name.clone(), uuid); + self.order.clear(); + self.py_parameters = None; + let mut uses = HashSet::new(); + if let Some(usage) = usage { + uses.insert_unique_unchecked(usage); + }; + entry.insert(ParameterInfo { + name, + uses, + element, + object: param_ob.clone().unbind(), + }); + } } + Ok(uuid) } - pub fn __len__(&self) -> usize { - self.index_ids.len() + /// Untrack one use of a single Python-space `Parameter` object from the table, discarding all + /// other tracking of that `Parameter` if this was the last usage of it. + pub fn untrack(&mut self, param_ob: &Bound, usage: ParameterUse) -> PyResult<()> { + self.remove_use(ParameterUuid::from_parameter(param_ob)?, usage) + .map_err(PyErr::from) } - pub fn __contains__(&self, key: (usize, usize)) -> bool { - self.index_ids.contains(&key) + /// Lookup the Python parameter object by name. + pub fn py_parameter_by_name(&self, name: &PyBackedStr) -> Option<&Py> { + self.by_name + .get(name) + .map(|uuid| &self.by_uuid[uuid].object) } - pub fn __iter__(&self) -> ParamEntryKeys { - ParamEntryKeys { - keys: self.index_ids.iter().copied().collect(), - iter_pos: 0, + /// Get the (maybe cached) Python list of the sorted `Parameter` objects. + pub fn py_parameters<'py>(&mut self, py: Python<'py>) -> Bound<'py, PyList> { + if let Some(py_parameters) = self.py_parameters.as_ref() { + return py_parameters.clone_ref(py).into_bound(py); } + self.ensure_sorted(); + let out = PyList::new_bound( + py, + self.order + .iter() + .map(|uuid| self.by_uuid[uuid].object.clone_ref(py).into_bound(py)), + ); + self.py_parameters = Some(out.clone().unbind()); + out } -} -#[derive(Clone, Debug)] -#[pyclass(freelist = 20, module = "qiskit._accelerate.circuit")] -pub(crate) struct ParamTable { - /// Mapping of parameter uuid (as an int) to the Parameter Entry - pub table: HashMap, - /// Mapping of parameter name to uuid as an int - pub names: HashMap, - /// Mapping of uuid to a parameter object - pub uuid_map: HashMap, -} + /// Get a Python set of all tracked `Parameter` objects. + pub fn py_parameters_unsorted<'py>(&self, py: Python<'py>) -> PyResult> { + PySet::new_bound(py, self.by_uuid.values().map(|info| &info.object)) + } -impl ParamTable { - pub fn insert(&mut self, py: Python, parameter: PyObject, entry: ParamEntry) -> PyResult<()> { - let uuid: u128 = parameter - .getattr(py, intern!(py, "_uuid"))? - .getattr(py, intern!(py, "int"))? - .extract(py)?; - let name: String = parameter.getattr(py, intern!(py, "name"))?.extract(py)?; - - if self.names.contains_key(&name) && !self.table.contains_key(&uuid) { - return Err(CircuitError::new_err(format!( - "Name conflict on adding parameter: {}", - name - ))); + /// Ensure that the `order` field is populated and sorted. + fn ensure_sorted(&mut self) { + // If `order` is already populated, it's sorted; it's the responsibility of the methods of + // this struct that mutate it to invalidate the cache. + if !self.order.is_empty() { + return; } - self.table.insert(uuid, entry); - self.names.insert(name, uuid); - self.uuid_map.insert(uuid, parameter); + self.order.reserve(self.by_uuid.len()); + self.order.extend(self.by_uuid.keys()); + self.order.sort_unstable_by_key(|uuid| { + let info = &self.by_uuid[uuid]; + if let Some(vec) = info.element.as_ref() { + (&self.vectors[&vec.vector_uuid].name, vec.index) + } else { + (&info.name, 0) + } + }) + } + + /// Add a use of a parameter to the table. + pub fn add_use( + &mut self, + uuid: ParameterUuid, + usage: ParameterUse, + ) -> Result<(), ParameterTableError> { + self.by_uuid + .get_mut(&uuid) + .ok_or(ParameterTableError::ParameterNotTracked(uuid))? + .uses + .insert(usage); Ok(()) } - pub fn discard_references( + /// Return a use of a parameter. + /// + /// If the last use a parameter is discarded, the parameter is untracked. + pub fn remove_use( &mut self, - uuid: u128, - inst_index: usize, - param_index: usize, - name: String, - ) { - if let Some(refs) = self.table.get_mut(&uuid) { - if refs.__len__() == 1 { - self.table.remove(&uuid); - self.names.remove(&name); - self.uuid_map.remove(&uuid); - } else { - refs.discard(inst_index, param_index); + uuid: ParameterUuid, + usage: ParameterUse, + ) -> Result<(), ParameterTableError> { + let Entry::Occupied(mut entry) = self.by_uuid.entry(uuid) else { + return Err(ParameterTableError::ParameterNotTracked(uuid)); + }; + let info = entry.get_mut(); + if !info.uses.remove(&usage) { + return Err(ParameterTableError::UsageNotTracked(usage)); + } + if info.uses.is_empty() { + self.by_name.remove(&info.name); + if let Some(vec) = info.element.as_ref() { + let Entry::Occupied(mut vec_entry) = self.vectors.entry(vec.vector_uuid) else { + unreachable!() + }; + vec_entry.get_mut().refcount -= 1; + if vec_entry.get().refcount == 0 { + vec_entry.remove_entry(); + } } + self.order.clear(); + self.py_parameters = None; + entry.remove_entry(); } + Ok(()) } -} -#[pymethods] -impl ParamTable { - #[new] - pub fn new() -> Self { - ParamTable { - table: HashMap::new(), - names: HashMap::new(), - uuid_map: HashMap::new(), + /// Remove a parameter from the table, returning the tracked uses of it. + pub fn pop( + &mut self, + uuid: ParameterUuid, + ) -> Result, ParameterTableError> { + let info = self + .by_uuid + .remove(&uuid) + .ok_or(ParameterTableError::ParameterNotTracked(uuid))?; + self.by_name + .remove(&info.name) + .expect("each parameter should be tracked by both UUID and name"); + if let Some(element) = info.element { + self.vectors + .entry(element.vector_uuid) + .and_replace_entry_with(|_k, mut vector_info| { + vector_info.refcount -= 1; + (vector_info.refcount > 0).then_some(vector_info) + }); } + self.order.clear(); + self.py_parameters = None; + Ok(info.uses) } - pub fn clear(&mut self) { - self.table.clear(); - self.names.clear(); - self.uuid_map.clear(); + /// Clear this table, yielding the Python parameter objects and their uses in sorted order. + pub fn drain_ordered( + &'_ mut self, + ) -> impl Iterator, HashSet)> + '_ { + self.ensure_sorted(); + self.by_name.clear(); + self.vectors.clear(); + self.py_parameters = None; + self.order.drain(..).map(|uuid| { + let info = self + .by_uuid + .remove(&uuid) + .expect("tracked UUIDs should be consistent"); + (info.object, info.uses) + }) } - pub fn pop(&mut self, key: u128, name: &str) -> Option { - self.names.remove(name); - self.uuid_map.remove(&key); - self.table.remove(&key) + /// Empty this `ParameterTable` of all its contents. This does not affect the capacities of the + /// internal data storage. + pub fn clear(&mut self) { + self.by_uuid.clear(); + self.by_name.clear(); + self.vectors.clear(); + self.order.clear(); + self.py_parameters = None; } - fn set(&mut self, uuid: u128, name: String, param: PyObject, refs: ParamEntry) { - self.names.insert(name, uuid); - self.table.insert(uuid, refs); - self.uuid_map.insert(uuid, param); + /// Expose the tracked data for a given parameter as directly as possible to Python space. + /// + /// This is only really intended for use in testing. + pub(crate) fn _py_raw_entry(&self, param: Bound) -> PyResult> { + let py = param.py(); + let uuid = ParameterUuid::from_parameter(¶m)?; + let info = self + .by_uuid + .get(&uuid) + .ok_or(ParameterTableError::ParameterNotTracked(uuid))?; + // PyO3's `PySet::new_bound` only accepts iterables of references. + let out = PySet::empty_bound(py)?; + for usage in info.uses.iter() { + match usage { + ParameterUse::GlobalPhase => out.add((py.None(), py.None()))?, + ParameterUse::Index { + instruction, + parameter, + } => out.add((*instruction, *parameter))?, + } + } + Ok(out.unbind()) } - pub fn get_param_from_name(&self, py: Python, name: String) -> Option { - self.names - .get(&name) - .map(|x| self.uuid_map.get(x).map(|y| y.clone_ref(py)))? + /// Accept traversal of this object by the Python garbage collector. + /// + /// This is not a pyclass, so it's up to our owner to delegate their own traversal to us. + pub fn py_gc_traverse(&self, visit: &PyVisit) -> Result<(), PyTraverseError> { + for info in self.by_uuid.values() { + visit.call(&info.object)? + } + // We don't need to / can't visit the `PyBackedStr` stores. + if let Some(list) = self.py_parameters.as_ref() { + visit.call(list)? + } + Ok(()) } } diff --git a/qiskit/circuit/_utils.py b/qiskit/circuit/_utils.py index 86a058e8852..db49cbf9057 100644 --- a/qiskit/circuit/_utils.py +++ b/qiskit/circuit/_utils.py @@ -19,19 +19,6 @@ from qiskit import _numpy_compat from qiskit.exceptions import QiskitError from qiskit.circuit.exceptions import CircuitError -from .parametervector import ParameterVectorElement - - -def sort_parameters(parameters): - """Sort an iterable of :class:`.Parameter` instances into a canonical order, respecting the - ordering relationships between elements of :class:`.ParameterVector`\\ s.""" - - def key(parameter): - if isinstance(parameter, ParameterVectorElement): - return (parameter.vector.name, parameter.index) - return (parameter.name,) - - return sorted(parameters, key=key) def _compute_control_matrix(base_mat, num_ctrl_qubits, ctrl_state=None): diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py index 3b2da762c51..ad635337335 100644 --- a/qiskit/circuit/quantumcircuit.py +++ b/qiskit/circuit/quantumcircuit.py @@ -15,6 +15,8 @@ """Quantum circuit object.""" from __future__ import annotations + +import collections.abc import copy as _copy import itertools import multiprocessing as mp @@ -46,7 +48,6 @@ from qiskit.circuit.exceptions import CircuitError from qiskit.utils import deprecate_func from . import _classical_resource_map -from ._utils import sort_parameters from .controlflow import ControlFlowOp, _builder_utils from .controlflow.builder import CircuitScopeInterface, ControlFlowBuilderBlock from .controlflow.break_loop import BreakLoopOp, BreakLoopPlaceholder @@ -1127,9 +1128,6 @@ def __init__( self._calibrations: DefaultDict[str, dict[tuple, Any]] = defaultdict(dict) self.add_register(*regs) - # Cache to avoid re-sorting parameters - self._parameters = None - self._layout = None self.global_phase = global_phase @@ -1266,7 +1264,6 @@ def data(self, data_input: Iterable): else: data_input = list(data_input) self._data.clear() - self._parameters = None # Repopulate the parameter table with any global-phase entries. self.global_phase = self.global_phase if not data_input: @@ -2568,9 +2565,7 @@ def _append(self, instruction, qargs=(), cargs=(), *, _standard_gate: bool = Fal :meta public: """ if _standard_gate: - new_param = self._data.append(instruction) - if new_param: - self._parameters = None + self._data.append(instruction) self.duration = None self.unit = "dt" return instruction @@ -2582,19 +2577,16 @@ def _append(self, instruction, qargs=(), cargs=(), *, _standard_gate: bool = Fal # instruction param the inner rust append method will raise a runtime error. # When this happens we need to handle the parameters separately. # This shouldn't happen in practice but 2 tests were doing this and it's not - # explicitly prohibted by the API so this and the `params` optional argument - # path guard against it. + # explicitly prohibted by the API. try: - new_param = self._data.append(instruction) + self._data.append(instruction) except RuntimeError: - params = [] - for idx, param in enumerate(instruction.operation.params): - if isinstance(param, (ParameterExpression, QuantumCircuit)): - params.append((idx, list(set(param.parameters)))) - new_param = self._data.append(instruction, params) - if new_param: - # clear cache if new parameter is added - self._parameters = None + params = [ + (idx, param.parameters) + for idx, param in enumerate(instruction.operation.params) + if isinstance(param, (ParameterExpression, QuantumCircuit)) + ] + self._data.append_manual_params(instruction, params) # Invalidate whole circuit duration if an instruction is added self.duration = None @@ -2650,7 +2642,7 @@ def get_parameter(self, name: str, default: typing.Any = ...) -> Parameter: A similar method, but for :class:`.expr.Var` run-time variables instead of :class:`.Parameter` compile-time parameters. """ - if (parameter := self._data.get_param_from_name(name)) is None: + if (parameter := self._data.get_parameter_by_name(name)) is None: if default is Ellipsis: raise KeyError(f"no parameter named '{name}' is present") return default @@ -3722,8 +3714,6 @@ def copy_empty_like( cpy._data = CircuitData( self._data.qubits, self._data.clbits, global_phase=self._data.global_phase ) - # Invalidate parameters caching. - cpy._parameters = None cpy._calibrations = _copy.deepcopy(self._calibrations) cpy._metadata = _copy.deepcopy(self._metadata) @@ -4110,16 +4100,6 @@ def global_phase(self, angle: ParameterValueType): Args: angle (float, ParameterExpression): radians """ - # If we're currently parametric, we need to throw away the references. This setter is - # called by some subclasses before the inner `_global_phase` is initialized. - if isinstance(getattr(self._data, "global_phase", None), ParameterExpression): - self._parameters = None - if isinstance(angle, ParameterExpression): - if angle.parameters: - self._parameters = None - else: - angle = _normalize_global_phase(angle) - if self._control_flow_scopes: self._control_flow_scopes[-1].global_phase = angle else: @@ -4184,16 +4164,13 @@ def parameters(self) -> ParameterView: Returns: The sorted :class:`.Parameter` objects in the circuit. """ - # parameters from gates - if self._parameters is None: - self._parameters = sort_parameters(self._unsorted_parameters()) # return as parameter view, which implements the set and list interface - return ParameterView(self._parameters) + return ParameterView(self._data.parameters) @property def num_parameters(self) -> int: """The number of parameter objects in the circuit.""" - return self._data.num_params() + return self._data.num_parameters() def _unsorted_parameters(self) -> set[Parameter]: """Efficiently get all parameters in the circuit, without any sorting overhead. @@ -4206,7 +4183,7 @@ def _unsorted_parameters(self) -> set[Parameter]: """ # This should be free, by accessing the actual backing data structure of the table, but that # means that we need to copy it if adding keys from the global phase. - return self._data.get_params_unsorted() + return self._data.unsorted_parameters() @overload def assign_parameters( @@ -4319,108 +4296,28 @@ def assign_parameters( # pylint: disable=missing-raises-doc if inplace: target = self else: + if not isinstance(parameters, dict): + # We're going to need to access the sorted order wihin the inner Rust method on + # `target`, so warm up our own cache first so that subsequent calls to + # `assign_parameters` on `self` benefit as well. + _ = self._data.parameters target = self.copy() target._increment_instances() target._name_update() - # Normalize the inputs into simple abstract interfaces, so we've dispatched the "iteration" - # logic in one place at the start of the function. This lets us do things like calculate - # and cache expensive properties for (e.g.) the sequence format only if they're used; for - # many large, close-to-hardware circuits, we won't need the extra handling for - # `global_phase` or recursive definition binding. - # - # During normalisation, be sure to reference 'parameters' and related things from 'self' not - # 'target' so we can take advantage of any caching we might be doing. - if isinstance(parameters, dict): + if isinstance(parameters, collections.abc.Mapping): raw_mapping = parameters if flat_input else self._unroll_param_dict(parameters) - # Remember that we _must not_ mutate the output of `_unsorted_parameters`. - our_parameters = self._unsorted_parameters() + our_parameters = self._data.unsorted_parameters() if strict and (extras := raw_mapping.keys() - our_parameters): raise CircuitError( f"Cannot bind parameters ({', '.join(str(x) for x in extras)}) not present in" " the circuit." ) parameter_binds = _ParameterBindsDict(raw_mapping, our_parameters) + target._data.assign_parameters_mapping(parameter_binds) else: - our_parameters = self.parameters - if len(parameters) != len(our_parameters): - raise ValueError( - "Mismatching number of values and parameters. For partial binding " - "please pass a dictionary of {parameter: value} pairs." - ) - parameter_binds = _ParameterBindsSequence(our_parameters, parameters) - - # Clear out the parameter table for the relevant entries, since we'll be binding those. - # Any new references to parameters are reinserted as part of the bind. - target._parameters = None - # This is deliberately eager, because we want the side effect of clearing the table. - all_references = [ - (parameter, value, target._data.pop_param(parameter.uuid.int, parameter.name, ())) - for parameter, value in parameter_binds.items() - ] - seen_operations = {} - # The meat of the actual binding for regular operations. - for to_bind, bound_value, references in all_references: - update_parameters = ( - tuple(bound_value.parameters) - if isinstance(bound_value, ParameterExpression) - else () - ) - for inst_index, index in references: - if inst_index == self._data.global_phase_param_index: - operation = None - seen_operations[inst_index] = None - assignee = target.global_phase - validate = _normalize_global_phase - else: - operation = target._data[inst_index].operation - seen_operations[inst_index] = operation - assignee = operation.params[index] - validate = operation.validate_parameter - if isinstance(assignee, ParameterExpression): - new_parameter = assignee.assign(to_bind, bound_value) - for parameter in update_parameters: - if not target._data.contains_param(parameter.uuid.int): - target._data.add_new_parameter(parameter, inst_index, index) - else: - target._data.update_parameter_entry( - parameter.uuid.int, - inst_index, - index, - ) - if not new_parameter.parameters: - new_parameter = validate(new_parameter.numeric()) - elif isinstance(assignee, QuantumCircuit): - new_parameter = assignee.assign_parameters( - {to_bind: bound_value}, inplace=False, flat_input=True - ) - else: - raise RuntimeError( # pragma: no cover - f"Saw an unknown type during symbolic binding: {assignee}." - " This may indicate an internal logic error in symbol tracking." - ) - if inst_index == self._data.global_phase_param_index: - # We've already handled parameter table updates in bulk, so we need to skip the - # public setter trying to do it again. - target._data.global_phase = new_parameter - else: - temp_params = operation.params - temp_params[index] = new_parameter - operation.params = temp_params - target._data.setitem_no_param_table_update( - inst_index, - target._data[inst_index].replace(operation=operation, params=temp_params), - ) - - # After we've been through everything at the top level, make a single visit to each - # operation we've seen, rebinding its definition if necessary. - for operation in seen_operations.values(): - if ( - definition := getattr(operation, "_definition", None) - ) is not None and definition.num_parameters: - definition.assign_parameters( - parameter_binds.mapping, inplace=True, flat_input=True, strict=False - ) + parameter_binds = _ParameterBindsSequence(target._data.parameters, parameters) + target._data.assign_parameters_sequence(parameters) # Finally, assign the parameters inside any of the calibrations. We don't track these in # the `ParameterTable`, so we manually reconstruct things. @@ -4457,7 +4354,6 @@ def map_calibration(qubits, parameters, schedule): for gate, calibrations in target._calibrations.items() ), ) - target._parameters = None return None if inplace else target def _unroll_param_dict( @@ -6054,7 +5950,6 @@ def _pop_previous_instruction_in_scope(self) -> CircuitInstruction: if not self._data: raise CircuitError("This circuit contains no instructions.") instruction = self._data.pop() - self._parameters = None return instruction @typing.overload @@ -6708,7 +6603,6 @@ def append(self, instruction, *, _standard_gate: bool = False): def extend(self, data: CircuitData): self.circuit._data.extend(data) - self.circuit._parameters = None self.circuit.duration = None self.circuit.unit = "dt" @@ -6867,11 +6761,3 @@ def _bit_argument_conversion_scalar(specifier, bit_sequence, bit_set, type_): else f"Invalid bit index: '{specifier}' of type '{type(specifier)}'" ) raise CircuitError(message) - - -def _normalize_global_phase(angle): - """Return the normalized form of an angle for use in the global phase. This coerces to float if - possible, and fixes to the interval :math:`[0, 2\\pi)`.""" - if isinstance(angle, ParameterExpression) and angle.parameters: - return angle - return float(angle) % (2.0 * np.pi) diff --git a/qiskit/qasm3/exporter.py b/qiskit/qasm3/exporter.py index b7a4aaea65a..098ab8578d4 100644 --- a/qiskit/qasm3/exporter.py +++ b/qiskit/qasm3/exporter.py @@ -1196,7 +1196,9 @@ def is_loop_variable(circuit, parameter): # _should_ be an intrinsic part of the parameter, or somewhere publicly accessible, but # Terra doesn't have those concepts yet. We can only try and guess at the type by looking # at all the places it's used in the circuit. - for instr_index, index in circuit._data._get_param(parameter.uuid.int): + for instr_index, index in circuit._data._raw_parameter_table_entry(parameter): + if instr_index is None: + continue instruction = circuit.data[instr_index].operation if isinstance(instruction, ForLoopOp): # The parameters of ForLoopOp are (indexset, loop_parameter, body). diff --git a/test/python/circuit/library/test_blueprintcircuit.py b/test/python/circuit/library/test_blueprintcircuit.py index 5f0a2814872..e7f67dae2c8 100644 --- a/test/python/circuit/library/test_blueprintcircuit.py +++ b/test/python/circuit/library/test_blueprintcircuit.py @@ -77,17 +77,17 @@ def test_invalidate_rebuild(self): with self.subTest(msg="after building"): self.assertGreater(len(mock._data), 0) - self.assertEqual(mock._data.num_params(), 1) + self.assertEqual(mock._data.num_parameters(), 1) mock._invalidate() with self.subTest(msg="after invalidating"): self.assertFalse(mock._is_built) - self.assertEqual(mock._data.num_params(), 0) + self.assertEqual(mock._data.num_parameters(), 0) mock._build() with self.subTest(msg="after re-building"): self.assertGreater(len(mock._data), 0) - self.assertEqual(mock._data.num_params(), 1) + self.assertEqual(mock._data.num_parameters(), 1) def test_calling_attributes_works(self): """Test that the circuit is constructed when attributes are called.""" diff --git a/test/python/circuit/library/test_pauli_feature_map.py b/test/python/circuit/library/test_pauli_feature_map.py index c43c2b5f6f1..60b7e5c28b9 100644 --- a/test/python/circuit/library/test_pauli_feature_map.py +++ b/test/python/circuit/library/test_pauli_feature_map.py @@ -213,9 +213,9 @@ def test_parameter_prefix(self): "ParameterView([ParameterVectorElement(x[0]), ParameterVectorElement(x[1])])", ) - encoding_pauli_param_y = encoding_pauli.assign_parameters({1, y}) - encoding_z_param_y = encoding_z.assign_parameters({1, y}) - encoding_zz_param_y = encoding_zz.assign_parameters({1, y}) + encoding_pauli_param_y = encoding_pauli.assign_parameters([1, y]) + encoding_z_param_y = encoding_z.assign_parameters([1, y]) + encoding_zz_param_y = encoding_zz.assign_parameters([1, y]) self.assertEqual(str(encoding_pauli_param_y.parameters), "ParameterView([Parameter(y)])") self.assertEqual(str(encoding_z_param_y.parameters), "ParameterView([Parameter(y)])") diff --git a/test/python/circuit/test_circuit_data.py b/test/python/circuit/test_circuit_data.py index e75d67ed5dc..ff241baeb61 100644 --- a/test/python/circuit/test_circuit_data.py +++ b/test/python/circuit/test_circuit_data.py @@ -859,6 +859,6 @@ def test_param_gate_instance(self): # A fancy way of doing qc0_instance = qc0.data[0] and qc1_instance = qc1.data[0] # but this at least verifies the parameter table is point from the parameter to # the correct instruction (which is the only one) - qc0_instance = qc0._data[next(iter(qc0._data._get_param(b.uuid.int)))[0]] - qc1_instance = qc1._data[next(iter(qc1._data._get_param(a.uuid.int)))[0]] + qc0_instance = qc0._data[next(iter(qc0._data._raw_parameter_table_entry(b)))[0]] + qc1_instance = qc1._data[next(iter(qc1._data._raw_parameter_table_entry(a)))[0]] self.assertNotEqual(qc0_instance, qc1_instance) diff --git a/test/python/circuit/test_circuit_operations.py b/test/python/circuit/test_circuit_operations.py index 517a7093e81..f374f82504a 100644 --- a/test/python/circuit/test_circuit_operations.py +++ b/test/python/circuit/test_circuit_operations.py @@ -593,7 +593,7 @@ def test_clear_circuit(self): qc.clear() self.assertEqual(len(qc.data), 0) - self.assertEqual(qc._data.num_params(), 0) + self.assertEqual(qc._data.num_parameters(), 0) def test_barrier(self): """Test multiple argument forms of barrier.""" diff --git a/test/python/circuit/test_parameters.py b/test/python/circuit/test_parameters.py index ad336a0ad6b..2261d903a63 100644 --- a/test/python/circuit/test_parameters.py +++ b/test/python/circuit/test_parameters.py @@ -54,7 +54,9 @@ def raise_if_parameter_table_invalid(circuit): for parameter in param.parameters if isinstance(param, ParameterExpression) } - table_parameters = set(circuit._data.get_params_unsorted()) + if isinstance(circuit.global_phase, ParameterExpression): + circuit_parameters |= circuit.global_phase.parameters + table_parameters = set(circuit._data.unsorted_parameters()) if circuit_parameters != table_parameters: raise CircuitError( @@ -67,24 +69,31 @@ def raise_if_parameter_table_invalid(circuit): circuit_instructions = [instr.operation for instr in circuit._data] for parameter in table_parameters: - instr_list = circuit._data._get_param(parameter.uuid.int) + instr_list = circuit._data._raw_parameter_table_entry(parameter) for instr_index, param_index in instr_list: - instr = circuit.data[instr_index].operation - if instr not in circuit_instructions: - raise CircuitError(f"ParameterTable instruction not present in circuit: {instr}.") - - if not isinstance(instr.params[param_index], ParameterExpression): + if instr_index is None: + # Global phase. + expression = circuit.global_phase + instr = "" + else: + instr = circuit.data[instr_index].operation + if instr not in circuit_instructions: + raise CircuitError( + f"ParameterTable instruction not present in circuit: {instr}." + ) + expression = instr.params[param_index] + + if not isinstance(expression, ParameterExpression): raise CircuitError( "ParameterTable instruction does not have a " f"ParameterExpression at param_index {param_index}: {instr}." ) - if parameter not in instr.params[param_index].parameters: + if parameter not in expression.parameters: raise CircuitError( - "ParameterTable instruction parameters does " - "not match ParameterTable key. Instruction " - f"parameters: {instr.params[param_index].parameters}" - f" ParameterTable key: {parameter}." + "ParameterTable instruction parameters does not match ParameterTable key." + f"\nInstruction parameters: {expression.parameters}" + f"\nParameterTable key: {parameter}." ) # Assert circuit has no other parameter locations other than those in table. @@ -94,8 +103,8 @@ def raise_if_parameter_table_invalid(circuit): parameters = param.parameters for parameter in parameters: - if (instr_index, param_index) not in circuit._data._get_param( - parameter.uuid.int + if (instr_index, param_index) not in circuit._data._raw_parameter_table_entry( + parameter ): raise CircuitError( "Found parameterized instruction not " @@ -183,9 +192,10 @@ def test_parameters_property(self): qc = QuantumCircuit(qr) rxg = RXGate(theta) qc.append(rxg, [qr[0]], []) - self.assertEqual(qc._data.num_params(), 1) - self.assertIs(theta, next(iter(qc._data.get_params_unsorted()))) - self.assertEqual(rxg, qc.data[next(iter(qc._data._get_param(theta.uuid.int)))[0]].operation) + self.assertEqual(qc._data.num_parameters(), 1) + self.assertIs(theta, next(iter(qc._data.unsorted_parameters()))) + ((instruction_index, _),) = list(qc._data._raw_parameter_table_entry(theta)) + self.assertEqual(rxg, qc.data[instruction_index].operation) def test_parameters_property_by_index(self): """Test getting parameters by index""" @@ -581,12 +591,12 @@ def test_two_parameter_expression_binding(self): qc.rx(theta, 0) qc.ry(phi, 0) - self.assertEqual(qc._data._get_entry_count(theta), 1) - self.assertEqual(qc._data._get_entry_count(phi), 1) + self.assertEqual(len(qc._data._raw_parameter_table_entry(theta)), 1) + self.assertEqual(len(qc._data._raw_parameter_table_entry(phi)), 1) qc.assign_parameters({theta: -phi}, inplace=True) - self.assertEqual(qc._data._get_entry_count(phi), 2) + self.assertEqual(len(qc._data._raw_parameter_table_entry(phi)), 2) def test_expression_partial_binding_zero(self): """Verify that binding remains possible even if a previous partial bind @@ -610,6 +620,47 @@ def test_expression_partial_binding_zero(self): self.assertEqual(fbqc.parameters, set()) self.assertEqual(float(fbqc.data[0].operation.params[0]), 0) + def test_assignment_to_annotated_operation(self): + """Test that assignments to an ``AnnotatedOperation`` are propagated all the way down.""" + + class MyGate(Gate): + """Arbitrary non-standard gate.""" + + def __init__(self, param): + super().__init__("my_gate", 1, [param]) + # Eagerly create our definition. + _ = self.definition + + def _define(self): + self._definition = QuantumCircuit(1, name="my_gate_inner") + self._definition.ry(self.params[0], 0) + + theta = Parameter("theta") + + # Sanity check for the test; it won't catch errors if this fails. + self.assertEqual(MyGate(theta), MyGate(theta)) + self.assertNotEqual(MyGate(theta), MyGate(1.23)) + + parametric_gate = MyGate(theta) + qc = QuantumCircuit(2) + qc.append(parametric_gate.control(1, annotated=True), [0, 1], copy=True) + assigned = qc.assign_parameters([1.23]) + + expected = QuantumCircuit(2) + expected.append(MyGate(1.23).control(1, annotated=True), [0, 1]) + self.assertEqual(assigned, expected) + self.assertEqual( + assigned.data[0].operation.base_op.definition, + expected.data[0].operation.base_op.definition, + ) + + qc.assign_parameters([1.23], inplace=True) + self.assertEqual(qc, expected) + + # Test that the underlying gate was not modified. + self.assertEqual(parametric_gate.params, [theta]) + self.assertEqual(set(parametric_gate.definition.parameters), {theta}) + def test_raise_if_assigning_params_not_in_circuit(self): """Verify binding parameters which are not present in the circuit raises an error.""" x = Parameter("x") @@ -641,7 +692,7 @@ def test_gate_multiplicity_binding(self): qc.append(gate, [0], []) qc.append(gate, [0], []) qc2 = qc.assign_parameters({theta: 1.0}) - self.assertEqual(qc2._data.num_params(), 0) + self.assertEqual(qc2._data.num_parameters(), 0) for instruction in qc2.data: self.assertEqual(float(instruction.operation.params[0]), 1.0) @@ -1384,6 +1435,18 @@ def test_copy_after_dot_data_setter(self): self.assertEqual(qc.parameters, set()) raise_if_parameter_table_invalid(qc) + def test_nonfinal_insert_maintains_valid_table(self): + """Inserts other than appends should still maintain valid tracking, for as long as we + continue to allow non-final inserts.""" + a, b, c = [Parameter(x) for x in "abc"] + qc = QuantumCircuit(1) + qc.global_phase = a / 2 + qc.rz(a, 0) + qc.rz(b + c, 0) + raise_if_parameter_table_invalid(qc) + qc.data.insert(0, qc.data.pop()) + raise_if_parameter_table_invalid(qc) + def test_circuit_with_ufunc(self): """Test construction of circuit and binding of parameters after we apply universal functions."""