qiboteam / qibolab

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

Do not rearrange real-time sweepers (ZI) #852

Closed hay-k closed 3 months ago

hay-k commented 3 months ago

Currently, the ZI driver may change the order of real-time sweepers in certain situations. This was most probably historically done for the following two reasons:

  1. hardware limitation(s)
  2. optimization of runtime (frequency sweep is considered to be slow, so it is moved to the outer loop, so it changes the least amount of time)

Recently, we discovered that (and confirmed by laboneq developers), that point 1 does not exist. As for point 2, with @Jacfomg we observed that order of real-time sweeps may affect the quality of acquired data (for physical reasons, and not specific to instrument). In light of this it seems that the better decision is that the driver not change the order of sweeps. It is better to let qibocal decide which order works best for which experiment.

Below is output from qubit flux dependence experiment, where flux sweep is done as an amplitude sweep for a long square flux pulse. So we have nested 2 real-time sweeps (flux pulse amplitude and qubit drive frequency). The first plot corresponds to the case where frequency sweep is the inner loop, and for the second plot the frequency sweep is the outer loop.

image image

Looks like the rapid changes in amplitude (when the amplitude sweep is the inner one) result into some vertical shadows. The two experiments above were done using relaxation_time = 2000. Increasing it mitigates the shadowing issue, as seen from the below plot done with relaxation_time=20_000:

image

Checklist:

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 66.66%. Comparing base (c109c51) to head (061aa18).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #852 +/- ## ========================================== + Coverage 66.60% 66.66% +0.05% ========================================== Files 55 55 Lines 5917 5900 -17 ========================================== - Hits 3941 3933 -8 + Misses 1976 1967 -9 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/852/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/852/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `66.66% <ø> (+0.05%)` | :arrow_up: | 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.

alecandido commented 3 months ago

Looks like the rapid changes in amplitude (when the amplitude sweep is the inner one) result into some vertical shadows.

I can understand that this is also solved by rearranging sweepers, but if there were other reasons to keep the second order, it should be also solved by a sufficient relaxation time. Isn't it?

hay-k commented 3 months ago

should be also solved by a sufficient relaxation time. Isn't it?

Yes!

EDIT: added a plot related to that to the main description of the PR for completeness.

Jacfomg commented 3 months ago

@hay-k rebased with the flux pulse hotfix so the issue I found is gone and we can merge it.