Skip to content

Commit

Permalink
Add base-gate callback to CUGate.params return
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
jakelishman and rrodenbusch committed Oct 17, 2023
1 parent 6246783 commit 1ccf4ba
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 30 deletions.
4 changes: 2 additions & 2 deletions qiskit/circuit/controlledgate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
72 changes: 44 additions & 28 deletions qiskit/circuit/library/standard_gates/u.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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><c|).1 + |c><c|.base` for control-state `c`, which
# this class does _not_ satisfy (so it shouldn't really be a `ControlledGate`).
super(ControlledGate, type(self)).params.fset(self, parameters)
self.base_gate.params = parameters[:-1]

def __deepcopy__(self, memo=None):
# We have to override this because `ControlledGate` doesn't copy the `_params` list,
# assuming that `params` will be a view onto the base gate's `_params`.
memo = memo if memo is not None else {}
out = super().__deepcopy__(memo)
out._params = copy.deepcopy(out._params, memo)
return out
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
fixes:
- |
:class:`.CUGate` will now behave correctly during calls to :meth:`.QuantumCircuit.assign_parameters`.
Previously, it would cause various odd errors, often some time after the initial circuit assignment.
See `#7326 <https://github.com/Qiskit/qiskit/issues/7326>`__, `#7410 <https://github.com/Qiskit/qiskit/issues/7410>`__,
`#9627 <https://github.com/Qiskit/qiskit/issues/9627>`__, `#10002 <https://github.com/Qiskit/qiskit/issues/10002>`__,
and `#10131 <https://github.com/Qiskit/qiskit/issues/10131>`__.
32 changes: 32 additions & 0 deletions test/python/circuit/test_controlled_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down

0 comments on commit 1ccf4ba

Please sign in to comment.