Fix panic in RemoveIdentityEquivalent with ParameterVector global phase (#16054)

If a circuit has a global phase containing a `ParameterVectorElement`,
the resulting `SymbolExpr` will transitively contain a `Py<PyAny>`
pointing back to the `ParameterVector` object.  When adding to the
global phase, a name/symbol-map clone occurs, which requires us to be
attached to the Python interpreter.  Since `RemoveIdentityEquivalent`
manually detaches from the Python interpreter, we may attempt to
increment the refcount without being attached.

This patch is intended to be minimal for backport to 2.4; there are
several more invasive changes we could make (and largely _should_ make)
to make `ParameterExpression` safer and more performant, and in general
it is becoming clear we still have a lot of work to do to be able to
remove the `py-clone` feature use of PyO3 and let the compiler catch
these mistakes for us.

Author: jakelishman

crates/transpiler/src/passes/remove_identity_equiv.rs

@@ -242,9 +242,23 @@ pub fn py_remove_identity_equiv(
     approx_degree: Option<f64>,
     target: Option<&Target>,
 ) -> PyResult<()> {
+    // TODO: This is a hack to avoid panicking in the case that the global phase contains `Py`
+    // pointers (such as backrefs to `ParameterVector` objects in an expression `Symbol`) that would
+    // get cloned when updating the global phase.  It's easier to do it out here than to try to
+    // reattach to Python-space within a pure-Rust function but only if we can spot a hiding `Py`
+    // (or we'd break standalone C).
+    //
+    // This doesn't account for control-flow blocks which _also_ might have set global phases, byt
+    // `run_remove_identity_equiv` as of Qiskit 2.4 doesn't recurse, so the hack should hold.
+    let old_phase = dag.global_phase().clone();
+    dag.set_global_phase_f64(0.0);
+
     // Explicitly release GIL because threads may call Python to get
     // the matrix for a PyGate
-    py.detach(|| run_remove_identity_equiv(dag, approx_degree, target))
+    py.detach(|| run_remove_identity_equiv(dag, approx_degree, target))?;
+
+    dag.add_global_phase(&old_phase)?;
+    Ok(())
 }
 
 pub fn run_remove_identity_equiv(

releasenotes/notes/fix-panic-remove-identity-equiv-vector-28396cfeee94a268.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixed a panic in :class:`.RemoveIdentityEquivalent` when the global phase of the circuit contained
+    :class:`.ParameterVectorElement` objects.  The message was:
+
+        Cannot clone pointer into Python heap without the thread being attached.
+
+    See `#16053 <https://github.com/Qiskit/qiskit/issues/16053>`__.

test/python/transpiler/test_remove_identity_equivalent.py

@@ -15,7 +15,7 @@
 import ddt
 import numpy as np
 
-from qiskit.circuit import Parameter, QuantumCircuit, QuantumRegister, Gate
+from qiskit.circuit import Parameter, QuantumCircuit, QuantumRegister, Gate, ParameterVector
 from qiskit.circuit.library import (
     CPhaseGate,
     RXGate,
@@ -213,6 +213,16 @@ def test_parameterized_global_phase_ignored(self):
         transpiled = RemoveIdentityEquivalent()(qc)
         self.assertEqual(qc, transpiled)
 
+    def test_no_panic_on_parametervector_phase(self):
+        """Regression test for gh-16053."""
+        pv = ParameterVector("v", 1)
+        qc = QuantumCircuit(1, global_phase=pv[0])
+        qc.rz(0.0, 0)
+        qc = RemoveIdentityEquivalent()(qc)
+
+        expected = QuantumCircuit(1, global_phase=pv[0])
+        self.assertEqual(qc, expected)
+
     def test_pauli_evo_equals_stdgate(self):
         """Test the Pauli evolution gate is consistent with std gates."""