add_control: check for predefined gates by type instead of name

qiskit/circuit/add_control.py

@@ -18,6 +18,7 @@
 
 from qiskit.circuit.exceptions import CircuitError
 from qiskit.extensions import UnitaryGate
+from qiskit.extensions.standard import XGate, RXGate, RYGate, RZGate, U1Gate, U3Gate
 from . import ControlledGate, Gate, QuantumRegister, QuantumCircuit
 
 
@@ -96,7 +97,6 @@ def control(operation: Union[Gate, ControlledGate],
     Raises:
         CircuitError: gate contains non-gate in definition
     """
-    from math import pi
     # pylint: disable=cyclic-import
     import qiskit.circuit.controlledgate as controlledgate
     # pylint: disable=unused-import
@@ -113,41 +113,38 @@ def control(operation: Union[Gate, ControlledGate],
     q_ancillae = None  # TODO: add
     qc = QuantumCircuit(q_control, q_target)
 
-    if operation.name == 'x' or (
-            isinstance(operation, controlledgate.ControlledGate) and
-            operation.base_gate.name == 'x'):
-        qc.mct(q_control[:] + q_target[:-1], q_target[-1], q_ancillae)
-    elif operation.name == 'rx':
-        qc.mcrx(operation.definition[0][0].params[0], q_control, q_target[0],
+    qubit_kwargs = {
+        'q_controls': q_control[:] + q_target[:-1],
+        'q_target': q_target[-1],
+    }
+
+    if _operation_has_base_gate(operation, XGate):
+        qc.mct(**qubit_kwargs,
+               q_ancilla=q_ancillae,
+               mode='noancilla')
+    elif _operation_has_base_gate(operation, RXGate):
+        qc.mcrx(_get_base_gate_params(operation)[0], **qubit_kwargs,
                 use_basis_gates=True)
-    elif operation.name == 'ry':
-        qc.mcry(operation.definition[0][0].params[0], q_control, q_target[0],
-                q_ancillae, mode='noancilla', use_basis_gates=True)
-    elif operation.name == 'rz':
-        qc.mcrz(operation.definition[0][0].params[0], q_control, q_target[0],
+    elif _operation_has_base_gate(operation, RYGate):
+        qc.mcry(_get_base_gate_params(operation)[0], **qubit_kwargs,
+                q_ancillae=q_ancillae, use_basis_gates=True)
+    elif isinstance(operation, RZGate):
+        qc.mcrz(operation.definition[0][0].params[0], **qubit_kwargs,
                 use_basis_gates=True)
+    elif _operation_has_base_gate(operation, U1Gate):
+        qc.mcu1(_get_base_gate_params(operation)[2],
+                qubit_kwargs['q_controls'], qubit_kwargs['q_target'])
+    elif _operation_has_base_gate(operation, U3Gate):
+        theta, phi, lamb = _get_base_gate_params(operation)
+        _apply_mcu3(qc, theta, phi, lamb, **qubit_kwargs, q_ancillae=q_ancillae)
     else:
         bgate = _unroll_gate(operation, ['u1', 'u3', 'cx'])
         # now we have a bunch of single qubit rotation gates and cx
         for rule in bgate.definition:
             if rule[0].name == 'u3':
                 theta, phi, lamb = rule[0].params
-                if phi == -pi / 2 and lamb == pi / 2:
-                    qc.mcrx(theta, q_control, q_target[rule[1][0].index],
-                            use_basis_gates=True)
-                elif phi == 0 and lamb == 0:
-                    qc.mcry(theta, q_control, q_target[rule[1][0].index],
-                            q_ancillae, use_basis_gates=True)
-                elif theta == 0 and phi == 0:
-                    qc.mcrz(lamb, q_control, q_target[rule[1][0].index],
-                            use_basis_gates=True)
-                else:
-                    qc.mcrz(lamb, q_control, q_target[rule[1][0].index],
-                            use_basis_gates=True)
-                    qc.mcry(theta, q_control, q_target[rule[1][0].index],
-                            q_ancillae, use_basis_gates=True)
-                    qc.mcrz(phi, q_control, q_target[rule[1][0].index],
-                            use_basis_gates=True)
+                _apply_mcu3(qc, theta, phi, lamb, q_control, q_target[rule[1][0].index],
+                            q_ancillae)
             elif rule[0].name == 'u1':
                 qc.mcu1(rule[0].params[0], q_control, q_target[rule[1][0].index])
             elif rule[0].name == 'cx':
@@ -201,6 +198,25 @@ def _gate_to_circuit(operation):
     return qc
 
 
+def _operation_has_base_gate(operation, gate):
+    return isinstance(operation, gate) or (
+        isinstance(operation, ControlledGate) and
+        isinstance(operation.base_gate, gate))
+
+
+def _get_base_gate_params(operation):
+    if isinstance(operation, ControlledGate):
+        if isinstance(operation.base_gate, U3Gate):
+            return operation.base_gate.params
+        else:
+            return operation.base_gate.definition[0][0].params
+    else:
+        if isinstance(operation, U3Gate):
+            return operation.params
+        else:
+            return operation.definition[0][0].params
+
+
 def _unroll_gate(operation, basis_gates):
     from qiskit.converters.circuit_to_dag import circuit_to_dag
     from qiskit.converters.dag_to_circuit import dag_to_circuit
@@ -209,3 +225,20 @@ def _unroll_gate(operation, basis_gates):
     dag = circuit_to_dag(_gate_to_circuit(operation))
     qc = dag_to_circuit(unroller.run(dag))
     return qc.to_gate()
+
+
+def _apply_mcu3(circuit, theta, phi, lamb, q_controls, q_target, q_ancillae):
+    from math import pi
+
+    if phi == -pi / 2 and lamb == pi / 2:
+        circuit.mcrx(theta, q_controls, q_target, use_basis_gates=True)
+    elif phi == 0 and lamb == 0:
+        circuit.mcry(theta, q_controls, q_target,
+                     q_ancillae, mode='noancilla', use_basis_gates=True)
+    elif theta == 0 and phi == 0:
+        circuit.mcrz(lamb, q_controls, q_target, use_basis_gates=True)
+    else:
+        circuit.mcrz(lamb, q_controls, q_target, use_basis_gates=True)
+        circuit.mcry(theta, q_controls, q_target,
+                     q_ancillae, mode='noancilla', use_basis_gates=True)
+        circuit.mcrz(phi, q_controls, q_target, use_basis_gates=True)

qiskit/circuit/controlledgate.py

@@ -120,6 +120,8 @@ def definition(self) -> List:
         elif isinstance(self, CXGate):
             qreg = QuantumRegister(self.num_qubits, 'q')
             definition = [(self, [qreg[0], qreg[1]], [])]
+        else:
+            return self._definition
         open_rules = []
         for qind, val in enumerate(bit_ctrl_state[::-1]):
             if val == '0':

releasenotes/notes/control-by-type-68f1e151d93daeeb.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes cases in add_control(). Non-standard gates that were unfortunately named
+    identically to some standard ones (in particular, X, RX, RY, RZ) would be
+    identified as those gates, and the control would be applied erroneously.

test/python/circuit/test_controlled_gate.py

@@ -172,13 +172,19 @@ def test_single_controlled_composite_gate(self):
         ref_mat = execute(qc, simulator).result().get_unitary(0)
         self.assertTrue(matrix_equal(cop_mat, ref_mat, ignore_phase=True))
 
-    def test_multi_control_u3(self):
+    @data(
+        [0.2, -pi/2, pi/2],  # handle as rx
+        [0.2, 0, 0],  # handle as ry
+        [0, 0, 0.4],  # handle as rz
+        [0.2, 0.3, 0.4],
+    )
+    def test_multi_control_u3(self, params):
         """Test the matrix representation of the controlled and controlled-controlled U3 gate."""
         import qiskit.extensions.standard.u3 as u3
 
         num_ctrl = 3
         # U3 gate params
-        alpha, beta, gamma = 0.2, 0.3, 0.4
+        [alpha, beta, gamma] = params
 
         # cnu3 gate
         u3gate = u3.U3Gate(alpha, beta, gamma)
@@ -598,6 +604,13 @@ def test_inverse_circuit(self, num_ctrl_qubits):
         result = Operator(cgate).compose(Operator(inv_cgate))
         np.testing.assert_array_almost_equal(result.data, np.identity(result.dim[0]))
 
+    def test_named_circuit(self):
+        """Tests control is applied to operation type, not name"""
+        qc = QuantumCircuit(1, name='x')
+        gate = qc.to_gate()
+
+        self.assertIsNone(gate.control().definition)  # Not a CnotGate
+
     @data(1, 2, 3, 4, 5)
     def test_controlled_unitary(self, num_ctrl_qubits):
         """Test the matrix data of an Operator, which is based on a controlled gate."""