add global_phase to QuantumCircuit class

[ewinston]

Summary

Adds phase property to QuantumCircuit.

Details and comments

This is an adaptation of #3930.

Depends on #4622.

[1ucian0]

It this PR addressing #3304 ?

[ewinston]

@1ucian0 That depends on whether to_matrix is supposed to agree with its definition and/or the associated EquivalenceLibrary circuit. If just the later, then this should resolve 3304. If it also needs to support definition then maybe not.

[ajavadia]

The approach looks good.

There are a few places that needs to be changed to accomodate global phases. I think we should leave anything that is about transpiling (or synthesis) for the future and here just make sure the following work:

• circuit.inverse() should negate the phase.
• circuit.compose() should add the two phases (combine/extend too but they are being deprecated Deprecate circuit.combine and circuit.extend #4208)
• I think we should add notes to circuit_to_instruction that circuit_to_gate that global phase will be lost in this conversion (since gates/instructions are considered immutable contrary to circuits).
• gate.to_matrix() as the gate definition, for all gates.
• gate.control() (seems like already covered here)
• Operator(circuit) (already done here)
• State.evolve(circuit) to respect global phase and add tests (this is missing)

[ajavadia]

@1ucian0 That depends on whether to_matrix is supposed to agree with its definition and/or the associated EquivalenceLibrary circuit. If just the later, then this should resolve 3304. If it also needs to support definition then maybe not.

I think it should agree with the definition (the matrix is what defines a gate).

[ajavadia]

Ok nice it seems to work well now:

theta = 0
circ = QuantumCircuit(2, global_phase=theta)
circ.cx(0, 1)
print(circ.to_gate().control().definition)

control_0: ──■──
             │
 target_0: ──■──
           ┌─┴─┐
 target_1: ┤ X ├
           └───┘

circ.global_phase = np.pi/2
print(circ.to_gate().control().definition)

                ┌──────────┐
control_0: ──■──┤ U1(pi/2) ├
             │  └──────────┘
 target_0: ──■──────────────
           ┌─┴─┐
 target_1: ┤ X ├────────────
           └───┘

print(circ.to_gate().control(2).definition)

control_0: ──■───■─────
             │   │pi/2
control_1: ──■───■─────
             │
 target_0: ──■─────────
           ┌─┴─┐
 target_1: ┤ X ├───────
           └───┘

print(circ.to_gate().control(3).definition)
control_0: ──■───■─────
             │   │
control_1: ──■───■─────
             │   │pi/2
control_2: ──■───■─────
             │
 target_0: ──■─────────
           ┌─┴─┐
 target_1: ┤ X ├───────
           └───┘

[ajavadia]

👍

[Cryoris]

Just so this is noted somewhere: BasicAer doesn't get the phases right (neither does Aer, but that'll be fixed within Aer I presume). The tests using BasicAer, which I think is only test_to_matrix in test_extensions_standard works by coincidence because the rotation angles are 0 and thereby the phase difference is e^{i0} = 1.

>>> qc = QuantumCircuit(1)
>>> qc.rz(0.2, 0)
>>> Operator(qc)
Operator([[0.99500417-0.09983342j, 0.        +0.j        ],
          [0.        +0.j        , 0.99500417+0.09983342j]],
         input_dims=(2,), output_dims=(2,))
>>> execute(qc, Aer.get_backend('unitary_simulator')).result().get_unitary()
array([[1.        +0.j        , 0.        +0.j        ],
       [0.        +0.j        , 0.98006658+0.19866933j]])
>>> execute(qc, BasicAer.get_backend('unitary_simulator')).result().get_unitary()
array([[1.        +0.j        , 0.        +0.j        ],
       [0.        +0.j        , 0.98006658+0.19866933j]])

[1ucian0]

BasicAer doesn't get the phases right (neither does Aer, but that'll be fixed within Aer I presume).

Do you mind submitting issues to terra and aer about this for better tracking?

[Cryoris]

Sure 👍 See #4805.

[faisaldebouni]

I apologies if my comment is out of place but I'm not sure I understand this pr correctly. Would you please help me understand the following points:

