Fix global phase tracking around DAGCircuit.substitute_node. (bp #5618)

qiskit/circuit/library/grover_operator.py

@@ -256,6 +256,8 @@ def _build(self):
             self.barrier()
         self.compose(self.state_preparation, list(range(self.state_preparation.num_qubits)),
                      inplace=True)
+        # minus sign
+        self.global_phase = numpy.pi
 
 
 # TODO use the oracle compiler or the bit string oracle

qiskit/circuit/quantumcircuit.py

@@ -1799,7 +1799,7 @@ def global_phase(self, angle):
         Args:
             angle (float, ParameterExpression): radians
         """
-        if isinstance(angle, ParameterExpression):
+        if isinstance(angle, ParameterExpression) and angle.parameters:
             self._global_phase = angle
         else:
             # Set the phase to the [-2 * pi, 2 * pi] interval

qiskit/transpiler/passes/basis/basis_translator.py

@@ -151,12 +151,14 @@ def run(self, dag):
                     bound_target_dag = circuit_to_dag(target_circuit)
                 else:
                     bound_target_dag = target_dag
-                if bound_target_dag.global_phase:
-                    dag.global_phase += bound_target_dag.global_phase
+
                 if (len(bound_target_dag.op_nodes()) == 1
                         and len(bound_target_dag.op_nodes()[0].qargs) == len(node.qargs)):
                     dag_op = bound_target_dag.op_nodes()[0].op
                     dag.substitute_node(node, dag_op, inplace=True)
+
+                    if bound_target_dag.global_phase:
+                        dag.global_phase += bound_target_dag.global_phase
                 else:
                     dag.substitute_node_with_dag(node, bound_target_dag)
             else:

qiskit/transpiler/passes/basis/unroller.py

@@ -95,8 +95,5 @@ 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 unrolled_dag.global_phase:
-                    dag.global_phase += unrolled_dag.global_phase
-                    unrolled_dag.global_phase = 0
                 dag.substitute_node_with_dag(node, unrolled_dag)
         return dag

releasenotes/notes/dagcircuit-fix-substitute-node-global-phase-usage-ee05476a7d0b24c6.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The :class:`~qiskit.transpiler.passes.BasisTranslator` and :class:`~qiskit.transpiler.passes.Unroller` passes, in some cases, had not been
+    preserving the global phase of the circuit under transpilation. This has
+    been fixed.

test/python/circuit/library/test_grover_operator.py

@@ -106,12 +106,11 @@ def test_custom_zero_reflection(self):
             self.assertGroverOperatorIsCorrect(grover_op, oracle)
 
         with self.subTest('circuits match'):
-            expected = QuantumCircuit(*grover_op.qregs)
+            expected = QuantumCircuit(*grover_op.qregs, global_phase=np.pi)
             expected.compose(oracle, inplace=True)
             expected.h(0)  # state_in is H
             expected.compose(zero_reflection, inplace=True)
             expected.h(0)
-
             self.assertEqual(expected, grover_op)
 
     def test_num_mcx_ancillas(self):

test/python/circuit/test_circuit_qasm.py

@@ -16,7 +16,6 @@
 
 from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit
 from qiskit.test import QiskitTestCase
-from qiskit.quantum_info import random_unitary
 
 
 class TestCircuitQasm(QiskitTestCase):
@@ -176,8 +175,7 @@ def test_circuit_qasm_pi(self):
         """Test circuit qasm() method with pi params.
         """
         circuit = QuantumCircuit(2)
-        circuit.append(random_unitary(4, seed=1234), [0, 1])
-        circuit = circuit.decompose()
+        circuit.cz(0, 1)
         circuit.u(2*pi, 3*pi, -5*pi, 0)
         qasm_str = circuit.qasm()
         circuit2 = QuantumCircuit.from_qasm_str(qasm_str)

test/python/compiler/test_transpiler.py

@@ -436,7 +436,7 @@ def test_parameterized_circuit_for_simulator(self):
 
         transpiled_qc = transpile(qc, backend=BasicAer.get_backend('qasm_simulator'))
 
-        expected_qc = QuantumCircuit(qr)
+        expected_qc = QuantumCircuit(qr, global_phase=-1 * theta / 2.0)
         expected_qc.append(U1Gate(theta), [qr[0]])
 
         self.assertEqual(expected_qc, transpiled_qc)
@@ -453,7 +453,7 @@ def test_parameterized_circuit_for_device(self):
                                   initial_layout=Layout.generate_trivial_layout(qr))
 
         qr = QuantumRegister(14, 'q')
-        expected_qc = QuantumCircuit(qr)
+        expected_qc = QuantumCircuit(qr, global_phase=-1 * theta / 2.0)
         expected_qc.append(U1Gate(theta), [qr[0]])
 
         self.assertEqual(expected_qc, transpiled_qc)
@@ -470,7 +470,7 @@ def test_parameter_expression_circuit_for_simulator(self):
 
         transpiled_qc = transpile(qc, backend=BasicAer.get_backend('qasm_simulator'))
 
-        expected_qc = QuantumCircuit(qr)
+        expected_qc = QuantumCircuit(qr, global_phase=-1 * square / 2.0)
         expected_qc.append(U1Gate(square), [qr[0]])
         self.assertEqual(expected_qc, transpiled_qc)
 
@@ -488,7 +488,7 @@ def test_parameter_expression_circuit_for_device(self):
                                   initial_layout=Layout.generate_trivial_layout(qr))
 
         qr = QuantumRegister(14, 'q')
-        expected_qc = QuantumCircuit(qr)
+        expected_qc = QuantumCircuit(qr, global_phase=-1 * square / 2.0)
         expected_qc.append(U1Gate(square), [qr[0]])
         self.assertEqual(expected_qc, transpiled_qc)
 
@@ -927,14 +927,16 @@ def test_circuit_with_delay(self, optimization_level):
     @data(1, 2, 3)
     def test_no_infinite_loop(self, optimization_level):
         """Verify circuit cost always descends and optimization does not flip flop indefinitely."""
-
         qc = QuantumCircuit(1)
         qc.ry(0.2, 0)
 
         out = transpile(qc, basis_gates=['id', 'p', 'sx', 'cx'],
                         optimization_level=optimization_level)
 
-        expected = QuantumCircuit(1)
+        # Expect a -pi/2 global phase for the U3 to RZ/SX conversion, and
+        # a -0.5 * theta phase for RZ to P twice, once at theta, and once at 3 pi
+        # for the second and third RZ gates in the U3 decomposition.
+        expected = QuantumCircuit(1, global_phase=-np.pi/2 - 0.5 * (0.2 + np.pi) - 0.5 * 3 * np.pi)
         expected.sx(0)
         expected.p(np.pi + 0.2, 0)
         expected.sx(0)

test/python/transpiler/test_basis_translator.py

@@ -156,6 +156,138 @@ def test_double_substitution(self):
 
         self.assertEqual(actual, expected_dag)
 
+    def test_single_substitution_with_global_phase(self):
+        """Verify we correctly unroll gates through a single equivalence with global phase."""
+        eq_lib = EquivalenceLibrary()
+
+        gate = OneQubitZeroParamGate()
+        equiv = QuantumCircuit(1, global_phase=0.2)
+        equiv.append(OneQubitOneParamGate(pi), [0])
+
+        eq_lib.add_equivalence(gate, equiv)
+
+        qc = QuantumCircuit(1, global_phase=0.1)
+        qc.append(OneQubitZeroParamGate(), [0])
+        dag = circuit_to_dag(qc)
+
+        expected = QuantumCircuit(1, global_phase=0.1 + 0.2)
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected_dag = circuit_to_dag(expected)
+
+        pass_ = BasisTranslator(eq_lib, ['1q1p'])
+        actual = pass_.run(dag)
+
+        self.assertEqual(actual, expected_dag)
+
+    def test_single_two_gate_substitution_with_global_phase(self):
+        """Verify we correctly unroll gates through a single equivalence with global phase."""
+        eq_lib = EquivalenceLibrary()
+
+        gate = OneQubitZeroParamGate()
+        equiv = QuantumCircuit(1, global_phase=0.2)
+        equiv.append(OneQubitOneParamGate(pi), [0])
+        equiv.append(OneQubitOneParamGate(pi), [0])
+
+        eq_lib.add_equivalence(gate, equiv)
+
+        qc = QuantumCircuit(1, global_phase=0.1)
+        qc.append(OneQubitZeroParamGate(), [0])
+        dag = circuit_to_dag(qc)
+
+        expected = QuantumCircuit(1, global_phase=0.1 + 0.2)
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected_dag = circuit_to_dag(expected)
+
+        pass_ = BasisTranslator(eq_lib, ['1q1p'])
+        actual = pass_.run(dag)
+
+        self.assertEqual(actual, expected_dag)
+
+    def test_two_substitutions_with_global_phase(self):
+        """Verify we correctly unroll gates through a single equivalences with global phase."""
+        eq_lib = EquivalenceLibrary()
+
+        gate = OneQubitZeroParamGate()
+        equiv = QuantumCircuit(1, global_phase=0.2)
+        equiv.append(OneQubitOneParamGate(pi), [0])
+
+        eq_lib.add_equivalence(gate, equiv)
+
+        qc = QuantumCircuit(1, global_phase=0.1)
+        qc.append(OneQubitZeroParamGate(), [0])
+        qc.append(OneQubitZeroParamGate(), [0])
+        dag = circuit_to_dag(qc)
+
+        expected = QuantumCircuit(1, global_phase=0.1 + 2 * 0.2)
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected_dag = circuit_to_dag(expected)
+
+        pass_ = BasisTranslator(eq_lib, ['1q1p'])
+        actual = pass_.run(dag)
+
+        self.assertEqual(actual, expected_dag)
+
+    def test_two_single_two_gate_substitutions_with_global_phase(self):
+        """Verify we correctly unroll gates through a single equivalence with global phase."""
+        eq_lib = EquivalenceLibrary()
+
+        gate = OneQubitZeroParamGate()
+        equiv = QuantumCircuit(1, global_phase=0.2)
+        equiv.append(OneQubitOneParamGate(pi), [0])
+        equiv.append(OneQubitOneParamGate(pi), [0])
+
+        eq_lib.add_equivalence(gate, equiv)
+
+        qc = QuantumCircuit(1, global_phase=0.1)
+        qc.append(OneQubitZeroParamGate(), [0])
+        qc.append(OneQubitZeroParamGate(), [0])
+        dag = circuit_to_dag(qc)
+
+        expected = QuantumCircuit(1, global_phase=0.1 + 2 * 0.2)
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected.append(OneQubitOneParamGate(pi), [0])
+        expected.append(OneQubitOneParamGate(pi), [0])
+
+        expected_dag = circuit_to_dag(expected)
+
+        pass_ = BasisTranslator(eq_lib, ['1q1p'])
+        actual = pass_.run(dag)
+
+        self.assertEqual(actual, expected_dag)
+
+    def test_double_substitution_with_global_phase(self):
+        """Verify we correctly unroll gates through multiple equivalences with global phase."""
+        eq_lib = EquivalenceLibrary()
+
+        gate = OneQubitZeroParamGate()
+        equiv = QuantumCircuit(1, global_phase=0.2)
+        equiv.append(OneQubitOneParamGate(pi), [0])
+
+        eq_lib.add_equivalence(gate, equiv)
+
+        theta = Parameter('theta')
+        gate = OneQubitOneParamGate(theta)
+        equiv = QuantumCircuit(1, global_phase=0.4)
+        equiv.append(OneQubitTwoParamGate(theta, pi/2), [0])
+
+        eq_lib.add_equivalence(gate, equiv)
+
+        qc = QuantumCircuit(1, global_phase=0.1)
+        qc.append(OneQubitZeroParamGate(), [0])
+        dag = circuit_to_dag(qc)
+
+        expected = QuantumCircuit(1, global_phase=0.1 + 0.2 + 0.4)
+        expected.append(OneQubitTwoParamGate(pi, pi/2), [0])
+        expected_dag = circuit_to_dag(expected)
+
+        pass_ = BasisTranslator(eq_lib, ['1q2p'])
+        actual = pass_.run(dag)
+
+        self.assertEqual(actual, expected_dag)
+
     def test_multiple_variadic(self):
         """Verify circuit with multiple instances of variadic gate."""
         eq_lib = EquivalenceLibrary()
@@ -299,7 +431,8 @@ def test_unroll_1q_chain_conditional(self):
         pass_ = BasisTranslator(std_eqlib, ['u1', 'u2', 'u3'])
         unrolled_dag = pass_.run(dag)
 
-        ref_circuit = QuantumCircuit(qr, cr)
+        # Pick up -1 * 0.3 / 2 global phase for one RZ -> U1.
+        ref_circuit = QuantumCircuit(qr, cr, global_phase=-0.3 / 2)
         ref_circuit.append(U2Gate(0, pi), [qr[0]])
         ref_circuit.append(U1Gate(-pi/4), [qr[0]])
         ref_circuit.append(U1Gate(pi), [qr[0]])
@@ -312,6 +445,7 @@ def test_unroll_1q_chain_conditional(self):
         ref_circuit.append(U3Gate(pi, pi/2, pi/2), [qr[0]]).c_if(cr, 1)
         ref_circuit.append(U1Gate(pi), [qr[0]]).c_if(cr, 1)
         ref_dag = circuit_to_dag(ref_circuit)
+
         self.assertEqual(unrolled_dag, ref_dag)
 
     def test_unroll_no_basis(self):
@@ -488,7 +622,7 @@ def test_simple_unroll_parameterized_without_expressions(self):
 
         unrolled_dag = BasisTranslator(std_eqlib, ['u1', 'cx']).run(dag)
 
-        expected = QuantumCircuit(qr)
+        expected = QuantumCircuit(qr, global_phase=-theta / 2)
         expected.append(U1Gate(theta), [qr[0]])
 
         self.assertEqual(circuit_to_dag(expected), unrolled_dag)
@@ -509,7 +643,7 @@ def test_simple_unroll_parameterized_with_expressions(self):
 
         unrolled_dag = BasisTranslator(std_eqlib, ['p', 'cx']).run(dag)
 
-        expected = QuantumCircuit(qr)
+        expected = QuantumCircuit(qr, global_phase=-sum_ / 2)
         expected.p(sum_, qr[0])
 
         self.assertEqual(circuit_to_dag(expected), unrolled_dag)
@@ -558,7 +692,9 @@ def test_unrolling_parameterized_composite_gates(self):
 
         out_dag = BasisTranslator(mock_sel, ['p', 'cx']).run(dag)
 
-        expected = QuantumCircuit(qr2)
+        # Pick up -1 * theta / 2 global phase four twice (once for each RZ -> P
+        # in each of the two sub_instr instructions).
+        expected = QuantumCircuit(qr2, global_phase=-1 * 4 * theta / 2.0)
         expected.p(theta, qr2[0])
         expected.p(theta, qr2[2])
         expected.cx(qr2[0], qr2[1])
@@ -585,7 +721,7 @@ def test_unrolling_parameterized_composite_gates(self):
 
         out_dag = BasisTranslator(mock_sel, ['p', 'cx']).run(dag)
 
-        expected = QuantumCircuit(qr2)
+        expected = QuantumCircuit(qr2, global_phase=-1 * (2 * phi + 2 * gamma) / 2.0)
         expected.p(phi, qr2[0])
         expected.p(gamma, qr2[2])
         expected.cx(qr2[0], qr2[1])
@@ -608,22 +744,8 @@ def test_cx_bell_to_cz(self):
         in_dag = circuit_to_dag(bell)
         out_dag = BasisTranslator(std_eqlib, ['cz', 'rx', 'rz']).run(in_dag)
 
-        qr = QuantumRegister(2, 'q')
-        expected = QuantumCircuit(qr)
-        expected.rz(pi, qr)
-        expected.rx(pi / 2, qr)
-        expected.rz(3 * pi / 2, qr)
-        expected.rx(pi / 2, qr)
-        expected.rz(3 * pi, qr)
-        expected.cz(qr[0], qr[1])
-        expected.rz(pi, qr[1])
-        expected.rx(pi / 2, qr[1])
-        expected.rz(3 * pi / 2, qr[1])
-        expected.rx(pi / 2, qr[1])
-        expected.rz(3 * pi, qr[1])
-        expected_dag = circuit_to_dag(expected)
-
-        self.assertEqual(out_dag, expected_dag)
+        self.assertTrue(set(out_dag.count_ops()).issubset(['cz', 'rx', 'rz']))
+        self.assertEqual(Operator(bell), Operator(dag_to_circuit(out_dag)))
 
     def test_cx_bell_to_iswap(self):
         """Verify we can translate a CX bell to iSwap,U3."""
@@ -634,22 +756,8 @@ def test_cx_bell_to_iswap(self):
         in_dag = circuit_to_dag(bell)
         out_dag = BasisTranslator(std_eqlib, ['iswap', 'u']).run(in_dag)
 
-        qr = QuantumRegister(2, 'q')
-        expected = QuantumCircuit(2)
-        expected.u(pi / 2, 0, pi, qr[0])
-        expected.u(pi, 0, pi, qr[1])
-        expected.u(pi / 2, 0, pi, qr)
-        expected.iswap(qr[0], qr[1])
-        expected.u(pi, 0, pi, qr)
-        expected.u(pi / 2, 0, pi, qr[1])
-        expected.iswap(qr[0], qr[1])
-        expected.u(pi / 2, 0, pi, qr[0])
-        expected.u(0, 0, pi / 2, qr)
-        expected.u(pi, 0, pi, qr[1])
-        expected.u(pi / 2, 0, pi, qr[1])
-        expected_dag = circuit_to_dag(expected)
-
-        self.assertEqual(out_dag, expected_dag)
+        self.assertTrue(set(out_dag.count_ops()).issubset(['iswap', 'u']))
+        self.assertEqual(Operator(bell), Operator(dag_to_circuit(out_dag)))
 
     def test_global_phase(self):
         """Verify global phase preserved in basis translation"""
@@ -668,3 +776,4 @@ def test_global_phase(self):
         expected_dag = circuit_to_dag(expected)
         self.assertEqual(out_dag, expected_dag)
         self.assertEqual(float(out_dag.global_phase), float(expected_dag.global_phase))
+        self.assertEqual(Operator(dag_to_circuit(out_dag)), Operator(expected))