ci: automatically test all notebooks under `examples/notebooks` and render them in docs page #756

wesk · 2023-09-06T12:52:00Z

This PR:

Adds automation to test notebooks with every pull request
Adds automation to render notebooks in our documentation website
Adds a new example notebook

More detail on the three aspects below.

Automatically test notebooks with every pull request

To add a new Jupyter (.ipynb) file to the repository, simply add it to the examples/notebooks folder.

Every notebook in the examples/notebooks folder will be converted to a python script and executed by a GitHub Actions workflow.

If you don't want specific cells in a notebook to be run as part of our automatic tests, tag them with the ci-skip-cell tag, and the CI will not execute the cell. The reason this feature exists is because some cells that are used to install dependencies, such as !pip install xgboost do not convert easily into directly executable scripts. The first code cell in the XGBoost notebook is the motivating example for this feature.

Automatically render notebooks in Syne Tune documentation

The idea with this PR is to keep a single source of truth: examples/notebooks as the location where all example notebooks are kept.

To have your notebook show up in documentation, first add it to the examples/notebooks folder, and then update the following syntax inside the examples_toc.rst page to make sure that your notebook is included in the documentation.

Example showing adding the xgboost notebook:

Examples
========

.. toctree::
   :name: Example Notebooks
   :caption: Example Notebooks
   :maxdepth: 1

   notebooks/xgboost_example.ipynb


.. toctree::
   :name: Example Tuning Scripts
   :caption: Example Tuning Scripts
   :maxdepth: 1

   examples

Before you add a new notebook, be sure to clear the cell outputs, unless you also want them to render in the resulting documentation.

Note that the documentation build process copies the notebooks to the docs/source/notebooks folder. This is because the nbsphinx extension only knows about files inside the docs/source/ folder.

New example notebook: `Tune XGBoost`

This PR adds a new example notebook demonstrating how you can use Syne Tune to tune XGBoost's hyperparameters on the UCI hand-written digits dataset.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2023-09-06T13:00:19Z

Codecov Report

Patch coverage has no change and project coverage change: -0.03% ⚠️

Comparison is base (78fe6ff) 64.23% compared to head (a1ee27c) 64.21%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
- Coverage   64.23%   64.21%   -0.03%     
==========================================
  Files         435      435              
  Lines       29049    29049              
==========================================
- Hits        18661    18654       -7     
- Misses      10388    10395       +7

see 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

geoalgo

It looks good to me overall.

One question would be what is the rationale for having a notebook and not an example?

I think there should be a clear way to say how functionalities are committed otherwise you may have a mix-and-match of notebook/python examples (for instance a bunch of the current examples could be notebooks).

geoalgo · 2023-09-11T13:03:18Z

docs/source/conf.py

+def copy_notebooks_into_docs_folder(app):
+    # .ipynb files must be inside the docs/ folder for Jupyter to be able to convert them
+    currpath = os.path.dirname(os.path.realpath(__file__))
+    source = os.path.join(currpath, "../../examples/notebooks")


Can we use Pathlib parent rather than "../../" which is not portable?

geoalgo · 2023-09-11T13:04:45Z

docs/source/conf.py

+    source = os.path.join(currpath, "../../examples/notebooks")
+    destination = os.path.join(currpath, "notebooks")
+    print(
+        f"current path: {currpath}; source path: {source}; destination path: {destination}"
+    )
+    try:
+        shutil.rmtree(destination)


Do we need this rmtree? It seems potentially dangerous, most of the time we would execute on the remote machine but I could see a case where we would execute locally and may get bad surprise.

Good point, I updated it to shutil.copytree(source, destination, dirs_exist_ok=True), the dirs_exist_ok=True is what I was missing previously.

https://docs.python.org/3/whatsnew/3.8.html#shutil

mseeger

I just have some small comments.

examples/notebooks/tune_xgboost.ipynb

mseeger · 2023-09-11T13:16:32Z

examples/notebooks/tune_xgboost.ipynb

+    "config_space = {\n",
+    "    \"max_depth\": randint(1,10),\n",
+    "    \"gamma\": uniform(1,10),\n",
+    "    \"reg_lambda\": uniform(.0000001, 1),\n",


Use loguniform instead of uniform

mseeger · 2023-09-11T13:17:05Z

examples/notebooks/tune_xgboost.ipynb

+    "    import xgboost\n",
+    "    import numpy as np\n",
+    "\n",
+    "    eval_metric = \"merror\"\n",


Is this needed?

