From 1ccf4ba469c9fcc8f14f7d993ba5c2995973c4a6 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Tue, 17 Oct 2023 22:22:57 +0100 Subject: [PATCH] Add base-gate callback to `CUGate.params` return This causes item-setting in `CUGate.params` to forward those sets on to the `UGate` in its `base_gate` unless they are setting the `gamma` parameter, which is not shared. `CUGate` breaks several assumptions and components of the `ControlledGate` interface, including by having more parameters than its `base_gate`; `ControlledGate` assumes that its subclasses will not have a separate `_params` field, but instead will always just view onto their `base_gate._params`. This commit changes the behaviour of the `params` getter to create a temporary list that backreferences to the base gate, satisfying the requirement. Co-authored-by: Richard Rodenbusch --- qiskit/circuit/controlledgate.py | 4 +- .../standard_gates/equivalence_library.py | 1 + qiskit/circuit/library/standard_gates/u.py | 72 +++++++++++-------- ...cu-assign-parameters-fbf6f21d3e0c8c0b.yaml | 8 +++ test/python/circuit/test_controlled_gate.py | 32 +++++++++ 5 files changed, 87 insertions(+), 30 deletions(-) create mode 100644 releasenotes/notes/fix-cu-assign-parameters-fbf6f21d3e0c8c0b.yaml diff --git a/qiskit/circuit/controlledgate.py b/qiskit/circuit/controlledgate.py index c8b5b8a187be..8920e873ba53 100644 --- a/qiskit/circuit/controlledgate.py +++ b/qiskit/circuit/controlledgate.py @@ -243,11 +243,11 @@ def params(self, parameters): else: raise CircuitError("Controlled gate does not define base gate for extracting params") - def __deepcopy__(self, _memo=None): + def __deepcopy__(self, memo=None): cpy = copy.copy(self) cpy.base_gate = self.base_gate.copy() if self._definition: - cpy._definition = copy.deepcopy(self._definition, _memo) + cpy._definition = copy.deepcopy(self._definition, memo) return cpy @property diff --git a/qiskit/circuit/library/standard_gates/equivalence_library.py b/qiskit/circuit/library/standard_gates/equivalence_library.py index 5a519bf520e3..329c72b17cca 100644 --- a/qiskit/circuit/library/standard_gates/equivalence_library.py +++ b/qiskit/circuit/library/standard_gates/equivalence_library.py @@ -1210,6 +1210,7 @@ lam = Parameter("lam") cu3_to_cu = QuantumCircuit(q) cu3_to_cu.cu(theta, phi, lam, 0, 0, 1) +_sel.add_equivalence(CU3Gate(theta, phi, lam), cu3_to_cu) # XGate # diff --git a/qiskit/circuit/library/standard_gates/u.py b/qiskit/circuit/library/standard_gates/u.py index bb3e26495678..39c269def741 100644 --- a/qiskit/circuit/library/standard_gates/u.py +++ b/qiskit/circuit/library/standard_gates/u.py @@ -11,6 +11,7 @@ # that they have been altered from the originals. """Two-pulse single-qubit gate.""" +import copy import math from cmath import exp from typing import Optional, Union @@ -19,7 +20,6 @@ from qiskit.circuit.gate import Gate from qiskit.circuit.parameterexpression import ParameterValueType from qiskit.circuit.quantumregister import QuantumRegister -from qiskit.circuit.exceptions import CircuitError class UGate(Gate): @@ -128,6 +128,35 @@ def __array__(self, dtype=complex): ) +class _CUGateParams(list): + # This awful class is to let `CUGate.params` have its keys settable (as + # `QuantumCircuit.assign_parameters` requires), while accounting for the problem that `CUGate` + # was defined to have a different number of parameters to its `base_gate`, which breaks + # `ControlledGate`'s assumptions, and would make most parametric `CUGate`s invalid. + # + # It's constructed only as part of the `CUGate.params` getter, and given that the general + # circuit model assumes that that's a directly mutable list that _must_ be kept in sync with the + # gate's requirements, we don't need this to support arbitrary mutation, just enough for + # `QuantumCircuit.assign_parameters` to work. + + __slots__ = ("_gate",) + + def __init__(self, gate): + super().__init__(gate._params) + self._gate = gate + + def __setitem__(self, key, value): + if isinstance(key, slice): + raise TypeError("'CUGate' only supports setting parameters by single index") + super().__setitem__(key, value) + self._gate._params[key] = value + # Magic numbers: CUGate has 4 parameters, UGate has 3. + if key < 0: + key = 4 + key + if key < 3: + self._gate.base_gate.params[key] = value + + class CUGate(ControlledGate): r"""Controlled-U gate (4-parameter two-qubit gate). @@ -273,33 +302,20 @@ def __array__(self, dtype=None): @property def params(self): - """Get parameters from base_gate. - - Returns: - list: List of gate parameters. - - Raises: - CircuitError: Controlled gate does not define a base gate - """ - if self.base_gate: - # CU has one additional parameter to the U base gate - return self.base_gate.params + self._params - else: - raise CircuitError("Controlled gate does not define base gate for extracting params") + return _CUGateParams(self) @params.setter def params(self, parameters): - """Set base gate parameters. - - Args: - parameters (list): The list of parameters to set. - - Raises: - CircuitError: If controlled gate does not define a base gate. - """ - # CU has one additional parameter to the U base gate - self._params = [parameters[-1]] - if self.base_gate: - self.base_gate.params = parameters[:-1] - else: - raise CircuitError("Controlled gate does not define base gate for extracting params") + # We need to skip `ControlledGate` in the inheritance tree, since it defines + # that all controlled gates are `(1-|c>`__, `#7410 `__, + `#9627 `__, `#10002 `__, + and `#10131 `__. diff --git a/test/python/circuit/test_controlled_gate.py b/test/python/circuit/test_controlled_gate.py index f0c33df0a600..6ebb9f9d75b2 100644 --- a/test/python/circuit/test_controlled_gate.py +++ b/test/python/circuit/test_controlled_gate.py @@ -1176,6 +1176,38 @@ def test_assign_parameters(self): self.assertEqual(bound1.parameters, {subs1[ptest]}) self.assertEqual(bound2.parameters, {subs2[ptest]}) + def test_assign_cugate(self): + """Test assignment of CUGate, which breaks the `ControlledGate` requirements by not being + equivalent to a direct control of its base gate.""" + + parameters = [Parameter("t"), Parameter("p"), Parameter("l"), Parameter("g")] + values = [0.1, 0.2, 0.3, 0.4] + + qc = QuantumCircuit(2) + qc.cu(*parameters, 0, 1) + assigned = qc.assign_parameters(dict(zip(parameters, values)), inplace=False) + + expected = QuantumCircuit(2) + expected.cu(*values, 0, 1) + + self.assertEqual(assigned.data[0].operation.base_gate, expected.data[0].operation.base_gate) + self.assertEqual(assigned, expected) + + def test_assign_nested_controlled_cu(self): + """Test assignment of an arbitrary controlled parametrised gate that appears through the + `Gate.control()` method on an already-controlled gate.""" + theta = Parameter("t") + qc_c = QuantumCircuit(2) + qc_c.crx(theta, 1, 0) + custom_gate = qc_c.to_gate().control() + qc = QuantumCircuit(3) + qc.append(custom_gate, [0, 1, 2]) + assigned = qc.assign_parameters({theta: 0.5}) + + # We're testing here that everything's been propagated through to the base gates; the `reps` + # is just some high number to make sure we unwrap any controlled and custom gates. + self.assertEqual(set(assigned.decompose(reps=3).parameters), set()) + @data(-1, 0, 1.4, "1", 4, 10) def test_improper_num_ctrl_qubits(self, num_ctrl_qubits): """