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

Mismatch issues of the rz #117

Closed
DevelopDaily opened this issue Dec 20, 2021 · 15 comments
Closed

Mismatch issues of the rz #117

DevelopDaily opened this issue Dec 20, 2021 · 15 comments

Comments

@DevelopDaily
Copy link

I have noticed three issues related to the rz gate.

  1. Run this QASM script on the Qiskit here (https://quantum-computing.ibm.com) and on qpp. The final states are different. The IBM produces the state [ 0.866-0.5j, 0+0j ] (see the attached screenshot), but qpp [1, 0]. The option USE_OPENQASM2_SPECS=false is used, so the qpp result should match that on IBM, shouldn't it?
OPENQASM 2.0;
include "qelib1.inc";
qreg q[1];

rz(pi/3) q[0];

Qiskit result:

rz

  1. In the preprocessor.hpp, rz is defined identically:

    https://github.com/softwareQinc/qpp/blob/main/qasmtools/include/qasmtools/parser/preprocessor.hpp#L66
    https://github.com/softwareQinc/qpp/blob/main/qasmtools/include/qasmtools/parser/preprocessor.hpp#L103

    But, their gate implementations are different depending on the USE_OPENQASM2_SPECS.

    https://github.com/softwareQinc/qpp/blob/main/include/qasm/qasm.hpp#L116

    Is that a problem?

  2. There seems to be a documentation problem here (https://github.com/softwareQinc/qpp/blob/main/DISCREPANCIES.md) The rz difference is not described but it is implemented differently.

@525125
Copy link
Contributor

525125 commented Dec 20, 2021

(2) They are defined in the same way, but the difference is in the matrix we use for the U gate:

https://github.com/softwareQinc/qpp/blob/main/include/qasm/qasm.hpp#L563

This is the main source of all existing discrepancies.

(3) In both qelib1.incs, rz is identical to the u1 gate, so you can refer to that row in the discrepancy table for the implementations.

However, in the "usual" gate definitions, rz and u1 differ by a phase shift. But it is impossible to implement both using OpenQASM 2 code because the language comes with only U and CX gates, and no way to apply a global phase.

(1) Using OpenQASM 2's U gate, rz matches the "usual" definition (which is why it's not in the discrepancy table). Using Qiskit's U gate, rz matches the usual definition of u1 (which is why qpp didn't have the same output as IBM).

This is also related to #70

@DevelopDaily
Copy link
Author

Thanks for the clarifications for the (2) and (3).

I am still referring to the mode USE_OPENQASM2_SPECS=false. In that mode, it is your design intent to match the QISKIT behaviors, isn't it? I am not suggesting QISKIT is absolutely right, but it seems to work. I suspect they have done something special in the rz and u1 implementation.

I did four tests to show the final states out of rz and u1 on QISKIT and qpp.

In QISKIT:

rz(pi/3) q[0]; // => [ 0.866-0.5j, 0+0j ]

u1(pi/3) q[0]; // => [ 1+0j, 0+0j ]

In qpp:

rz(pi/3) q[0]; // => 1 0

u1(pi/3) q[0]; // => 1 0

So, the QISKIT does reflect what you said about "..., rz and u1 differ by a phase shift", qpp does not.

@525125
Copy link
Contributor

525125 commented Dec 20, 2021

