Fix phase for controlled Rz

qiskit/circuit/add_control.py

@@ -44,6 +44,8 @@ def _control_definition_known(operation, num_ctrl_qubits):
         return True
     elif num_ctrl_qubits == 1:
         return operation.name in {'x', 'y', 'z', 'h', 'rz', 'swap', 'u1', 'u3', 'cx'}
+    elif operation.name == 'rz' and num_ctrl_qubits > 1:
+        return True
     else:
         return False
 
@@ -70,6 +72,11 @@ def _control_predefined(operation, num_ctrl_qubits):
     elif operation.name == 'rz':
         import qiskit.extensions.standard.crz
         cgate = qiskit.extensions.standard.crz.CrzGate(*operation.params)
+        if num_ctrl_qubits == 1:
+            return cgate
+        else:
+            # use crz as base gate for phase correctness
+            return cgate.control(num_ctrl_qubits - 1)
     elif operation.name == 'swap':
         import qiskit.extensions.standard.cswap
         cgate = qiskit.extensions.standard.cswap.FredkinGate()

qiskit/extensions/unitary.py

@@ -111,6 +111,44 @@ def _define(self):
             raise NotImplementedError("Not able to generate a subcircuit for "
                                       "a {}-qubit unitary".format(self.num_qubits))
 
+    def control(self, num_ctrl_qubits=1, label=None):
+        """Return controlled version of gate
+
+        Args:
+            num_ctrl_qubits (int): number of controls to add to gate (default=1)
+            label (str): optional gate label
+
+        Returns:
+            UnitaryGate: controlled version of gate.
+
+        Raises:
+            QiskitError: unrecognized mode
+        """
+        cmat = self._compute_control_matrix(self.to_matrix(), num_ctrl_qubits)
+        return UnitaryGate(cmat, label=label)
+
+    def _compute_control_matrix(self, base_mat, num_ctrl_qubits):
+        """
+        Compute the controlled version of the input matrix with qiskit ordering.
+
+        Args:
+            base_mat (ndarray): unitary to be controlled
+            num_ctrl_qubits (int): number of controls for new unitary
+
+        Returns:
+            ndarray: controlled version of base matrix.
+        """
+        num_target = int(numpy.log2(base_mat.shape[0]))
+        ctrl_dim = 2**num_ctrl_qubits
+        ctrl_grnd = numpy.repeat([[1], [0]], [1, ctrl_dim-1])
+        full_mat_dim = ctrl_dim * base_mat.shape[0]
+        full_mat = numpy.zeros((full_mat_dim, full_mat_dim), dtype=base_mat.dtype)
+        ctrl_proj = numpy.diag(numpy.roll(ctrl_grnd, ctrl_dim - 1))
+        full_mat = (numpy.kron(numpy.eye(2**num_target),
+                               numpy.eye(ctrl_dim) - ctrl_proj)
+                    + numpy.kron(base_mat, ctrl_proj))
+        return full_mat
+
     def qasm(self):
         """ The qasm for a custom unitary gate
         This is achieved by adding a custom gate that corresponds to the definition

test/python/circuit/test_controlled_gate.py

@@ -19,9 +19,10 @@
 from inspect import signature
 import numpy as np
 from numpy import pi
+import scipy
 from ddt import ddt, data
 
-from qiskit import QuantumRegister, QuantumCircuit, execute, BasicAer
+from qiskit import QuantumRegister, QuantumCircuit, execute, BasicAer, QiskitError
 from qiskit.test import QiskitTestCase
 from qiskit.circuit import ControlledGate
 from qiskit.quantum_info.operators.predicates import matrix_equal, is_unitary_matrix
@@ -33,7 +34,8 @@
 from qiskit.extensions.standard import (CnotGate, XGate, YGate, ZGate, U1Gate,
                                         CyGate, CzGate, Cu1Gate, SwapGate,
                                         ToffoliGate, HGate, RZGate, FredkinGate,
-                                        U3Gate, CHGate, CrzGate, Cu3Gate)
+                                        U3Gate, CHGate, CrzGate, Cu3Gate,
+                                        MSGate, Barrier)
 from qiskit.extensions.unitary import UnitaryGate
 import qiskit.extensions.standard as allGates
 
@@ -287,33 +289,37 @@ def test_rotation_gates(self):
         num_target = 1
         qreg = QuantumRegister(num_ctrl + num_target)
 
-        gu1 = u1.U1Gate(pi)
-        grx = rx.RXGate(pi)
-        gry = ry.RYGate(pi)
-        grz = rz.RZGate(pi)
+        theta = pi
+        gu1 = u1.U1Gate(theta)
+        grx = rx.RXGate(theta)
+        gry = ry.RYGate(theta)
+        grz = rz.RZGate(theta)
 
         ugu1 = ac._unroll_gate(gu1, ['u1', 'u3', 'cx'])
         ugrx = ac._unroll_gate(grx, ['u1', 'u3', 'cx'])
         ugry = ac._unroll_gate(gry, ['u1', 'u3', 'cx'])
         ugrz = ac._unroll_gate(grz, ['u1', 'u3', 'cx'])
+        ugrz.params = grz.params
 
         cgu1 = ugu1.control(num_ctrl)
         cgrx = ugrx.control(num_ctrl)
         cgry = ugry.control(num_ctrl)
         cgrz = ugrz.control(num_ctrl)
 
-        simulator = BasicAer.get_backend('unitary_simulator')
         for gate, cgate in zip([gu1, grx, gry, grz], [cgu1, cgrx, cgry, cgrz]):
             with self.subTest(i=gate.name):
-                qc = QuantumCircuit(num_target)
-                qc.append(gate, qc.qregs[0])
-                op_mat = execute(qc, simulator).result().get_unitary(0)
-                cqc = QuantumCircuit(num_ctrl + num_target)
-                cqc.append(cgate, cqc.qregs[0])
-                ref_mat = execute(cqc, simulator).result().get_unitary(0)
+                if gate.name == 'rz':
+                    iden = Operator.from_label('I')
+                    zgen = Operator.from_label('Z')
+                    op_mat = (np.cos(0.5 * theta) * iden - 1j * np.sin(0.5 * theta) * zgen).data
+                else:
+                    op_mat = Operator(gate).data
+                ref_mat = Operator(cgate).data
                 cop_mat = _compute_control_matrix(op_mat, num_ctrl)
                 self.assertTrue(matrix_equal(cop_mat, ref_mat,
                                              ignore_phase=True))
+                cqc = QuantumCircuit(num_ctrl + num_target)
+                cqc.append(cgate, cqc.qregs[0])
                 dag = circuit_to_dag(cqc)
                 unroller = Unroller(['u3', 'cx'])
                 uqc = dag_to_circuit(unroller.run(dag))
@@ -322,6 +328,8 @@ def test_rotation_gates(self):
                 # these limits could be changed
                 if gate.name == 'ry':
                     self.assertTrue(uqc.size() <= 32)
+                elif gate.name == 'rz':
+                    self.assertTrue(uqc.size() <= 40)
                 else:
                     self.assertTrue(uqc.size() <= 20)
         qc = QuantumCircuit(qreg, name='composite')
@@ -333,8 +341,9 @@ def test_rotation_gates(self):
         dag = circuit_to_dag(qc)
         unroller = Unroller(['u3', 'cx'])
         uqc = dag_to_circuit(unroller.run(dag))
+        print(uqc.size())
         self.log.info('%s gate count: %d', uqc.name, uqc.size())
-        self.assertTrue(uqc.size() <= 73)  # this limit could be changed
+        self.assertTrue(uqc.size() <= 93)  # this limit could be changed
 
     @data(1, 2, 3, 4)
     def test_inverse_x(self, num_ctrl_qubits):
@@ -379,23 +388,16 @@ def test_controlled_unitary(self, num_ctrl_qubits):
         self.assertTrue(is_unitary_matrix(base_mat))
         self.assertTrue(matrix_equal(cop_mat, test_op.data, ignore_phase=True))
 
-    @data(1, 2, 3)
-    def test_global_phase(self, num_ctrl_qubits):
-        """test global phase"""
-        mat1 = np.array([[0, 1], [1, 0]], dtype=float)
-        mat2 = np.exp(1j) * mat1
-        gate1 = UnitaryGate(mat1)
-        gate2 = UnitaryGate(mat2)
-        cgate1 = gate1.control(num_ctrl_qubits)
-        cgate2 = gate2.control(num_ctrl_qubits)
-        cop_mat1 = _compute_control_matrix(mat1, num_ctrl_qubits)
-        cop_mat2 = _compute_control_matrix(mat2, num_ctrl_qubits, phase=1)
-        self.assertTrue(is_unitary_matrix(mat1))
-        self.assertTrue(is_unitary_matrix(mat2))
-        self.assertTrue(is_unitary_matrix(cop_mat1))
-        self.assertTrue(is_unitary_matrix(cop_mat2))
-        self.assertTrue(Operator(cgate1).equiv(Operator(cgate2)))
-        self.assertTrue(matrix_equal(cop_mat1, cop_mat2, ignore_phase=True))
+    @data(1, 2, 3, 4, 5)
+    def test_controlled_random_unitary(self, num_ctrl_qubits):
+        """test controlled unitary"""
+        num_target = 2
+        base_gate = UnitaryGate(scipy.stats.unitary_group.rvs(num_target))
+        base_mat = base_gate.to_matrix()
+        cgate = base_gate.control(num_ctrl_qubits)
+        test_op = Operator(cgate)
+        cop_mat = _compute_control_matrix(base_mat, num_ctrl_qubits)
+        self.assertTrue(matrix_equal(cop_mat, test_op.data, ignore_phase=True))
 
     def test_base_gate_setting(self):
         """
