qiboteam / qibolab

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

Fix for TOF routine on QM #898

Closed stavros11 closed 1 month ago

stavros11 commented 1 month ago

I think this is needed to see the time of flight.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 66.60%. Comparing base (49632a5) to head (5d792e2).

Files Patch % Lines
src/qibolab/instruments/qm/acquisition.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #898 +/- ## ========================================== - Coverage 66.61% 66.60% -0.02% ========================================== Files 55 55 Lines 5943 5944 +1 ========================================== Hits 3959 3959 - Misses 1984 1985 +1 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/898/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/qiboteam/qibolab/pull/898/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `66.60% <0.00%> (-0.02%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

stavros11 commented 1 month ago

This should be ready, I think it is fixing the time of flight routine. I don't think I can fix the coverage because it is qua code which I am not yet sure how to execute in the CI.

stavros11 commented 1 month ago

Just to understand: why do you need an explicit phase reset?

Thanks. I took it from here https://github.com/qua-platform/qua-libs/blob/066d7c7588419f84cf8c3a20a47ac2696ac52257/Quantum-Control-Applications/Superconducting/Single-Fixed-Transmon/02_raw_adc_traces.py#L29 and there is a brief explanation on the comment above. If you search for reset_phase in the same repository, it seems that it appears in all experiments that are averaging raw acquisitions. The explanation is always that without it the signal would average to zero (I am guessing due to averaging random phases?). However, I am not sure why this not needed when integrating (not using the raw signal).

stavros11 commented 1 month ago

Just to understand: why do you need an explicit phase reset?

Thanks. I took it from here https://github.com/qua-platform/qua-libs/blob/066d7c7588419f84cf8c3a20a47ac2696ac52257/Quantum-Control-Applications/Superconducting/Single-Fixed-Transmon/02_raw_adc_traces.py#L29 and there is a brief explanation on the comment above. If you search for reset_phase in the same repository, it seems that it appears in all experiments that are averaging raw acquisitions. The explanation is always that without it the signal would average to zero (I am guessing due to averaging random phases?). However, I am not sure why this not needed when integrating (not using the raw signal).

About integration, maybe the phase is cancelled because we are also demodulating?

alecandido commented 1 month ago

I get that the bare reading would be a two-dimensional array, sample and time. Since you average over sample, you want to establish a common reference, otherwise you would get a random average (possibly zero) for the first entry of each given sample.

However, if the variation is consistent over time, the reset_phase would be just a constant subtraction (constant over time, subtraction because you remove as much as you need to fix the initial value), and the random part related to the initial phase should average to a constant (possibly zero), leaving the time-dependence invariant (up to a constant shift).

I'm reasoning about the acquired value, though the phase being set is the one of the readout pulse. For sure I'm missing something...

alecandido commented 1 month ago

About integration, maybe the phase is cancelled because we are also demodulating?

Demodulation should subtract a frequency, so only a time-dependent phase, not the initial value (you could consider as if the initial value of the modulating wave has always a 0 phase, which most likely is true...). I'm definitely missing something...