From 84ecc802ded6870d7e3b9f4a2c14337a4895a762 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 3 Sep 2024 16:21:18 +0000 Subject: [PATCH] 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 8f3308475ce4d0fbce1a95e7397360a9b352b496) Co-authored-by: Jake Lishman --- crates/circuit/src/circuit_instruction.rs | 12 +++- .../library/standard_gates/__init__.py | 57 +++++++++++-------- ...arametric-controlled-1a495f6f7ce89397.yaml | 8 +++ test/python/circuit/test_rust_equivalence.py | 17 ++++++ 4 files changed, 66 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/extract-standard-parametric-controlled-1a495f6f7ce89397.yaml diff --git a/crates/circuit/src/circuit_instruction.rs b/crates/circuit/src/circuit_instruction.rs index d44051745a89..9aa5a1935fbb 100644 --- a/crates/circuit/src/circuit_instruction.rs +++ b/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::()? != (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; } diff --git a/qiskit/circuit/library/standard_gates/__init__.py b/qiskit/circuit/library/standard_gates/__init__.py index a4741386343f..19a2bf770da2 100644 --- a/qiskit/circuit/library/standard_gates/__init__.py +++ b/qiskit/circuit/library/standard_gates/__init__.py @@ -54,6 +54,13 @@ 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 = [ @@ -61,38 +68,37 @@ def get_standard_gate_name_mapping(): 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} diff --git a/releasenotes/notes/extract-standard-parametric-controlled-1a495f6f7ce89397.yaml b/releasenotes/notes/extract-standard-parametric-controlled-1a495f6f7ce89397.yaml new file mode 100644 index 000000000000..ea4e69dd245f --- /dev/null +++ b/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. diff --git a/test/python/circuit/test_rust_equivalence.py b/test/python/circuit/test_rust_equivalence.py index b6de438d04dd..b6ec28f82e8d 100644 --- a/test/python/circuit/test_rust_equivalence.py +++ b/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)