inconsistent Rz definition with Yao and qiskit

[Roger-luo]

currently PastaQ is using the following definition

https://github.com/GTorlai/PastaQ.jl/blob/master/src/circuits/gates.jl#L113

However, I believe if you look at the more general definition of a rotation,

\mathbf{I} cos(θ / 2) - im sin(θ / 2) * mat(U)

the Rz case should be

https://qiskit.org/documentation/stubs/qiskit.circuit.library.RZGate.html

which is what is used in Yao and qiskit, and this is causing the inconsistency in this issue

https://github.com/QuantumBFS/YaoPastaQ.jl/issues/18

I'm wondering if there were any specific reasons for picking up this definition? git blame doesn't seem to show anything meaningful in the commit message.

[mtfishman]

@Roger-luo, thanks for bringing up this issue. I don't remember the history of that definition (if I remember correctly, the gates were mostly defined by @GTorlai).

I'm fine with either definition, so it sounds best to follow the conventions of other libraries like Qiskit and Yao (I also see Wikipedia uses the definition you are proposing). I have taken the physicist view that the phase doesn't matter, but in practice it is important for doing direct comparisons of wavefunctions.

Do you see any other gates we've defined that have definitions that clash with Qiskit/Yao? Our "CRz" gate looks like it is also off by a phase.

[VarLad]

@mtfishman True, same for CRz gate, Yao/Qiskit use the same definition for CRz, while PastaQ uses a different notation

Although to be honest, if I made a C-Shift gate in Yao, that would be the same as the CRz gate in PastaQ.jl @Roger-luo Whats your opinion on that?

Also, those are the only 2 gates with different conventions I see in PastaQ.jl

[Roger-luo]

I think depends on how @mtfishman or @GTorlai on deciding this. I definitely prefer the definition to be consistent with Yao and make downstream packages like YaoPastaQ much easier to maintain.

[GTorlai]

I'm OK with modifying our current definition for better compatibility.

[mtfishman]

Alright, sounds like we're in agreement. Here is a plan:

1. Redefine "Rz" and "CRz" to match Yao/Quiskit (and Wikipedia: https://en.wikipedia.org/wiki/Quantum_logic_gate#Rotation_operator_gates).
2. Rename the current "CRz" to "CPHASE" (https://en.wikipedia.org/wiki/Quantum_logic_gate#Controlled_phase_shift).
3. Maybe add a phase shift gate (https://en.wikipedia.org/wiki/Quantum_logic_gate#Phase_shift_gates). Wikipedia calls this P(ϕ) so we could use "P", though we seem to be using "P" to specifically be the case of P(π/2) right now. That one we could actually do without breaking things if we define it with a keyword argument that defaults to ϕ=π/2, but that seems like a random default value so maybe that's not a good idea.

[mtfishman]

Anyone want to volunteer to make a PR?

@GTorlai, what do you think of generalizing the definition of "P"?

[GTorlai]

Sounds good, I'm making this changes.

[mtfishman]

Thanks @GTorlai!