qiboteam / qibolab

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

Remove folder argument from platform `create` #784

Closed stavros11 closed 4 months ago

stavros11 commented 6 months ago

Following the discussion in https://github.com/qiboteam/qibolab_platforms_qrc/pull/108#discussion_r1467390932.

I believe some lines in qibo (and maybe also qibocal) need to be updated.

Checklist:

stavros11 commented 6 months ago

Thanks, the only issue is that doctest requires qibo to be updated, because qibo.set_backend is currently using the removed path argument.

andrea-pasquale commented 6 months ago

Thanks, the only issue is that doctest requires qibo to be updated, because qibo.set_backend is currently using the removed path argument.

Ahhh, too many repositories.

andrea-pasquale commented 6 months ago

Btw, shall we release qibolab now that the milestone is complete? @scarrazza @stavros11 @alecandido I know that maybe some changes were not tested on hardware but I will be able to merge some PRs in qibocal as soon as we release qibolab.

alecandido commented 6 months ago

Btw, shall we release qibolab now that the milestone is complete? @scarrazza @stavros11 @alecandido I know that maybe some changes were not tested on hardware but I will be able to merge some PRs in qibocal as soon as we release qibolab.

Fine by me :)

scarrazza commented 6 months ago

@andrea-pasquale fine by me too.

stavros11 commented 4 months ago

@andrea-pasquale @alecandido I updated qibo to 0.2.6. As soon as CI passes, I will merge this.

alecandido commented 4 months ago

@andrea-pasquale @alecandido I updated qibo to 0.2.6. As soon as CI passes, I will merge this.

 = 46 failed, 2198 passed, 104 skipped, 47 deselected, 3 xfailed, 3 xpassed, 5 warnings in 18.20s =

bad luck...

stavros11 commented 4 months ago

@alecandido in 2603611562e45f05a59e9cd08f9e541e0c325266 I dropped the default transpiler from QibolabBackend. This is a significant change because it will no longer be possible to run circuits that contain non-native gates out-of-the box, including through the openqasm, C and Rust APIs (btw I also pushed a fix for these tests but for some reason github doesn't show it).

The reason for dropping is that otherwise we are testing the transpiler in qibolab tests while the code is in qibo (which leads to the failures above). It is still possible to add a transpiler of choice manually:

backend = QibolabBackend("my_platform")
backend.transpiler = ... # my transpiler imported from qibo

or even better the user transpiles their circuit manually:

backend = QibolabBackend("my_platform")

circuit = Circuit(...)
...

native_circuit = my_transpiler(circuit)

result = backend.execute_circuit(native_circuit)
stavros11 commented 4 months ago

After https://github.com/qiboteam/qibolab/actions/runs/8360122834/job/22885054536?pr=784 I think I should officially give up

Cannot install setuptools.
codecov[bot] commented 4 months ago

Codecov Report

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

Project coverage is 66.57%. Comparing base (535dcdb) to head (21369b5).

Files Patch % Lines
src/qibolab/backends.py 85.71% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #784 +/- ## ======================================= Coverage 66.56% 66.57% ======================================= Files 50 50 Lines 6063 6061 -2 ======================================= - Hits 4036 4035 -1 + Misses 2027 2026 -1 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/784/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/784/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `66.57% <90.90%> (+<0.01%)` | :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 4 months ago

Yes, please, just merge.

All in all, Qibolab is the low-level layer, it is fine to require circuits to be transpiled by someone else (i.e. Qibo).