qiboteam / qibolab

Quantum hardware module and drivers for Qibo.
https://qibo.science
Apache License 2.0
40 stars 10 forks source link

Phase unwrapping #897

Closed andrea-pasquale closed 1 month ago

andrea-pasquale commented 1 month ago

It would be useful for calibration purposes to perform the unwrapping of the phase. I remember that we talked about this already some time ago. If you agree I believe that the unwrapping should be performed directly in qibolab.

alecandido commented 1 month ago

If this is not needed in Qibolab, I'm not sure if I'd place it here...

A general rule of thumb for the separation is to put things where you need them. The drawback of not doing it is to depend more on each other (if ever you'll need an extension in Qibocal, you will have to change again Qibolab, and release a new version) and to make it harder to test.

However, for sure it would be relevant to have it in Qibolab if calibration is not the only case in which is useful, but also other kinds of hardware execution.

Btw, Matlab has a simple description of the algorithm: https://www.mathworks.com/help/dsp/ref/unwrap.html#mw_8069b378-b385-4b9b-8211-3fd603b2465f (interestingly, on the page for the hardware block for it)

andrea-pasquale commented 1 month ago

If this is not needed in Qibolab, I'm not sure if I'd place it here...

You can make the same arguments for several properties in the results objects... I'm just proposing to modify one property, unless now you decided that you want to drop all of the properties that are not needed by Qibolab I don't see why this is a problem. Moreover, the phase not un-wrapped provides almost no information.

alecandido commented 1 month ago

I'm just proposing to modify one property, unless now you decided that you want to drop all of the properties that are not needed by Qibolab I don't see why this is a problem.

I would actually propose that...

But we could even keep it for Qibolab 0.2

Moreover, the phase not un-wrapped provides almost no information.

Well, the wrapped phase is enough to obtain the un-wrapped one. So, arguably there is all the information (then it is not in a very useful format, but it is there...)

andrea-pasquale commented 1 month ago

Actually the phase was unwrapped in #896, we can close this issue.