softwareQinc / qpp

Modern C++ quantum computing library
https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0208073
MIT License
532 stars 116 forks source link

Mismatch issues of the rz #117

Closed DevelopDaily closed 2 years ago

DevelopDaily commented 2 years ago

I have noticed three issues related to the rzgate.

  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 qppresult 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, rzis 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 rzdifference is not described but it is implemented differently.

525125 commented 2 years ago

(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 commented 2 years ago

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 QISKITbehaviors, isn't it? I am not suggesting QISKITis absolutely right, but it seems to work. I suspect they have done something special in the rzand u1implementation.

I did four tests to show the final states out of rzand u1on QISKITand 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 QISKITdoes reflect what you said about "..., rzand u1differ by a phase shift", qppdoes not.

525125 commented 2 years ago

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 commented 2 years ago

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 rzand u1in qpp won't allow me to do any Quantum Phase Estimation apps in QASM. So, I tweaked the rzand 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 commented 2 years ago

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 commented 2 years ago

@DevelopDaily can you check this now please?

DevelopDaily commented 2 years ago

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

DevelopDaily commented 2 years ago

The staqfails 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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

vsoftco commented 2 years ago

closing here, and leaving it open on staq https://github.com/softwareQinc/staq/issues/45