Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A little inconsistency of the gate RZ #70

Closed
DevelopDaily opened this issue Jan 5, 2020 · 3 comments
Closed

A little inconsistency of the gate RZ #70

DevelopDaily opened this issue Jan 5, 2020 · 3 comments
Assignees
Labels

Comments

@DevelopDaily
Copy link

The current implementation of the gate RZ is simply to call Rn(theta, {0, 0, 1}). That is good. The current definition of the rz in the QASM is gate rz(phi) a { u1(phi) a; }. They will end up with two different matrices. Both are correct, but they just look different. Let me illustrate them in this image:

RZ

The Rn(theta, {0, 0, 1}) will be in the form of "A"; gate rz(phi) a { u1(phi) a; } "B".

Since the latter looks prettier, would it be nice to shift the phase by θ/2, as suggested in the image, before the cmat RZ(double theta) const returns?

How do you think?

@vsoftco vsoftco self-assigned this Jan 5, 2020
@vsoftco
Copy link
Member

vsoftco commented Jan 6, 2020

Thanks! The only issue is that a phase becomes important in a controlled-operation. Say that the matrix is Id_phi := e^{i \phi} x Identity. Then this unitary produces the same effect as Identity if acting on one qubit. But, if we construct something like CTRL-Id_phi, then the effects are visible, e.g. acting this gate on |00>+|11> will produce |00> + e^{i\phi}|11>, which is not just a phase times the initial state. In our case, we need to be consistent.

Currently the qasm rz returns gt.RZ, see

return gt.RZ(args[0]);

But if one uses qiskit, one would expect (as you mentioned), to have rz return diag(1, phase). I am tempted to just make rz return diag(1, phase) (which will be different from gt.RZ, so we implement the qiskit spec correctly.

I assume that the other rotations don't have inconsistencies (rx and ry), do they?

@vsoftco
Copy link
Member

vsoftco commented Jan 6, 2020

So I ended up implementing rz as the QISKIT specs, i.e. diag(1, e^{i\phi}), and kept gt.RZ as is in qpp; also documented in ast.h why the choice. Basically now every QASM code is fully compliant with the QISKIT implementation (which is slightly different from the openQASM specs, as you mentioned). The gates in class Gates use the standard Nielsen & Chuang conventions though (the same as the one in the openQASM specs).

@vsoftco vsoftco closed this as completed in fa55b73 Jan 6, 2020
@DevelopDaily
Copy link
Author

Great. A well-thought-out solution. Thanks.

The rx and ry are consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants