Fix extraction of controlled parametric gates (#13067) (#13080)

The `mutable` check in the controlled-gate `OperationFromPython`
extraction logic to check for a mutated `base_gate` was overzealous,
and would return false positives for parametric controlled gates.  The
only modification to `base_gate` of a standard-library gate that would
not have caused data-model problems from Python space would be setting
the base-gate label, which is used for a public feature of the circuit
visualisers.

The change to `get_standard_gate_name_mapping` is just a minor
convenience to make the gate objects directly appendable to a circuit;
previously, each `Parameter` object was distinct and had a UUID clash
with others of the same name, so could not be used together.  The new
behaviour is purely a convenience for tests; it largely should not be
useful for users to directly append these gates.

(cherry picked from commit 8f33084)

Co-authored-by: Jake Lishman <[email protected]>

crates/circuit/src/circuit_instruction.rs

@@ -547,12 +547,18 @@ impl<'py> FromPyObject<'py> for OperationFromPython {
             // mapping to avoid an `isinstance` check on `ControlledGate` - a standard gate has
             // nonzero `num_ctrl_qubits` iff it is a `ControlledGate`.
             //
-            // `ControlledGate` also has a `base_gate` attribute, and we don't track enough in Rust
-            // space to handle the case that that was mutated away from a standard gate.
+            // `ControlledGate` also has a `base_gate` attribute related to its historical
+            // implementation, which technically allows mutations from Python space.  The only
+            // mutation of a standard gate's `base_gate` that wouldn't have already broken the
+            // Python-space data model is setting a label, so we just catch that case and default
+            // back to non-standard-gate handling in that case.
             if standard.num_ctrl_qubits() != 0
                 && ((ob.getattr(intern!(py, "ctrl_state"))?.extract::<usize>()?
                     != (1 << standard.num_ctrl_qubits()) - 1)
-                    || ob.getattr(intern!(py, "mutable"))?.extract()?)
+                    || !ob
+                        .getattr(intern!(py, "base_gate"))?
+                        .getattr(intern!(py, "label"))?
+                        .is_none())
             {
                 break 'standard;
             }

qiskit/circuit/library/standard_gates/__init__.py

@@ -54,45 +54,51 @@ def get_standard_gate_name_mapping():
     from qiskit.circuit.delay import Delay
     from qiskit.circuit.reset import Reset
 
+    lambda_ = Parameter("λ")
+    theta = Parameter("ϴ")
+    phi = Parameter("φ")
+    gamma = Parameter("γ")
+    beta = Parameter("β")
+    time = Parameter("t")
+
     # Standard gates library mapping, multicontrolled gates not included since they're
     # variable width
     gates = [
         IGate(),
         SXGate(),
         XGate(),
         CXGate(),
-        RZGate(Parameter("λ")),
-        RGate(Parameter("ϴ"), Parameter("φ")),
-        Reset(),
+        RZGate(lambda_),
+        RGate(theta, phi),
         C3SXGate(),
         CCXGate(),
         DCXGate(),
         CHGate(),
-        CPhaseGate(Parameter("ϴ")),
-        CRXGate(Parameter("ϴ")),
-        CRYGate(Parameter("ϴ")),
-        CRZGate(Parameter("ϴ")),
+        CPhaseGate(theta),
+        CRXGate(theta),
+        CRYGate(theta),
+        CRZGate(theta),
         CSwapGate(),
         CSXGate(),
-        CUGate(Parameter("ϴ"), Parameter("φ"), Parameter("λ"), Parameter("γ")),
-        CU1Gate(Parameter("λ")),
-        CU3Gate(Parameter("ϴ"), Parameter("φ"), Parameter("λ")),
+        CUGate(theta, phi, lambda_, gamma),
+        CU1Gate(lambda_),
+        CU3Gate(theta, phi, lambda_),
         CYGate(),
         CZGate(),
         CCZGate(),
-        GlobalPhaseGate(Parameter("ϴ")),
+        GlobalPhaseGate(theta),
         HGate(),
-        PhaseGate(Parameter("ϴ")),
+        PhaseGate(theta),
         RCCXGate(),
         RC3XGate(),
-        RXGate(Parameter("ϴ")),
-        RXXGate(Parameter("ϴ")),
-        RYGate(Parameter("ϴ")),
-        RYYGate(Parameter("ϴ")),
-        RZZGate(Parameter("ϴ")),
-        RZXGate(Parameter("ϴ")),
-        XXMinusYYGate(Parameter("ϴ"), Parameter("β")),
-        XXPlusYYGate(Parameter("ϴ"), Parameter("β")),
+        RXGate(theta),
+        RXXGate(theta),
+        RYGate(theta),
+        RYYGate(theta),
+        RZZGate(theta),
+        RZXGate(theta),
+        XXMinusYYGate(theta, beta),
+        XXPlusYYGate(theta, beta),
         ECRGate(),
         SGate(),
         SdgGate(),
@@ -103,13 +109,14 @@ def get_standard_gate_name_mapping():
         SXdgGate(),
         TGate(),
         TdgGate(),
-        UGate(Parameter("ϴ"), Parameter("φ"), Parameter("λ")),
-        U1Gate(Parameter("λ")),
-        U2Gate(Parameter("φ"), Parameter("λ")),
-        U3Gate(Parameter("ϴ"), Parameter("φ"), Parameter("λ")),
+        UGate(theta, phi, lambda_),
+        U1Gate(lambda_),
+        U2Gate(phi, lambda_),
+        U3Gate(theta, phi, lambda_),
         YGate(),
         ZGate(),
-        Delay(Parameter("t")),
+        Delay(time),
+        Reset(),
         Measure(),
     ]
     name_mapping = {gate.name: gate for gate in gates}

releasenotes/notes/extract-standard-parametric-controlled-1a495f6f7ce89397.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Parametric controlled standard-library gates (such as :class:`.CRXGate`) will now get correctly
+    extracted to a Rust-space standard gate when using :meth:`.QuantumCircuit.append` and the gate
+    object.  Previously there was a discrepancy where using the :meth:`.QuantumCircuit.crx` method
+    would cause a correct extraction in Rust space, but the :meth:`~.QuantumCirucit.append` form
+    would not.  The bug should generally not have caused any unsoundness from Python.

test/python/circuit/test_rust_equivalence.py

@@ -227,3 +227,20 @@ def test_non_default_controls(self):
                 circuit.append(gate, [0, 1, 2])
                 self.assertIsNotNone(getattr(gate, "_standard_gate", None))
                 np.testing.assert_almost_equal(Operator(circuit.data[0].operation).to_matrix(), op)
+
+    def test_extracted_as_standard_gate(self):
+        """Test that every gate in the standard library gets correctly extracted as a Rust-space
+        `StandardGate` in its default configuration when passed through `append`."""
+        standards = set()
+        qc = QuantumCircuit(4)
+        for name, gate in get_standard_gate_name_mapping().items():
+            if gate._standard_gate is None:
+                # Not a standard gate.
+                continue
+            standards.add(name)
+            qc.append(gate, qc.qubits[: gate.num_qubits], [])
+        # Sanity check: the test should have found at least one standard gate in the mapping.
+        self.assertNotEqual(standards, set())
+
+        extracted = {inst.name for inst in qc.data if inst.is_standard_gate()}
+        self.assertEqual(standards, extracted)