-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Enable testing of quimb-dependent notebook #6461
Conversation
Temporary change to be reverted before the merge.
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Positively confirmed that Contract-a-Grid-Circuit.ipynb is CI tested - https://github.com/quantumlib/Cirq/actions/runs/7894131175/job/21544168715?pr=6461#step:5:272 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6461 +/- ##
==========================================
- Coverage 97.82% 97.81% -0.01%
==========================================
Files 1115 1115
Lines 97448 97448
==========================================
- Hits 95327 95323 -4
- Misses 2121 2125 +4 ☔ View full report in Codecov by Sentry. |
Check if the notebooks-stable CI test is failing there. Also list all installed packages in the notebook environment.
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.
This is still a draft, but it LGTM anyway.
@@ -402,7 +404,7 @@ | |||
"metadata": {}, | |||
"outputs": [], | |||
"source": [ | |||
"ccq.tensor_expectation_value(circuit, ZZ)" | |||
"# ccq.tensor_expectation_value(circuit, ZZ)" |
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.
Is this intentional? Why not remove?
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.
This was a temporary change to check if that call causes CI failure in https://github.com/quantumlib/Cirq/actions/runs/7894234524/job/21545039838.
It indeed did, because the CI passed for 1251781.
I have also run the notebook in a stock colab where the last call crashed with out-of-memory.
Testing with a pre-release cirq works, because it has a pinned quimb version.
I am reverting the notebook to its mainline version and excluding it from tests with stable cirq.
Note: CI passes with the last cell commented. A fresh Colab run confirms that it causes out-of-memory for stable cirq. This reverts commit 1251781.
Requires a pinned version of quimb specified after last release. Otherwise the tensor_expectation_value() in the last-cell causes OOM.
Also add the required note text to the notebook.
* Enable Contract-a-Grid-Circuit.ipynb in notebook tests, but test it only with the pre-release Cirq. The notebook requires a pinned version of quimb from quantumlib#6438 otherwise the tensor_expectation_value() call in the last-cell causes out-of-memory error. Fixes quantumlib#6088
Enable Contract-a-Grid-Circuit.ipynb in notebook tests
Fixes #6088