qiboteam / qibolab

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

Split software (de)modulation #786

Closed alecandido closed 5 months ago

alecandido commented 6 months ago

Software (de)modulation was currently entangled with the PulseShape class, and often repeated in the drivers. The goal is just to simplify the presence of software modulation, for the cases in which is still used.

codecov[bot] commented 5 months ago

Codecov Report

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

Comparison is base (b692e60) 64.26% compared to head (fb0ef12) 64.43%.

Files Patch % Lines
src/qibolab/instruments/qblox/sequencer.py 28.57% 5 Missing :warning:
src/qibolab/instruments/qblox/acquisition.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## 0.2 #786 +/- ## ========================================== + Coverage 64.26% 64.43% +0.16% ========================================== Files 54 54 Lines 5835 5814 -21 ========================================== - Hits 3750 3746 -4 + Misses 2085 2068 -17 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/786/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/786/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `64.43% <85.00%> (+0.16%)` | :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 5 months ago

@hay-k I should have addressed all your comments, and the branch has been rebased on the current 0.2.

If you agree, I'd merge the branch (formal approvals are not required, since it's not targeting main, but I preferred to tell you, since it will be hard to review within 0.2).

alecandido commented 5 months ago

Thanks @stavros11 and @hay-k for the review!