Fix global phase tracking for Unroller, OneQubitEulerDecomposer, and …

…ControlledGate (#5248)

* fix bugs on global phases

* lint

* lint

Co-authored-by: Kevin Krsulich <[email protected]>
(cherry picked from commit 2d11db2)

qiskit/circuit/add_control.py

@@ -113,6 +113,8 @@ def control(operation: Union[Gate, ControlledGate],
     else:
         basis = ['p', 'u', 'x', 'rx', 'ry', 'rz', 'cx']
         unrolled_gate = _unroll_gate(operation, basis_gates=basis)
+        if unrolled_gate.definition.global_phase:
+            global_phase += unrolled_gate.definition.global_phase
         for gate, qreg, _ in unrolled_gate.definition.data:
             if gate.name == 'x':
                 controlled_circ.mct(q_control, q_target[qreg[0].index],
@@ -162,11 +164,11 @@ def control(operation: Union[Gate, ControlledGate],
             if gate.definition is not None and gate.definition.global_phase:
                 global_phase += gate.definition.global_phase
     # apply controlled global phase
-    if ((operation.definition is not None and operation.definition.global_phase) or global_phase):
+    if global_phase:
         if len(q_control) < 2:
-            controlled_circ.p(operation.definition.global_phase + global_phase, q_control)
+            controlled_circ.p(global_phase, q_control)
         else:
-            controlled_circ.mcp(operation.definition.global_phase + global_phase,
+            controlled_circ.mcp(global_phase,
                                 q_control[:-1], q_control[-1])
     if isinstance(operation, controlledgate.ControlledGate):
         new_num_ctrl_qubits = num_ctrl_qubits + operation.num_ctrl_qubits

qiskit/quantum_info/synthesis/one_qubit_decompose.py

@@ -125,8 +125,8 @@ def __call__(self,
         if not is_unitary_matrix(unitary):
             raise QiskitError("OneQubitEulerDecomposer: "
                               "input matrix is not unitary.")
-        theta, phi, lam, _ = self._params(unitary)
-        circuit = self._circuit(theta, phi, lam,
+        theta, phi, lam, phase = self._params(unitary)
+        circuit = self._circuit(theta, phi, lam, phase,
                                 simplify=simplify,
                                 atol=atol)
         return circuit
@@ -244,9 +244,10 @@ def _params_u1x(mat):
     def _circuit_zyz(theta,
                      phi,
                      lam,
+                     phase,
                      simplify=True,
                      atol=DEFAULT_ATOL):
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         if simplify and np.isclose(theta, 0.0, atol=atol):
             circuit.append(RZGate(phi + lam), [0])
             return circuit
@@ -262,13 +263,14 @@ def _circuit_zyz(theta,
     def _circuit_zxz(theta,
                      phi,
                      lam,
+                     phase,
                      simplify=False,
                      atol=DEFAULT_ATOL):
         if simplify and np.isclose(theta, 0.0, atol=atol):
-            circuit = QuantumCircuit(1)
+            circuit = QuantumCircuit(1, global_phase=phase)
             circuit.append(RZGate(phi + lam), [0])
             return circuit
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         if not simplify or not np.isclose(lam, 0.0, atol=atol):
             circuit.append(RZGate(lam), [0])
         if not simplify or not np.isclose(theta, 0.0, atol=atol):
@@ -281,9 +283,10 @@ def _circuit_zxz(theta,
     def _circuit_xyx(theta,
                      phi,
                      lam,
+                     phase,
                      simplify=True,
                      atol=DEFAULT_ATOL):
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         if simplify and np.isclose(theta, 0.0, atol=atol):
             circuit.append(RXGate(phi + lam), [0])
             return circuit
@@ -299,35 +302,38 @@ def _circuit_xyx(theta,
     def _circuit_u3(theta,
                     phi,
                     lam,
+                    phase,
                     simplify=True,
                     atol=DEFAULT_ATOL):
         # pylint: disable=unused-argument
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         circuit.append(U3Gate(theta, phi, lam), [0])
         return circuit
 
     @staticmethod
     def _circuit_u(theta,
                    phi,
                    lam,
+                   phase,
                    simplify=True,
                    atol=DEFAULT_ATOL):
         # pylint: disable=unused-argument
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         circuit.u(theta, phi, lam, 0)
         return circuit
 
     @staticmethod
     def _circuit_psx(theta,
                      phi,
                      lam,
+                     phase,
                      simplify=True,
                      atol=DEFAULT_ATOL):
         # Shift theta and phi so decomposition is
         # Phase(phi+pi).SX.Phase(theta+pi).SX.Phase(lam)
         theta = _mod2pi(theta + np.pi)
         phi = _mod2pi(phi + np.pi)
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         # Check for decomposition into minimimal number required SX gates
         if simplify and np.isclose(abs(theta), np.pi, atol=atol):
             if not np.isclose(_mod2pi(abs(lam + phi + theta)),
@@ -357,6 +363,7 @@ def _circuit_psx(theta,
     def _circuit_u1x(theta,
                      phi,
                      lam,
+                     phase,
                      simplify=True,
                      atol=DEFAULT_ATOL):
         # Shift theta and phi so decomposition is
@@ -366,18 +373,18 @@ def _circuit_u1x(theta,
         # Check for decomposition into minimimal number required X90 pulses
         if simplify and np.isclose(abs(theta), np.pi, atol=atol):
             # Zero X90 gate decomposition
-            circuit = QuantumCircuit(1)
+            circuit = QuantumCircuit(1, global_phase=phase)
             circuit.append(U1Gate(lam + phi + theta), [0])
             return circuit
         if simplify and np.isclose(abs(theta), np.pi/2, atol=atol):
             # Single X90 gate decomposition
-            circuit = QuantumCircuit(1)
+            circuit = QuantumCircuit(1, global_phase=phase)
             circuit.append(U1Gate(lam + theta), [0])
             circuit.append(RXGate(np.pi / 2), [0])
             circuit.append(U1Gate(phi + theta), [0])
             return circuit
         # General two-X90 gate decomposition
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         circuit.append(U1Gate(lam), [0])
         circuit.append(RXGate(np.pi / 2), [0])
         circuit.append(U1Gate(theta), [0])
@@ -389,9 +396,10 @@ def _circuit_u1x(theta,
     def _circuit_rr(theta,
                     phi,
                     lam,
+                    phase,
                     simplify=True,
                     atol=DEFAULT_ATOL):
-        circuit = QuantumCircuit(1)
+        circuit = QuantumCircuit(1, global_phase=phase)
         if not simplify or not np.isclose(theta, -np.pi, atol=atol):
             circuit.append(RGate(theta + np.pi, np.pi / 2 - lam), [0])
         circuit.append(RGate(-np.pi, 0.5 * (phi - lam + np.pi)), [0])

qiskit/transpiler/passes/basis/decompose.py

@@ -46,12 +46,12 @@ def run(self, dag: DAGCircuit) -> DAGCircuit:
             # opaque or built-in gates are not decomposable
             if not node.op.definition:
                 continue
-            if node.op.definition.global_phase:
-                dag.global_phase += node.op.definition.global_phase
             # TODO: allow choosing among multiple decomposition rules
             rule = node.op.definition.data
 
             if len(rule) == 1 and len(node.qargs) == len(rule[0][1]) == 1:
+                if node.op.definition.global_phase:
+                    dag.global_phase += node.op.definition.global_phase
                 dag.substitute_node(node, rule[0][0], inplace=True)
             else:
                 decomposition = circuit_to_dag(node.op.definition)

qiskit/transpiler/passes/basis/unroller.py

@@ -95,8 +95,6 @@ def run(self, dag):
                                       (str(self.basis), node.op.name))
                 decomposition = circuit_to_dag(node.op.definition)
                 unrolled_dag = self.run(decomposition)  # recursively unroll ops
-                if node.op.definition and node.op.definition.global_phase:
-                    dag.global_phase += node.op.definition.global_phase
                 if unrolled_dag.global_phase:
                     dag.global_phase += unrolled_dag.global_phase
                     unrolled_dag.global_phase = 0

test/python/circuit/test_controlled_gate.py

@@ -1046,6 +1046,24 @@ def test_rz_composite_global_phase(self, num_ctrl_qubits):
         self.assertEqual(Operator(cgate), Operator(target))
         self.assertEqual(Operator(ccirc), Operator(target))
 
+    @data(1, 2)
+    def test_nested_global_phase(self, num_ctrl_qubits):
+        """
+        Test controlling a gate with nested global phase.
+        """
+        theta = pi/4
+        circ = QuantumCircuit(1, global_phase=theta)
+        circ.z(0)
+        v = circ.to_gate()
+
+        qc = QuantumCircuit(1)
+        qc.append(v, [0])
+        ctrl_qc = qc.control(num_ctrl_qubits)
+
+        base_mat = Operator(qc).data
+        target = _compute_control_matrix(base_mat, num_ctrl_qubits)
+        self.assertEqual(Operator(ctrl_qc), Operator(target))
+
 
 @ddt
 class TestOpenControlledToMatrix(QiskitTestCase):

test/python/circuit/test_unitary.py

@@ -291,3 +291,8 @@ def test_unitary_decomposition_via_definition(self):
         """Test decomposition for 1Q unitary via definition."""
         mat = numpy.array([[0, 1], [1, 0]])
         numpy.allclose(Operator(UnitaryGate(mat).definition).data, mat)
+
+    def test_unitary_decomposition_via_definition_2q(self):
+        """Test decomposition for 2Q unitary via definition."""
+        mat = numpy.array([[0, 0, 1, 0], [0, 0, 0, -1], [1, 0, 0, 0], [0, -1, 0, 0]])
+        numpy.allclose(Operator(UnitaryGate(mat).definition).data, mat)

test/python/transpiler/test_decompose.py

@@ -133,3 +133,15 @@ def test_decompose_global_phase_2q(self):
         qc.rxx(0.2, 0, 1)
         qcd = qc.decompose()
         self.assertEqual(Operator(qc), Operator(qcd))
+
+    def test_decompose_global_phase_1q_composite(self):
+        """Test decomposition of circuit with global phase in a composite gate."""
+        circ = QuantumCircuit(1, global_phase=pi / 2)
+        circ.x(0)
+        circ.h(0)
+        v = circ.to_gate()
+
+        qc = QuantumCircuit(1)
+        qc.append(v, [0])
+        qcd = qc.decompose()
+        self.assertEqual(Operator(qc), Operator(qcd))

test/python/transpiler/test_unroller.py

@@ -19,7 +19,8 @@
 from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit
 from qiskit.extensions.simulator import snapshot
 from qiskit.transpiler.passes import Unroller
-from qiskit.converters import circuit_to_dag
+from qiskit.converters import circuit_to_dag, dag_to_circuit
+from qiskit.quantum_info import Operator
 from qiskit.test import QiskitTestCase
 from qiskit.exceptions import QiskitError
 from qiskit.circuit import Parameter
@@ -264,6 +265,22 @@ def test_unrolling_nested_gates_preserves_qregs_order(self):
 
         self.assertEqual(circuit_to_dag(expected), out_dag)
 
+    def test_unrolling_global_phase_1q(self):
+        """Test unrolling a circuit with global phase in a composite gate."""
+        circ = QuantumCircuit(1, global_phase=pi / 2)
+        circ.x(0)
+        circ.h(0)
+        v = circ.to_gate()
+
+        qc = QuantumCircuit(1)
+        qc.append(v, [0])
+
+        dag = circuit_to_dag(qc)
+        out_dag = Unroller(['cx', 'x', 'h']).run(dag)
+        qcd = dag_to_circuit(out_dag)
+
+        self.assertEqual(Operator(qc), Operator(qcd))
+
 
 class TestUnrollAllInstructions(QiskitTestCase):
     """Test unrolling a circuit containing all standard instructions."""