-
Notifications
You must be signed in to change notification settings - Fork 61
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
Client backends for the remote execution of circuits #1157
Conversation
@BrunoLiegiBastonLiegi thanks for this, however let me suggest to move these backends directly in https://github.com/qiboteam/qibo-cloud-backends, so we avoid mixing the core code with external providers. |
Ah ok, sorry about that, I'll move them. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1157 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 69 69
Lines 10110 10145 +35
=========================================
+ Hits 10110 10145 +35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Despite the parser being the major upgrade, I didn't pay much attention to it, because we should use a better strategy for defining it (either using the official parser or make our own one, with a specification of a subset of the formal language used).
Using regexes is a simple option for a quick parser, but when things grow and last long it becomes quite involved and complex (and thus much harder to review...).
Thanks @alecandido, I agree the parser has to be replaced with something more controllable. If the execution through cloud functionality is not urgent, we could delay this and try to incorporate the use of https://github.com/openqasm/openqasm as we discussed separately. |
It is fine, but even if it's not urgent, I would merge this with the current parser. Just to decouple the two tasks. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1157 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 72 72
Lines 10480 10516 +36
=========================================
+ Hits 10480 10516 +36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@BrunoLiegiBastonLiegi do you prefer to merge this now or wait until qibo-cloud-backens is released? Openqasm could be further upgraded in a separate PR. |
I don't know, the only additional thing this PR is providing is the support to the qasm gate command. So it depends, if that is something that is nice to have without waiting for the cloud backends we could merge this. Otherwise we could wait. |
Ok, lets wait until we test and release the cloud backends. |
Meanwhile, could you please fix tests? |
Ok I added a reference in the |
@BrunoLiegiBastonLiegi any known blocker for this? I believe @MatteoRobbiati could be busy, so I'm adding @stavros11 for the review (and @scarrazza in cc). |
✅ great to see you again! |
Co-authored-by: Alessandro Candido <[email protected]>
Co-authored-by: Alessandro Candido <[email protected]>
…dded optional cloud group in poetry
Co-authored-by: Stefano Carrazza <[email protected]>
17cd92d
to
7d8849b
Compare
I rebased on Whenever tests are passing, I'd merge this @BrunoLiegiBastonLiegi |
This PR add support for the
QiboClientBackend
and aQiskitClientBackend
implemented in https://github.com/qiboteam/qibo-cloud-backends that allow for the remote execution of circuits. Furthermore, support for QASMgate
command is added to the parser.Checklist: