-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handling global backend for protocols involving circuits #1076
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1076 +/- ##
==========================================
- Coverage 97.30% 97.29% -0.02%
==========================================
Files 124 123 -1
Lines 9884 9845 -39
==========================================
- Hits 9618 9579 -39
Misses 266 266
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of comments, but in general I agree with the overall idea (since we're now focusing on the platform, assuming the backend to be Qibolab, there is no need to require everywhere to create the entire backend)
src/qibocal/auto/transpile.py
Outdated
"""Qibolab platforms.""" | ||
try: | ||
platforms = list(MetaBackend().list_available()) | ||
except RuntimeError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a comment on this PR, but just a point raised as a consequence.
Maybe we should actually drop this exception: if there is no path, we should already return an empty list.
It should be sufficient to do it here:
https://github.com/qiboteam/qibolab/blob/6b2a7b790e8d58d52be937c2f41b8606459a3bf1/src/qibolab/_core/platform/load.py#L23-L26
This should result in .list_available()
returning an empty list, and .load()
raising a different error.
src/qibocal/auto/transpile.py
Outdated
qubit_maps[i] = list(qubit_map) | ||
if backend.name == "qibolab": | ||
platform_nqubits = backend.platform.nqubits | ||
if platform.name in AVAILABLE_PLATFORMS: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I just forgot, but in which case is this condition supposed to be false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be False when we transpile circuits that are executed on simultation so no transpilation at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be False when we transpile circuits that are executed on simultation so no transpilation at all.
Ah, I see. But in that case there should be no Platform
at all, since it is the qibolab.Platform
.
My advice would be to drop any simulation reference, and assume it is always Qibolab (possibly the emulator, but still Qibolab).
I would reintroduce the option for simulation as part of a larger refactor, taking into account #913. I'd expect there should be other as well not supporting a backend other than Qibolab.
0674c61
to
8ea8342
Compare
I realized that by properly dropping the possibility to run on simultation we don't need many shenaningans involving |
Co-authored-by: Alessandro Candido <[email protected]>
@stavros11 I believe that with this approach we don't need anymore #1063, feel free to have a look and eventually we can just merge this PR if you agree. |
c2d5df9
to
29bc178
Compare
29bc178
to
59f95e3
Compare
Closes #1065, by directly instatiating a local backend in each acquisition function.
Other changes implemented in this PR:
QIBO_PLATFORM
to properly run tests on the cluster