@@ -437,16 +439,48 @@ def test_all_inverses(self):
                 # skip gates that do not have a control attribute (e.g. barrier)
                 pass
 
+    @data(1, 2, 3)
+    def test_controlled_standard_gates(self, num_ctrl_qubits):
+        """
+        Test controlled versions of all standard gates.
+        """
+        gate_classes = [cls for name, cls in allGates.__dict__.items()
+                        if isinstance(cls, type)]
+        theta = pi/2
+        for cls in gate_classes:
+            with self.subTest(i=cls):
+                sig = signature(cls)
+                numargs = len([param for param in sig.parameters.values()
+                               if param.kind == param.POSITIONAL_ONLY
+                               or (param.kind == param.POSITIONAL_OR_KEYWORD
+                                   and param.default is param.empty)])
+                args = [theta] * numargs
+                if cls in [MSGate, Barrier]:
+                    args[0] = 2
+                gate = cls(*args)
+                try:
+                    cgate = gate.control(num_ctrl_qubits)
+                except (AttributeError, QiskitError):
+                    # 'object has no attribute "control"'
+                    # skipping Id and Barrier
+                    continue
+                if gate.name == 'rz':
+                    iden = Operator.from_label('I')
+                    zgen = Operator.from_label('Z')
+                    base_mat = (np.cos(0.5 * theta) * iden - 1j * np.sin(0.5 * theta) * zgen).data
+                else:
+                    base_mat = Operator(gate).data
+                target_mat = _compute_control_matrix(base_mat, num_ctrl_qubits)
+                self.assertTrue(matrix_equal(Operator(cgate).data, target_mat, ignore_phase=True))
+
 
-def _compute_control_matrix(base_mat, num_ctrl_qubits, phase=0):
+def _compute_control_matrix(base_mat, num_ctrl_qubits):
     """
     Compute the controlled version of the input matrix with qiskit ordering.
 
     Args:
         base_mat (ndarray): unitary to be controlled
         num_ctrl_qubits (int): number of controls for new unitary
-        phase (float): The global phase of base_mat which is promoted to the
-            global phase of the controlled matrix
 
     Returns:
         ndarray: controlled version of base matrix.
@@ -456,12 +490,10 @@ def _compute_control_matrix(base_mat, num_ctrl_qubits, phase=0):
     ctrl_grnd = np.repeat([[1], [0]], [1, ctrl_dim-1])
     full_mat_dim = ctrl_dim * base_mat.shape[0]
     full_mat = np.zeros((full_mat_dim, full_mat_dim), dtype=base_mat.dtype)
-    for i in range(ctrl_dim-1):
-        full_mat += np.kron(np.eye(2**num_target),
-                            np.diag(np.roll(ctrl_grnd, i)))
-    if phase != 0:
-        full_mat = np.exp(1j * phase) * full_mat
-    full_mat += np.kron(base_mat, np.diag(np.roll(ctrl_grnd, ctrl_dim-1)))
+    ctrl_proj = np.diag(np.roll(ctrl_grnd, ctrl_dim - 1))
+    full_mat = (np.kron(np.eye(2**num_target),
+                        np.eye(ctrl_dim) - ctrl_proj)
+                + np.kron(base_mat, ctrl_proj))
     return full_mat