From ab2e4108120aaf57ac5d7b6358945a17ec9d324c Mon Sep 17 00:00:00 2001
From: Jake Lishman <jake.lishman@ibm.com>
Date: Wed, 10 Jan 2024 02:28:06 +0000
Subject: [PATCH] Fix quadratic construction time of `QuantumCircuit` (#11517)

* 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
---
 .../src/quantum_circuit/circuit_data.rs       | 41 ++++++++++++-------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/crates/accelerate/src/quantum_circuit/circuit_data.rs b/crates/accelerate/src/quantum_circuit/circuit_data.rs
index 76c3c78d57ed..033bd2ea7005 100644
--- a/crates/accelerate/src/quantum_circuit/circuit_data.rs
+++ b/crates/accelerate/src/quantum_circuit/circuit_data.rs
@@ -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.
@@ -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",
@@ -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.",
@@ -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",
@@ -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.",