First, when building a controlled gate (add_control.py), you add phase gates to the control wires. As I understood it, this is to account for the global phase in the gate's circuit definition . However when you do that, you also add the accumulated phase from the gates you unrolled to (lines 168 & 170). If I understand this correctly, shouldn't you subtract the accumulated global phase and not add it? Because if a gate/circuit has a global phase of say theta, then you unroll it, but the new set of gates adds a controlled global phase of say phi, then you only need to account for theta - phi and not theta + phi.

Second, this pr adds global phase of -theta/2 to Rz's definition (line72 in rz.py). however, there are other standard gates which use Rz in their definitions, but do not inherit the global phase of Rz. Namely:
RXXGate, RYYGate, RZXGate, RZZGate and MSGate (there might be others).
Would it be appropriate to add global phase to those gates?

Finally, it's mentioned in the documentation, and in the discussion that global phase should be attached to circuits and not gates. Furthermore, that circuit_to_gate would lose the global phase. If that is the case, why then the global phase needs to be accounted for when building a controlled gate? (notice that circuit.control() converts a circuit to a gate, controls that gate, then creates a new circuit with the controlled gate). If however, you need to account for the phase when directly controlling a circuit, then this should be done outside the scope of building the controlled gate.

I apologies again if my comments are inappropriate, I'm working on another pr and I don't want to step on anyone's toe by making changes that would interfere with this pr.

[Cryoris]

Second, this pr adds global phase of -theta/2 to Rz's definition (line72 in rz.py). however, there are other standard gates which use Rz in their definitions, but do not inherit the global phase of Rz.

RZ was a bit a special case since before this PR it was not implementing the matrix you would expect exactly, but only up to a global phase of -theta/2. Since that doesn't matter on measurements we could still define RZ this way and all other gates that use it, like RZZ etc. The problem was just, that all the matrices of these gates were also incorrect. Now that RZ is fixed, all these are fixed automatically. They don't need an additional global phase, since RZ is correct now.

Finally, it's mentioned in the documentation, and in the discussion that global phase should be attached to circuits and not gates. Furthermore, that circuit_to_gate would lose the global phase. If that is the case, why then the global phase needs to be accounted for when building a controlled gate?

If we have a NxN matrix that represents the action of the circuit the measurement outcome will be the same even if we multiply this entire matrix by a factor exp(i theta). This theta is the global phase. However if we only multiply a sub-part of the matrix with this factor, the results change. E.g. if you have a global phase on a single qubit gate, the 2x2 matrix of the gate, say G, becomes phase * G. If you control that gate what you get is

( I |          )             ( I |   ) 
(   | phase * G)  != phase * (   | G ) 

Since we cannot pull phase out of the matrix the global phase matters and therefore we need to account for the global phase if we control a gate/circuit.

[faisaldebouni]

I'm not sure my points were clear. This PR implements "to_matrix()" for Rz with the correct matrix. Thus, Rz's matrix and the matrices for gate/circuits that uses Rz are indeed correct now. But what is used in running the circuit is the gate's circuit definition. Not the matrix representation.

The global_phase variable used in Rz circuit definition doesn't actually affect Rz. It just gets tracked.

If you control a gate with Rz in it's definition, it will give the correct gate if and only if it get's unrolled to Rz in add_control.py. Which shouldn't be the case, as long as you unroll to universal set of gates. For example:

qc = QuantumCircuit(1)
qc.sx(0)
qc.control(1).decompose().draw()

The generated circuit is not correct. It doesn't have controlled phase since we don't unroll to sx.

I apologies again, I just wanted to make sure I understand this PR correctly.

I will link to a dummy branch with what I thought this PR would do to help make my point clearer. Meanwhile, I'll assume that what is in this PR is the intended behavior and move on.

Dummy branch
Implements global_phase as gates once circuit is converted to gate/dag. (as mentioned in this pr's documentation), controls Rz correctly and adds controlled phase to the circuit, not the gate (in circuit.control() ).

As a result, it solves the the issues with BasicAer (matrix definition matches unitary simulator when optimization level=0) and other inconsistencies in the current implementation.

note: This is not a rigorous implementation and only meant to clarify my point