Maybe it is more clear if we look at the OpenQASM 3.0 standard library (from https://arxiv.org/pdf/2104.14722.pdf):

gate rz(λ) a { gphase(-λ/2); U(0, 0, λ) a; }
gate u1(λ) q { U(0, 0, λ) q; }

This matches the Qiskit specification. However, there is no equivalent to gphase in OpenQASM 2. If we ignore that and use a different matrix for rz, that will cause phase shift issues if the rz gate is inlined.

@DevelopDaily
Copy link
Author

I am ignorant of the "inlined" use cases, so I cannot comment on that. Can I read about it somewhere?

Any workaround possible to deal with the situation? I don't have any useful suggestions.

The identicality of rz and u1 in qpp won't allow me to do any Quantum Phase Estimation apps in QASM. So, I tweaked the rz and crz mimicking QISKIT to suit my apps. So far, so good. But, I guess I will run into troubles when there is an "inlined" situation down the road.

@525125
Copy link
Contributor

525125 commented Dec 21, 2021

One example of a use case is if there are hardware constraints. The staq mapper fully inlines all gates to the builtin U, CX gate set.

To resolve the issue, we can define rz as

gate rz(phi) a { u1(phi) a; x a; u1(-phi/2) a; x a; u1(-phi/2) a; }

The extra x-u1-x-u1 sequence adds the required phase (assuming Qiskit standard gates). We're currently testing this change.

@vsoftco
Copy link
Member

vsoftco commented Jan 11, 2022

@DevelopDaily can you check this now please?

@DevelopDaily
Copy link
Author

Passed the simple test cases. Will test more complex cases...

@DevelopDaily
Copy link
Author

The staq fails a slightly more complex test case. Here are two files (input-O2-output.zip): input.qasm and output.qasm, the latter being the result of this command:

./staq -O2 -o output.qasm input.qasm

Executing the example program qpp_qasm on them produces drastically different final states.

From the input.qasm:

>> Final state (transpose):
0.132458 - 0.135273i  0  0  0  0.0779287 - 0.0721376i  0  0  0  0.0566401 - 0.0474889i  0  0  0  0.0452555 - 0.0343073i  0  0  0  0.0381398 - 0.0260685i  0  0  0  0.0332513 - 0.0204084i  0  0  0  0.0296709 - 0.0162629i  0  0  0  0.0269239 - 0.0130823i  0  0  0  0.0247401 - 0.0105539i  0  0  0  0.0229548 - 0.00848669i  0  0  0  0.0214613 - 0.00675747i  0  0  0  0.0201878 - 0.00528302i  0  0  0  0.0190842 - 0.00400516i  0  0  0  0.0181141 - 0.00288197i  0  0  0  0.0172508 - 0.00188243i  0  0  0  0.0164741 - 0.000983081i  0  0  0  0.0157682 - 0.00016583i  0  0  0  0.015121 + 0.000583523i  0  0  0  0.0145227 + 0.00127631i  0  0  0  0.0139653 + 0.00192169i  0  0  0  0.0134423 + 0.00252719i  0  0  0  0.0129484 + 0.00309907i  0  0  0  0.012479 + 0.00364259i  0  0  0  0.0120302 + 0.00416228i  0  0  0  0.0115985 + 0.00466199i  0  0  0  0.0111812 + 0.00514515i  0  0  0  0.0107756 + 0.0056148i  0  0  0  0.0103793 + 0.00607368i  0  0  0  0.00999013 + 0.00652428i  0  0  0  0.00960607 + 0.00696897i  0  0  0  0.00922522 + 0.00740997i  0  0  0  0.00884558 + 0.00784932i  0  0  0  0.0084656 + 0.00828939i  0  0  0  0.00808322 + 0.00873213i  0  0  0  0.00769661 + 0.00917977i  0  0  0  0.0073038 + 0.00963461i  0  0  0  0.00690267 + 0.010099i  0  0  0  0.00649098 + 0.0105757i  0  0  0  0.00606621 + 0.0110675i  0  0  0  0.00562559 + 0.0115777i  0  0  0  0.00516594 + 0.0121099i  0  0  0  0.00468365 + 0.0126683i  0  0  0  0.00417447 + 0.0132578i  0  0  0  0.00363343 + 0.0138843i  0  0  0  0.00305454 + 0.0145546i  0  0  0  0.00243058 + 0.015277i  0  0  0  0.00175269 + 0.0160619i  0  0  0  0.00100981 + 0.016922i  0  0  0  0.000187972 + 0.0178736i  0  0  0  -0.000730796 + 0.0189374i  0  0  0  -0.00177003 + 0.0201406i  0  0  0  -0.00296123 + 0.0215199i  0  0  0  -0.00434755 + 0.023125i  0  0  0  -0.00598983 + 0.0250265i  0  0  0  -0.0079767 + 0.0273269i  0  0  0  -0.0104426 + 0.0301821i  0  0  0  -0.0136018 + 0.0338399i  0  0  0  -0.0178182 + 0.0387218i  0  0  0  -0.0237634 + 0.0456054i  0  0  0  -0.0328298 + 0.0561028i  0  0  0  -0.048452 + 0.0741908i  0  0  0  -0.0820209 + 0.113058i  0  0  0  -0.207621 + 0.258483i  0  0  0  0.585184 - 0.659457i  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0

From the output.qasm:

>> Final state (transpose):
-0.707107 - 0.707107i  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0

@vsoftco
Copy link
Member

vsoftco commented Jan 14, 2022

Adding @meamy In staq, those changes are still on the dev branch (not yet merged to main). Can you @DevelopDaily
please checkout the dev branch of staq and check again? https://github.com/softwareQinc/staq/tree/dev

@DevelopDaily
Copy link
Author

DevelopDaily commented Jan 15, 2022

From the output.qasm, based on the dev branch:

>> Final state (transpose):
-0.189314 + 0.00199097i  0  0  0  -0.106113 - 0.00409491i  0  0  0  -0.0736303 - 0.0064709i  0  0  0  -0.0562593 - 0.00774154i  0  0  0  -0.0454021 - 0.0085357i  0  0  0  -0.0379431 - 0.0090813i  0  0  0  -0.0324801 - 0.0094809i  0  0  0  -0.0282886 - 0.0097875i  0  0  0  -0.0249566 - 0.0100312i  0  0  0  -0.0222325 - 0.0102305i  0  0  0  -0.0199537 - 0.0103972i  0  0  0  -0.0180106 - 0.0105393i  0  0  0  -0.0163266 - 0.0106625i  0  0  0  -0.0148465 - 0.0107707i  0  0  0  -0.0135292 - 0.0108671i  0  0  0  -0.0123441 - 0.0109538i  0  0  0  -0.0112671 - 0.0110326i  0  0  0  -0.0102796 - 0.0111048i  0  0  0  -0.0093666 - 0.0111716i  0  0  0  -0.00851612 - 0.0112338i  0  0  0  -0.00771817 - 0.0112922i  0  0  0  -0.00696453 - 0.0113473i  0  0  0  -0.00624826 - 0.0113997i  0  0  0  -0.00556343 - 0.0114498i  0  0  0  -0.00490489 - 0.0114979i  0  0  0  -0.00426816 - 0.0115445i  0  0  0  -0.00364925 - 0.0115898i  0  0  0  -0.00304454 - 0.011634i  0  0  0  -0.00245073 - 0.0116775i  0  0  0  -0.00186471 - 0.0117203i  0  0  0  -0.00128358 - 0.0117629i  0  0  0  -0.000704461 - 0.0118051i  0  0  0  -0.000124598 - 0.0118476i  0  0  0  0.000458847 - 0.0118903i  0  0  0  0.00104875 - 0.0119334i  0  0  0  0.00164813 - 0.0119773i  0  0  0  0.00226017 - 0.012022i  0  0  0  0.00288834 - 0.012068i  0  0  0  0.00353646 - 0.0121154i  0  0  0  0.00420878 - 0.0121646i  0  0  0  0.00491012 - 0.0122159i  0  0  0  0.00564601 - 0.0122697i  0  0  0  0.00642292 - 0.0123265i  0  0  0  0.00724847 - 0.0123869i  0  0  0  0.00813174 - 0.0124515i  0  0  0  0.00908378 - 0.0125211i  0  0  0  0.0101181 - 0.0125968i  0  0  0  0.0112516 - 0.0126797i  0  0  0  0.0125056 - 0.0127714i  0  0  0  0.0139075 - 0.012874i  0  0  0  0.0154932 - 0.01299i  0  0  0  0.0173108 - 0.0131229i  0  0  0  0.019426 - 0.0132777i  0  0  0  0.0219318 - 0.0134609i  0  0  0  0.0249634 - 0.0136827i  0  0  0  0.028726 - 0.0139579i  0  0  0  0.0335464 - 0.0143105i  0  0  0  0.0399798 - 0.0147811i  0  0  0  0.0490511 - 0.0154446i  0  0  0  0.0628848 - 0.0164565i  0  0  0  0.0867215 - 0.0182001i  0  0  0  0.137942 - 0.0219467i  0  0  0  0.329585 - 0.0359647i  0  0  0  -0.880094 + 0.0525191i  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0  0

It still does not match that from the input.qasm.

Is it technically possible to preserve the final state of a circuit after it is optimized?

The current implementation seems to have been improved significantly. By the way, the input.qasm does quantum phase estimation. I couldn't do that on the output.qasm before this fix, but now I can. That being said, I am not sure if it will work for all the future more complex circuits.

@vsoftco
Copy link
Member

vsoftco commented Jan 15, 2022

@meamy any idea? Looks like the final state in output.qasm is (at least at a quick glance) the same up to a phase (the coefficients seem to have same magnitudes as the ones in input.qasm)

@DevelopDaily
Copy link
Author

I did another test.

./staq -O3 -o output3.qasm input.qasm

The final state of the output3.qasm matches that of the input.qasm, bit by bit. Amazing! Looks promising.

I don't know enough about the optimization theory. I did a superficial analysis of the output files from -O2 and -O3 options. The former contains two rz gates and the latter none.

@meamy
Copy link
Contributor

meamy commented Jan 17, 2022

@vsoftco There's an issue with the rotation folding optimization with the changes to rz and u1, which @DevelopDaily identified above. With both types of rotation gates, the overall global phase correction becomes harder to compute.

For the time being, we can merge in the changes and turn off global phase corrections, since they'll be incorrect anyway. This kind of semantics-breaking behaviour is something common in optimizing compilers anyway, but we should note it and plan to fix it eventually.

Thanks for the help @DevelopDaily!

@vsoftco
Copy link
Member

vsoftco commented Jan 17, 2022

@meamy Thanks, merging into main, but will keep the issue open

@vsoftco
Copy link
Member

vsoftco commented Jan 17, 2022

closing here, and leaving it open on staq softwareQinc/staq#45

@vsoftco vsoftco closed this as completed Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants