qiboteam / qibolab

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

Splitting coupler_flux sequence into multiple sub sections #723

Closed GabrielePalazzo closed 9 months ago

GabrielePalazzo commented 10 months ago

Closes #722.

Checklist:

codecov[bot] commented 10 months ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (56bf837) 62.58% compared to head (545a865) 62.57%. Report is 79 commits behind head on main.

Files Patch % Lines
src/qibolab/instruments/zhinst.py 94.33% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #723 +/- ## ========================================== - Coverage 62.58% 62.57% -0.02% ========================================== Files 48 48 Lines 6644 6642 -2 ========================================== - Hits 4158 4156 -2 Misses 2486 2486 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/723/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/723/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `62.57% <94.33%> (-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.

hay-k commented 10 months ago

In general, there is a lot of duplication of code in the ZI driver, and this is another example of it. E.g. similar logic for handling subsequences and delays between pulses exists for qubit drive and qubit flux as well. This is not your fault and should not be addressed in this PR. Everything else is done like this in the driver so to fix the bug you are kind of forced to do it like this as well, because otherwise you will need to rewrite half of the driver :D. I just wanted to point this out and emphasize that we need to address it ASAP. I will be taking care of that, so no action point for you, just FYI.