Skip to content

Commit

Permalink
Fix quadratic construction time of QuantumCircuit (Qiskit#11517)
Browse files Browse the repository at this point in the history
* Fix quadratic construction time of `QuantumCircuit`

The current implementation of `CircuitData.add_qubit` (ditto `clbit`)
has linear time complexity because it reconstructs the list on each
addition, while the `CircuitData.qubits` getter silently clones the
list on return.  This was intended to avoid inadvertant Python-space
modifications from getting the Rust components out of sync, but in
practice, this cost being hidden in a standard attribute access makes it
very easy to introduce accidental quadratic dependencies.

In this case, `QuantumCircuit(num_qubits)` and subsequent
`qc.append(..., qc.qubits)` calls were accidentally quadratic in
`num_qubits` due to repeated accesses of `QuantumCircuit.qubits`.

* Restore explicit Python token

* Strengthen warnings
  • Loading branch information
jakelishman authored and ShellyGarion committed Jan 18, 2024
1 parent 28778c1 commit ab2e410
Showing 1 changed file with 27 additions and 14 deletions.
41 changes: 27 additions & 14 deletions crates/accelerate/src/quantum_circuit/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,32 +213,33 @@ impl CircuitData {
Ok((ty, args, None::<()>, self_.iter()?).into_py(py))
}

/// Returns the current sequence of registered :class:`.Qubit`
/// instances as a list.
/// Returns the current sequence of registered :class:`.Qubit` instances as a list.
///
/// .. note::
/// .. warning::
///
/// This list is not kept in sync with the container.
/// Do not modify this list yourself. It will invalidate the :class:`CircuitData` data
/// structures.
///
/// Returns:
/// list(:class:`.Qubit`): The current sequence of registered qubits.
#[getter]
pub fn qubits(&self, py: Python<'_>) -> PyObject {
PyList::new(py, self.qubits.as_ref(py)).into_py(py)
pub fn qubits(&self, py: Python<'_>) -> Py<PyList> {
self.qubits.clone_ref(py)
}

/// Returns the current sequence of registered :class:`.Clbit`
/// instances as a list.
///
/// .. note::
/// .. warning::
///
/// This list is not kept in sync with the container.
/// Do not modify this list yourself. It will invalidate the :class:`CircuitData` data
/// structures.
///
/// Returns:
/// list(:class:`.Clbit`): The current sequence of registered clbits.
#[getter]
pub fn clbits(&self, py: Python<'_>) -> PyObject {
PyList::new(py, self.clbits.as_ref(py)).into_py(py)
pub fn clbits(&self, py: Python<'_>) -> Py<PyList> {
self.clbits.clone_ref(py)
}

/// Registers a :class:`.Qubit` instance.
Expand All @@ -251,7 +252,13 @@ impl CircuitData {
/// ValueError: The specified ``bit`` is already present and flag ``strict``
/// was provided.
#[pyo3(signature = (bit, *, strict=true))]
pub fn add_qubit(&mut self, py: Python<'_>, bit: &PyAny, strict: bool) -> PyResult<()> {
pub fn add_qubit(&mut self, py: Python, bit: &PyAny, strict: bool) -> PyResult<()> {
if self.qubits_native.len() != self.qubits.as_ref(bit.py()).len() {
return Err(PyRuntimeError::new_err(concat!(
"This circuit's 'qubits' list has become out of sync with the circuit data.",
" Did something modify it?"
)));
}
let idx: BitType = self.qubits_native.len().try_into().map_err(|_| {
PyRuntimeError::new_err(
"The number of qubits in the circuit has exceeded the maximum capacity",
Expand All @@ -263,7 +270,7 @@ impl CircuitData {
.is_ok()
{
self.qubits_native.push(bit.into_py(py));
self.qubits = PyList::new(py, &self.qubits_native).into_py(py);
self.qubits.as_ref(py).append(bit)?;
} else if strict {
return Err(PyValueError::new_err(format!(
"Existing bit {:?} cannot be re-added in strict mode.",
Expand All @@ -283,7 +290,13 @@ impl CircuitData {
/// ValueError: The specified ``bit`` is already present and flag ``strict``
/// was provided.
#[pyo3(signature = (bit, *, strict=true))]
pub fn add_clbit(&mut self, py: Python<'_>, bit: &PyAny, strict: bool) -> PyResult<()> {
pub fn add_clbit(&mut self, py: Python, bit: &PyAny, strict: bool) -> PyResult<()> {
if self.clbits_native.len() != self.clbits.as_ref(bit.py()).len() {
return Err(PyRuntimeError::new_err(concat!(
"This circuit's 'clbits' list has become out of sync with the circuit data.",
" Did something modify it?"
)));
}
let idx: BitType = self.clbits_native.len().try_into().map_err(|_| {
PyRuntimeError::new_err(
"The number of clbits in the circuit has exceeded the maximum capacity",
Expand All @@ -295,7 +308,7 @@ impl CircuitData {
.is_ok()
{
self.clbits_native.push(bit.into_py(py));
self.clbits = PyList::new(py, &self.clbits_native).into_py(py);
self.clbits.as_ref(py).append(bit)?;
} else if strict {
return Err(PyValueError::new_err(format!(
"Existing bit {:?} cannot be re-added in strict mode.",
Expand Down

0 comments on commit ab2e410

Please sign in to comment.