qiboteam / qibolab

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

Group common Qblox modules functions #730

Closed PiergiorgioButtarini closed 4 months ago

PiergiorgioButtarini commented 8 months ago

This PR follows the philosophy established in https://github.com/qiboteam/qibolab/pull/686 where common properties between the three diferent Qblox modules are defined directly in the class ClusterModule (in module.py) and then are inherited by all of them.

TODO:

Test on hardware.

Checklist:

codecov[bot] commented 8 months ago

Codecov Report

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

Comparison is base (e11f5c8) 64.00% compared to head (c72f618) 64.45%.

Files Patch % Lines
src/qibolab/instruments/qblox/module.py 35.55% 29 Missing :warning:
src/qibolab/instruments/qblox/cluster_qrm_rf.py 3.84% 25 Missing :warning:
src/qibolab/instruments/qblox/cluster_qcm_rf.py 4.34% 22 Missing :warning:
src/qibolab/instruments/qblox/cluster_qcm_bb.py 5.26% 18 Missing :warning:
src/qibolab/instruments/qblox/controller.py 11.76% 15 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #730 +/- ## ========================================== + Coverage 64.00% 64.45% +0.45% ========================================== Files 49 49 Lines 5778 5700 -78 ========================================== - Hits 3698 3674 -24 + Misses 2080 2026 -54 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/730/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/730/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `64.45% <16.79%> (+0.45%)` | :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 7 months ago

This is also outdated wrt main. If you know that is working, and the review is just the routine check, please merge/rebase.

Otherwise, make sure that the branch is updated, and you'd like to merge (I'm asking just because it's not very recent).

stavros11 commented 7 months ago

@alecandido I put us as reviewers because I think in the last Qibo meeting @PiergiorgioButtarini said that this is ready to merge and overall is reducing the lines of code so would be good to merge. But indeed let's wait for him to fix the conflict and let us know.

alecandido commented 7 months ago

Yes, I got your point, but there are a few PRs that are aging a bit in Qibolab, so we should be slightly more careful when merging.

codecov-commenter commented 7 months ago

Codecov Report

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

Project coverage is 64.54%. Comparing base (2d48d2a) to head (e24294a). Report is 85 commits behind head on main.

Files Patch % Lines
src/qibolab/instruments/qblox/module.py 38.23% 42 Missing :warning:
src/qibolab/instruments/qblox/cluster_qrm_rf.py 15.00% 17 Missing :warning:
src/qibolab/instruments/qblox/controller.py 11.76% 15 Missing :warning:
src/qibolab/instruments/qblox/cluster_qcm_rf.py 12.50% 14 Missing :warning:
src/qibolab/instruments/qblox/cluster_qcm_bb.py 8.33% 11 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #730 +/- ## ========================================== + Coverage 63.89% 64.54% +0.64% ========================================== Files 49 49 Lines 5792 5697 -95 ========================================== - Hits 3701 3677 -24 + Misses 2091 2020 -71 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/730/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/730/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `64.54% <26.11%> (+0.64%)` | :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 7 months ago

Pylint is failing https://github.com/qiboteam/qibolab/actions/runs/7844876458/job/21408077607?pr=730#step:10:17

alecandido commented 4 months ago

Superseded by #868 and the subsequent rewriting