No - removed in next revision

mseeger · 2023-09-11T13:17:26Z

examples/notebooks/tune_xgboost.ipynb

+    "        max_depth=max_depth,\n",
+    "        eval_metric=eval_metric,\n",
+    "    )\n",
+    "    clf.fit(X_train, y_train, eval_set=[(X_val, y_val)])\n",


Is the eval_set parameter needed?

No, it's not required, removed in next revision.

wesk · 2023-09-11T15:42:20Z

It looks good to me overall.

One question would be what is the rationale for having a notebook and not an example?

I think there should be a clear way to say how functionalities are committed otherwise you may have a mix-and-match of notebook/python examples (for instance a bunch of the current examples could be notebooks).

Thanks for the feedback -- I updated CONTRIBUTING.md with some general guidance in the next revision.

I intentionally kept the guidance general - we can update/refine it over time.

Another consideration is that not every example can easily be run today as a notebook.

My hypothesis is that users, especially those who are new to Syne Tune, will appreciate notebook examples, and we may want to consider converting some existing script examples into notebooks over time.

wesk added 2 commits September 5, 2023 13:56

add notebook to docs

b356802

update content path

2986032

wesk added 10 commits September 7, 2023 11:12

lint, update GH workflow, troubleshoot conf.py

d76c9f0

update workflow, lint

e98c3b1

update workflow

eb33723

update GH action to upload/download script

17be484

pin SHA, checkout syne tune

648204d

try updated workflow

1bb3371

test

a094cb6

update workflow with more arguments

d6482f2

try static matrix

7cf1a11

move to dedicated helper script

7ecc2fd

wesk changed the title ~~docs: add notebook~~ ci: automatically test all notebooks under examples/notebooks and render them in docs page Sep 7, 2023

wesk added 4 commits September 7, 2023 15:25

lint

7995bfb

update tag from install-dependencies to ci-skip-cell

a9d19e4

rename example

ef77e55

update artifact name

57efa0e

wesk marked this pull request as ready for review September 7, 2023 15:12

wesk requested review from mseeger and geoalgo September 7, 2023 15:12

geoalgo reviewed Sep 11, 2023

View reviewed changes

mseeger previously approved these changes Sep 11, 2023

View reviewed changes

address comments

7b4a3c2

wesk dismissed mseeger’s stale review via 7b4a3c2 September 11, 2023 14:50

wesk added 2 commits September 11, 2023 17:07

remove param

6b5ad5a

update CONTRIBUTING.md

a1ee27c

wesk requested review from geoalgo and mseeger September 11, 2023 15:50

geoalgo approved these changes Sep 15, 2023

View reviewed changes

wesk merged commit 8546a4a into main Sep 18, 2023
35 checks passed

wesk deleted the 2023-09-05example branch September 18, 2023 23:59

wesk added the documentation Improvements or additions to documentation label Oct 30, 2023

wesk mentioned this pull request Oct 30, 2023

Upgrade from traitlets 5.11.2 to 5.12.0 broke script ipython/traitlets#887

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: automatically test all notebooks under `examples/notebooks` and render them in docs page #756

ci: automatically test all notebooks under `examples/notebooks` and render them in docs page #756

wesk commented Sep 6, 2023 •

edited

codecov bot commented Sep 6, 2023 •

edited

geoalgo left a comment

geoalgo Sep 11, 2023

wesk Sep 11, 2023

geoalgo Sep 11, 2023

wesk Sep 11, 2023

mseeger left a comment

mseeger Sep 11, 2023

wesk Sep 11, 2023

mseeger Sep 11, 2023

wesk Sep 11, 2023

mseeger Sep 11, 2023

wesk Sep 11, 2023

wesk commented Sep 11, 2023

ci: automatically test all notebooks under examples/notebooks and render them in docs page #756

ci: automatically test all notebooks under examples/notebooks and render them in docs page #756

Conversation

wesk commented Sep 6, 2023 • edited

Automatically test notebooks with every pull request

Automatically render notebooks in Syne Tune documentation

New example notebook: Tune XGBoost

codecov bot commented Sep 6, 2023 • edited

Codecov Report

geoalgo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mseeger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wesk commented Sep 11, 2023

ci: automatically test all notebooks under `examples/notebooks` and render them in docs page #756

ci: automatically test all notebooks under `examples/notebooks` and render them in docs page #756

wesk commented Sep 6, 2023 •

edited

New example notebook: `Tune XGBoost`

codecov bot commented Sep 6, 2023 •

edited