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

Update start notebook to use get_engine instead of get_qcs_objects_for_notebook #6483

Merged
merged 9 commits into from
Mar 5, 2024

Conversation

senecameeks
Copy link
Collaborator

@senecameeks senecameeks commented Feb 29, 2024

Addresses #6427 by removing the incorrect processor_id in the notebook.

Although this doesn't fix the underlying issue as to why get_qcs_objects_for_notebook is incorrect and doesn't address Will's second point in his comment

FYI, I verified this works as a good replacement for get_qcs_objects_for_notebook

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CirqBot CirqBot added the size: S 10< lines changed <50 label Feb 29, 2024
Copy link
Collaborator

@eliottrosenberg eliottrosenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link

codecov bot commented Feb 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.76%. Comparing base (d75d43f) to head (02d998b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6483   +/-   ##
=======================================
  Coverage   97.76%   97.76%           
=======================================
  Files        1105     1105           
  Lines       94936    94963   +27     
=======================================
+ Hits        92810    92837   +27     
  Misses       2126     2126           

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

@NoureldinYosri
Copy link
Collaborator

NoureldinYosri commented Feb 29, 2024

this might not work if it's run from outside corp network in which case authentication is needed which is part of get_qcs_objects_for_notebook

a = auth.authenticate_user(clear_output=False)


to check if it will from outside corp network try to run then notebook in colab.research

@senecameeks
Copy link
Collaborator Author

Thanks @NoureldinYosri for the catch!

#6485 exposes authenticate_user, which I've added to this start notebook.
In the case where get_engine throws an exception, it will default to a virtual engine instance using get_qcs_notebook_objects. Note that because virtual is set to True, it does not print the erroneous message Using processor: which was the impetus for this change (See #6427).

@senecameeks senecameeks enabled auto-merge (squash) March 5, 2024 19:20
@senecameeks senecameeks disabled auto-merge March 5, 2024 19:53
@senecameeks senecameeks enabled auto-merge (squash) March 5, 2024 19:53
Copy link
Collaborator

@wcourtney wcourtney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@senecameeks senecameeks merged commit 4513892 into quantumlib:main Mar 5, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: S 10< lines changed <50
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants