Abstract out auth+device+sampler boilerplate from cirq_google tutorials #4286
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Copies the boilerplate code from cirq-google tutorial notebooks and adds that as a function in library.
The imports for cirq_google package needed to be changed now that the boilerplate code is no longer in an external notebook, so fixed that.
Fixing the `get_device_sampler` function, making the part that gets the device and sampler optional so that the tutorials that use the engine can avoid it. Complete type annotations and docstring for the function.
The tutorial boilerplate has been abstracted out, so replaced that code in the notebooks with function calls. These notebooks haven't been tested yet.
AnimeshSinha1309
requested review from
cduck,
vtomole,
wcourtney and
a team
as code owners
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Imported Tuple from typing, and fixed missing imports with TYPE_CHECKING.
Ran the autoformatter, added the cirq_google imports (sometimes as duplicate) and corrected the name of the function call.
Lint check should pass now. Type check just has 2 modules (IPython and google.colab) with no stubs. Coverage has the G-Cloud signin lines with no coverage.
|
The code that was copied from the tutorials includes Unable to figure out a solution to these problems, requesting help on them. |
|
Also, I haven't run the notebooks, they will need to be tested before this is merged. I don't think whether or not the notebooks work in all cases gets tested in the CI pipeline. Testing them with GCloud signin will be hard since I don't have an account. I have tested the call to the local noisy simulator. |
|
You should be able to modify the mypy.ini configuration file to ignore 3rd party libraries that lack stubs. Please check that file for examples of how we do it for other dependencies. Maybe @wcourtney could provide some guidance on how to test google auth functions, but at the very worst you can add |
mpharrigan
reviewed
Jul 8, 2021
Mypy issue fixed (library stubs), Google Auth testing ignored with pragmas, function return types cleaned up.
The function prototype for auth changed, changing the same in notebooks.
Removing the option of not getting the device broke stuff in tests, fixing it.
Ran the formatter.
AnimeshSinha1309
force-pushed
the
issue/example-template
branch
from
1ce38d9
to
d55876d
Compare
Had missed a not in the sampler test.
Trivial change to trigger tests, packaging had failed due to network error.
|
I am again done on my end. I have fixed the errors, factored out the code, and I really hope I haven't caused new ones. |
mpharrigan
requested changes
Aug 30, 2021
Now the property is is_simulator, as asked for in the review.
Ran the autoformatter again.
mpharrigan
requested changes
Sep 13, 2021
Fixed the inverted logic in the Floquet calibration notebook.
mpharrigan
approved these changes
Sep 14, 2021
mpharrigan
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
MichaelBroughton
added a commit
that referenced
this pull request
Sep 15, 2021
#4286 Broke our nightly build of https://quantumai.google/cirq/tutorials/google/floquet_calibration_example Without the `--pre` flag this tutorial wouldn't pull in the latest changes and broke. Added the `--pre` to fix this.
CirqBot
pushed a commit
that referenced
this pull request
Sep 15, 2021
https://quantumai.google/cirq/tutorials/google/floquet_calibration_example broke after #4286 (because it was missing a `--pre` flag). Without the `--pre` flag this tutorial wouldn't pull in the latest changes and broke. Added the `--pre` to fix this.
CirqBot
pushed a commit
that referenced
this pull request
Sep 17, 2021
#4286 introduced a new module (`qcs_notebook`) and utility method (`get_qcs_objects_for_notebook`) to abstract out boilerplate code for notebooks. However, all notebooks using the new method now depend on the pre-released version and should `pip install --pre cirq` instead of `pip install cirq` to work correctly. Right now, all these notebooks are broken and this PR fixes it. Note: In some notebooks, already existing `--pre` and `--quiet` is reordered because of recent tests which assert that pre released notebooks have the pip install statement: https://github.com/quantumlib/Cirq/blob/6ecaf0793c0d0865b09603e50fcfbc11ce567273/dev_tools/notebooks/isolated_notebook_test.py#L206 Even though vendor notebooks (`"**/google/*.ipynb",`) are skipped from testing against cirq, I think it's good to add them to the `NOTEBOOKS_DEPENDING_ON_UNRELEASED_FEATURES` so that `--pre` can be removed in the next release. cc @AnimeshSinha1309 for future reference.
rht
pushed a commit
to rht/Cirq
that referenced
this pull request
May 1, 2023
…ls (quantumlib#4286) Code from the tutorial notebooks has been abstracted out into a function in the `cirq_google` library. The function goes by the name `cirq_google.engine.engine_sampler.get_device_sampler()` All the notebooks are being refactored to use this abstraction. Closes quantumlib#4260.
rht
pushed a commit
to rht/Cirq
that referenced
this pull request
May 1, 2023
https://quantumai.google/cirq/tutorials/google/floquet_calibration_example broke after quantumlib#4286 (because it was missing a `--pre` flag). Without the `--pre` flag this tutorial wouldn't pull in the latest changes and broke. Added the `--pre` to fix this.
rht
pushed a commit
to rht/Cirq
that referenced
this pull request
May 1, 2023
quantumlib#4286 introduced a new module (`qcs_notebook`) and utility method (`get_qcs_objects_for_notebook`) to abstract out boilerplate code for notebooks. However, all notebooks using the new method now depend on the pre-released version and should `pip install --pre cirq` instead of `pip install cirq` to work correctly. Right now, all these notebooks are broken and this PR fixes it. Note: In some notebooks, already existing `--pre` and `--quiet` is reordered because of recent tests which assert that pre released notebooks have the pip install statement: https://github.com/quantumlib/Cirq/blob/6ecaf0793c0d0865b09603e50fcfbc11ce567273/dev_tools/notebooks/isolated_notebook_test.py#L206 Even though vendor notebooks (`"**/google/*.ipynb",`) are skipped from testing against cirq, I think it's good to add them to the `NOTEBOOKS_DEPENDING_ON_UNRELEASED_FEATURES` so that `--pre` can be removed in the next release. cc @AnimeshSinha1309 for future reference.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Code from the tutorial notebooks has been abstracted out into a function in the
cirq_googlelibrary.The function goes by the name
cirq_google.engine.engine_sampler.get_device_sampler()All the notebooks are being refactored to use this abstraction.
Closes #4260.