Skip to content

Conversation

@DavidBAEpstein
Copy link
Collaborator

@DavidBAEpstein DavidBAEpstein commented Aug 5, 2022

  • DOC: Edit metadata to make all cells visible to rtd.
  • MAINT: edit raw source of list for jnb-rtd consistency.
  • MAINT: Remove redundant initial ">" and incorrect double quotes. Then edit for clarity.
  • MAINT: change "temp" to "tmp"
  • MAINT: change "tail --lines 1" to "tail -n 1". Change wording around ON_GPU.
  • MAINT: correct typo
  • BUG: Remove pretence that ON_GPU=False is viable/meaningful with current software packages.
  • MAINT: Run Colab with GPU and make requisite changes.
  • MAINT: Change number of WORKERS to 4. Add comment that the notebook crashes if run without Cuda-enabled GPU.

1. Modify instruction about restarting runtime. Recommend using "Run all"
twice.
2. Set ON_GPU = True and improve comment about this.
3. Define WORKERS as 0 or 1 depending on value of ON_GPU---an attempt to
get a version without GPU working.
4. Improve clean up after run (in fact at the start of the next run) by
putting all files downloaded or created into a single tmp directory. This
makes clean up bullet proof against, for example, user interruption of
execution at a random time.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #439 (58e6c93) into develop (b3ace85) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #439   +/-   ##
========================================
  Coverage    98.59%   98.59%           
========================================
  Files           58       58           
  Lines         5784     5784           
  Branches      1047     1047           
========================================
  Hits          5703     5703           
  Misses          69       69           
  Partials        12       12           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

"outputs": [],
"source": [
"ON_GPU = True # Set `ON_GPU=True` if cuda-enabled GPU is availabe,\n",
"ON_GPU = True # Set `ON_GPU` ti `True` if cuda-enabled GPU is available,\n",
Copy link
Member

Choose a reason for hiding this comment

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

ON_GPU to True?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the notebook crashes without GPU, I rewrote this saying that ON_GPU = False is not allowed. Some other notebooks have been written so that they still run successfully, though slowly, without GPU. But that is not the case here.

@DavidBAEpstein
Copy link
Collaborator Author

I have checked the rendering of this jupyter notebook against the rendering provided by readthedocs. These two renderings are as similar as it is reasonable to require. The PR has been in a reasonable state for approval for some time, and I think it should now be approved by the reviewers.

@shaneahmed shaneahmed added the documentation Improvements or additions to documentation label Sep 1, 2022
Copy link
Member

@shaneahmed shaneahmed left a comment

Choose a reason for hiding this comment

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

@shaneahmed shaneahmed merged commit fb7f884 into develop Sep 2, 2022
@shaneahmed shaneahmed deleted the doc-07 branch September 2, 2022 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants