Skip to content

Conversation

@dstrain115
Copy link
Collaborator

  • Install cirq pre-release to get new qubit placement
    stuff like hard coded qubit placement/
  • This should hopefully fix docs errors.

- Install cirq pre-release to get new qubit placement
stuff like hard coded qubit placement/
- This should hopefully fix docs errors.
@dstrain115 dstrain115 requested review from a team, cduck, verult, vtomole and wcourtney as code owners May 2, 2022 19:27
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@dstrain115 dstrain115 requested a review from maffoo May 2, 2022 19:27
@CirqBot CirqBot added the Size: XS <10 lines changed label May 2, 2022
@MichaelBroughton MichaelBroughton self-assigned this May 3, 2022
@MichaelBroughton
Copy link
Collaborator

If I open: https://colab.sandbox.google.com/github/quantumlib/Cirq/blob/137fcd7200801e1dbacf55f99085d428fd92ff8a/docs/google/qubit-placement.ipynb

and run from start to finish, the notebook fails. Can we diagnose why and fix it as a part of this PR ?

@tanujkhattar
Copy link
Collaborator

tanujkhattar commented May 3, 2022

It would also be nice if instead of ignoring all /google/ notebooks from testing, we only ignored notebooks that actually depend upon cloud auth.

Also, the "view on Quantum AI" link seems broken: https://quantumai.google/cirq/google/qubit-placement gives 404 error.

@tanujkhattar tanujkhattar self-assigned this May 3, 2022
@dstrain115
Copy link
Collaborator Author

If I open: https://colab.sandbox.google.com/github/quantumlib/Cirq/blob/137fcd7200801e1dbacf55f99085d428fd92ff8a/docs/google/qubit-placement.ipynb

and run from start to finish, the notebook fails. Can we diagnose why and fix it as a part of this PR ?

Done.

@dstrain115
Copy link
Collaborator Author

It would also be nice if instead of ignoring all /google/ notebooks from testing, we only ignored notebooks that actually depend upon cloud auth.

Sure, but that is out of scope for this PR. I will open a new PR for that in a bit.

The view on quantum AI is broken since the notebook does not run, which is what this PR is fixing.

@@ -1,10 +1,9 @@
{
"cells": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line #5.        !pip install --pre --quiet cirq

Let's add the note on top that this notebook depends on unreleased features and add the notebook to https://github.com/quantumlib/Cirq/blob/4ba978807b163c6d1ee51c1dea994c731d6400c5/dev_tools/notebooks/isolated_notebook_test.py#L43 so that we remember to remove the --pre after next release.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. Done.

Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM % nit

@tanujkhattar tanujkhattar merged commit 059dd25 into quantumlib:master May 5, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
* Fix Qubit Placement docs

- Install cirq pre-release to get new qubit placement
stuff like hard coded qubit placement/
- This should hopefully fix docs errors.

* Fix hard coded qubit placement issues.

* Add pre release justification.

* Reorder arguments so that test finds --pre

* Add note about unreleased features.

* Fix extra quote.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: XS <10 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants