Update demo notebook dependencies and initialization cells - #531
Update demo notebook dependencies and initialization cells#531zitterbewegung wants to merge 22 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #531 +/- ##
========================================
Coverage 87.06% 87.06%
========================================
Files 88 88
Lines 7106 7106
========================================
Hits 6187 6187
Misses 919 919
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thanks for working on Colab/demo notebook consistency. Please address the following issues before merging.
-
Please fix the CI test failures. When implementing these fixes, it is crucial that the changes injected into the notebooks remain minimal and consistent with the latest
developbranch content. For example, the content indemos/elliptic-pde.ipynbcurrently differs from thedevelopbranch, beyond the cell you added to importgoogle.colab. Leave the other cells (code, markdown, or outputs) unchanged. -
Please correct the typo "dependancies" found in the comments.
-
Please test updated notebooks in Google Colab.
a) I have not been able to open for example
gbm_demo.ipynbin Colab by clicking the "Open in Colab" button in my local notebooks. Please check and make sure the buttons work.b) The notebook
elliptic-pde.ipynbopens successfully via the "Open in Colab" button, but it runs with errors.
There was a problem hiding this comment.
I will restructiure this branch to reflect the new version of develop I have been working on with codex.
- CI tests are passing
- Fix Typo
- Correct updated notebooks in google colab
There was a problem hiding this comment.
Thank you for your continued work on this task.
I have three follow-up questions regarding the implementation and scope:
- Is the redirection
>> /dev/nulltruly necessary in the installation cells and is it a good practice to hide installation error message? - Is "import os" necessary?
- I noticed that
gbm_demo.ipynb,elliptic-pde.ipynb, andiris.ipynbstill have run-time errors in Colab. Could you ensure these notebooks and others are Colab-ready? - Given that the
demosdirectory contains over 40 notebooks, and this PR has covered 15, is there a way we could create a task (perhaps inmakefile) to ensure Colab-notebook consistency across the entire suite, which is growing in number over time?
I appreciate your help and thought in making this process robust!
There was a problem hiding this comment.
- Piping to dev null suppresses all output of installation. If there is an error message we should catch that before we release that
- import os will be removed
- Pending
- We should make a template instead of a makefile. The notebooks would have to be tested on google colab.
|
Following up on our review meeting yesterday, I have added scripts to improve the notebooks' compatibility with Colab. Please review and test these scripts in both the local development environment and the Colab platform. |
|
The information previously in this comment has been moved to the Jupyter notebook QA document see the second sheet of the following https://docs.google.com/spreadsheets/d/17KJoNhVyJBGuWoBqz1oujnAvuUm_8CrJ/edit?pli=1&gid=830782523#gid=830782523 |
|
@zitterbewegung I don't follow the checklist you made on Jun 4. Kindly elaborate. |
|
The information had been moved to the QA notebook spreadsheet we were using. See https://drive.google.com/drive/u/0/mobile/recent?dmr=1&ec=wgc-drive-%5Bmodule%5D-goto |
|
What is the status of this PR? |
2 participants
This PR updates demo notebook dependency/install and initialization cells to improve notebook execution consistency. Two notebooks were removed due to different locations.
Updated notebooks:
Supersedes PR #384.