Skip to content
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

Polish Qibo cloud #1241

Merged
merged 3 commits into from
Mar 1, 2024
Merged

Polish Qibo cloud #1241

merged 3 commits into from
Mar 1, 2024

Conversation

alecandido
Copy link
Member

@alecandido alecandido commented Mar 1, 2024

Sorry @BrunoLiegiBastonLiegi, there is some aftermath after closing #1157, I didn't notice during the review.

  • rename backend "qibo-client"
  • drop the qibo-cloud-backends dependency

Closes #1240

@alecandido
Copy link
Member Author

The motivation for the rename is that qibo-cloud is the global client supporting remote execution, but on multiple backends, while the Qibo server is really qibo-client, so that's the backend.

(you can think as qibo-cloud-qiskit and qibo-cloud-qibo-client, with the initial qibo-cloud- omitted)

Sorry not to have pointed out when we discussed the naming in #1157

@BrunoLiegiBastonLiegi
Copy link
Contributor

The motivation for the rename is that qibo-cloud is the global client supporting remote execution, but on multiple backends, while the Qibo server is really qibo-client, so that's the backend.

(you can think as qibo-cloud-qiskit and qibo-cloud-qibo-client, with the initial qibo-cloud- omitted)

Sorry not to have pointed out when we discussed the naming in #1157

No, indeed, I was planning of changing it in the MetaBackend PR anyway.

@alecandido
Copy link
Member Author

The rationale for the dependency removal is instead the same of qibojit: otherwise it would be circular.

Currently (before qibo-core will land), the dependency on qibo by external backends has to be explicit. Then, qibo is not going to depend on them.
Following the discussion in #822 most likely the explicit dependency on the external backends won't happen even afterward, but qibo will automatically discover dependencies (scoped import) when they are installed.

@alecandido alecandido requested a review from scarrazza March 1, 2024 11:04
@alecandido alecandido marked this pull request as ready for review March 1, 2024 11:04
Copy link

codecov bot commented Mar 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (0912a3a) to head (eff6478).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1241   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           73        73           
  Lines        10531     10531           
=========================================
  Hits         10531     10531           
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@scarrazza
Copy link
Member

@alecandido thanks. At the end you are not moving the cloud-backends dependency to tests, right?

@alecandido
Copy link
Member Author

alecandido commented Mar 1, 2024

@alecandido thanks. At the end you are not moving the cloud-backends dependency to tests, right?

I scan through it, and it seems the two backends are not currently used in tests, thus there is no need for the dependency.

Can you confirm this @BrunoLiegiBastonLiegi?

@BrunoLiegiBastonLiegi
Copy link
Contributor

I scan through it, and it seems the two backends are not currently used in tests, thus there is no need for the dependency

Yes I can confirm, their tests are in qiboteam/qibo-cloud-backends indeed

@scarrazza
Copy link
Member

Ok, thanks, could you please fix the conflict and merge?

@alecandido
Copy link
Member Author

alecandido commented Mar 1, 2024

Ok, thanks, could you please fix the conflict and merge?

Conflicts just fixed, I'm just waiting for the tests to pass

@scarrazza scarrazza added this pull request to the merge queue Mar 1, 2024
Merged via the queue into master with commit c01fb24 Mar 1, 2024
18 of 21 checks passed
@alecandido alecandido deleted the polish-qibo-cloud branch March 1, 2024 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change qibo-cloud with qibo-client
3 participants