Restructure the minimal examples section [DOC-144] #200

lebrice · 2023-07-05T15:59:46Z

Les autres PR d'exemples minimalistes devraient être rebase sur ces changements (ou bien merge ces commits). Cette PR-là change un peu la structure de la section d'exemples minimalistes:

le nom des fichiers passe de _index.rst --> index.rst
Les exemples ne se trouvent plus tous dans la même page
Améliore l'auto-génération des README.rst pour voir les exemples sur GitHub:
- Ajoute un header dans le fichier README.rst auto-généré pour dire que le contenu ne devrait pas être modifié, et indiquer la source
- Force chaque exemple a contenir un lien vers le code source sur GitHub.
- Fix l'indentation à trois espaces

satyaog

If you don't agree with the suggestions I'll change my review to approved ;)

(you can batch all the changes you would like in a single commit btw)

.gitignore

docs/Minimal_examples.rst

satyaog · 2023-07-12T19:48:51Z

docs/Minimal_examples.rst

+.. toctree::
+    :maxdepth: 1
+
+    examples/frameworks/index
+    examples/distributed/index
+    examples/good_practices/index


Couldn't this go in doc/index.rst instead so we can see all of the sections from the home page?

Interesting idea, I'll try it out.

I'm unable to make this work locally. Perhaps I'm doing something wrong. Did you mean to remove the toctree here instead have something like this in the index.rst?

.. toctree:: :caption: Minimal Examples :maxdepth: 2 Minimal_examples examples/frameworks/index examples/distributed/index examples/good_practices/index

If so, this doesn't seem to work correctly, there is still only "Minimal Examples" under the "MINIMAL EXAMPLES" section in the left navbar.
Perhaps we could check this out together sometime this week if you don't mind? (I'm pretty bad at this)

Oh also: I think that using .. include would make them all end up in the same page, right? (Not sure if that would be part of the potential solution, but I wouldn't want that to happen here with the examples)

Fixed in 0f5ddd0

docs/examples/distributed/multi_gpu/index.rst

docs/examples/good_practices/index.rst

docs/examples/frameworks/index.rst

docs/examples/frameworks/pytorch_setup/README.rst

docs/examples/distributed/index.rst

docs/examples/frameworks/pytorch_setup/index.rst

docs/Minimal_examples.rst

Signed-off-by: Fabrice Normandin <[email protected]>

Co-authored-by: satyaog <[email protected]>

Signed-off-by: Fabrice Normandin <[email protected]>

docs/examples/preprocess.py

Signed-off-by: Fabrice Normandin <[email protected]>

docs/Minimal_examples.rst

Signed-off-by: Fabrice Normandin <[email protected]>

breuleux · 2023-08-01T20:47:15Z

docs/examples/preprocess.py

+                elif line.startswith(".. literalinclude:: ") or not line:
+                    break


This seems a bit brittle as a way to figure out where the block ends, compared to simply checking if the (non-stripped) line starts with a non-space character.

Fixed in 4bc0bae

docs/examples/preprocess.py

breuleux · 2023-08-01T20:55:51Z

docs/examples/generate_diffs.sh

+generate_diff distributed/single_gpu/job.sh distributed/multi_gpu/job.sh
+generate_diff distributed/single_gpu/main.py distributed/multi_gpu/main.py

 # multi_gpu -> multi_node
-generate_diff distributed/002_multi_gpu/job.sh distributed/003_multi_node/job.sh
-generate_diff distributed/002_multi_gpu/main.py distributed/003_multi_node/main.py
+generate_diff distributed/multi_gpu/job.sh distributed/multi_node/job.sh
+generate_diff distributed/multi_gpu/main.py distributed/multi_node/main.py

 # single_gpu -> checkpointing
-generate_diff distributed/001_single_gpu/job.sh good_practices/checkpointing/job.sh
-generate_diff distributed/001_single_gpu/main.py good_practices/checkpointing/main.py
+generate_diff distributed/single_gpu/job.sh good_practices/checkpointing/job.sh
+generate_diff distributed/single_gpu/main.py good_practices/checkpointing/main.py

 # single_gpu -> hpo_with_orion
-generate_diff distributed/001_single_gpu/job.sh good_practices/hpo_with_orion/job.sh
-generate_diff distributed/001_single_gpu/main.py good_practices/hpo_with_orion/main.py
+generate_diff distributed/single_gpu/job.sh good_practices/hpo_with_orion/job.sh
+generate_diff distributed/single_gpu/main.py good_practices/hpo_with_orion/main.py


Could we loop over **/job.sh and **/main.py instead, so we don't have to remember to update this file whenever we add a new example?

Perhaps I don't understand, but here we need to define the from-to relationship between examples. How do you suggest we do that automatically?

Ah yeah I had a brain fart here, sorry.

Although, one way to do this automatically would be to represent the examples as a Git arborescence: you write the code for single_gpu, then you create a multi_gpu branch that has one commit that is the difference, and so on, each example being its own branch. Then you can run a series of checkouts and cp to populate the examples directory in the actual docs repo. With this method you can also update the whole set of examples more easily by changing the base and looping on git rebase. But in any case this is out of scope for this PR.

Yeah that's interesting!

I also played around with the idea of us editing the .diff files directly instead of the example files, and re-generating the derived examples if something changes in the base.

For now, with this limited number example dependencies, it's pretty manageable to make a change in the base example and then copy-paste it to the 5-6 derived examples. However there is potentially a better solution in the long run for sure.
I agree with you, I don't think it's worth it to spend too much time working on that right now.

breuleux · 2023-08-01T20:59:35Z

docs/examples/preprocess.py

+                f"   {_l}".rstrip(" ")
+                for _l in (docs_root / path).read_text().split("\n")
+            ]
+            content_lines = content_lines[:i] + insert + content_lines[i + 1 :]


This drops content_lines[i], is this what we want? We already removed the literalinclude lines.

Fixed in 4bc0bae

Signed-off-by: Fabrice Normandin <[email protected]>

breuleux · 2023-08-03T19:35:39Z

docs/examples/preprocess.py

+        def _get_block_length() -> int:
+            length = 1
+            while (
+                line_index + length < len(content_lines)
+                and content_lines[line_index + length]
+                and not content_lines[line_index + length].isspace()
+            ):
+                length += 1
+            return length


I haven't tested so maybe I'm following the code wrong, but you only stop on empty/whitespace lines, so it seems to me that the function will return 4 on the following block:

.. literalinclude:: x.py :language: python .. important:: Don't run vscode on the login nodes you guys.

And 1 on this one, which I'm not certain is legal or not:

.. literalinclude:: x.py :language: python

Fixed here: 806c59d

Signed-off-by: Fabrice Normandin <[email protected]>

lebrice changed the title ~~Restructuring the minimal examples section~~ Restructure the minimal examples section Jul 5, 2023

lebrice marked this pull request as ready for review July 5, 2023 18:17

lebrice requested review from btravouillon and satyaog as code owners July 5, 2023 18:17

lebrice requested a review from gyom July 5, 2023 18:18

lebrice force-pushed the lebrice/fix_examples_structure branch 2 times, most recently from 93cbb86 to 68e20f3 Compare July 5, 2023 19:16

lebrice mentioned this pull request Jul 6, 2023

Adding WANDB example #199

Merged

lebrice force-pushed the lebrice/fix_examples_structure branch 2 times, most recently from 60ee7db to 1b3fbb7 Compare July 7, 2023 19:11

satyaog requested changes Jul 12, 2023

View reviewed changes

lebrice commented Jul 18, 2023

View reviewed changes

docs/Minimal_examples.rst Outdated Show resolved Hide resolved

lebrice and others added 5 commits July 24, 2023 13:08

Improve structure of the Minimal Examples section

5fbae54

Signed-off-by: Fabrice Normandin <[email protected]>

Replace doc refs with Hyperlinks in GitHub READMEs

945fdb3

Signed-off-by: Fabrice Normandin <[email protected]>

Adapt the checkpointing example to new structure

809e93d

Signed-off-by: Fabrice Normandin <[email protected]>

Apply suggestions from code review

bcc98c3

Co-authored-by: satyaog <[email protected]>

Add changes from pre-commit

f3d940c

Signed-off-by: Fabrice Normandin <[email protected]>

lebrice force-pushed the lebrice/fix_examples_structure branch from 10c220b to f3d940c Compare July 24, 2023 17:08

lebrice added 7 commits July 24, 2023 13:11

Fix the generate_diffs.sh script for Orion

6aca590

Signed-off-by: Fabrice Normandin <[email protected]>

Improve the link to the PyTorch setup ex in Orion

bf26df8

Signed-off-by: Fabrice Normandin <[email protected]>

Fix a comment line in the single-GPU example

328a811

Signed-off-by: Fabrice Normandin <[email protected]>

Simplify a logging line in the Orion script

9a94ac9

Signed-off-by: Fabrice Normandin <[email protected]>

Add missing link to GitHub page in Orion example

f88f8a2

Signed-off-by: Fabrice Normandin <[email protected]>

Remove some empty space in Orion's index.rst

d5561f3

Signed-off-by: Fabrice Normandin <[email protected]>

Fix issues with the Orion index.rst

54c9413

Signed-off-by: Fabrice Normandin <[email protected]>

satyaog reviewed Jul 24, 2023

View reviewed changes

docs/examples/preprocess.py Outdated Show resolved Hide resolved

Reset indent in .rst files to 3 spaces

918355e

Signed-off-by: Fabrice Normandin <[email protected]>

satyaog approved these changes Jul 24, 2023

View reviewed changes

lebrice requested a review from obilaniu July 24, 2023 21:03

lebrice mentioned this pull request Jul 27, 2023

Jax #205

Merged

soline-b reviewed Jul 31, 2023

View reviewed changes

docs/Minimal_examples.rst Outdated Show resolved Hide resolved

lebrice commented Jul 31, 2023

View reviewed changes

docs/Minimal_examples.rst Outdated Show resolved Hide resolved

lebrice and others added 2 commits July 31, 2023 18:10

Apply suggestions from code review

b65f079

Fix line length and other rst tweaks

65d6157

Signed-off-by: Fabrice Normandin <[email protected]>

lebrice changed the title ~~Restructure the minimal examples section~~ Restructure the minimal examples section [DOC-144] Aug 1, 2023

lebrice requested a review from breuleux August 1, 2023 16:00

breuleux reviewed Aug 1, 2023

View reviewed changes

lebrice added 3 commits August 2, 2023 13:13

Add a -v/--verbose option to preprocess.py

0769bfa

Signed-off-by: Fabrice Normandin <[email protected]>

Refactor the inlining code of preprocess.py

4bc0bae

Signed-off-by: Fabrice Normandin <[email protected]>

Remove old comments in multi_gpu and multi_node

cd154ff

Signed-off-by: Fabrice Normandin <[email protected]>

lebrice force-pushed the lebrice/fix_examples_structure branch from 71f5b94 to cd154ff Compare August 2, 2023 18:10

lebrice added 4 commits August 2, 2023 14:13

Fix potential bug in preprocess.py

95fce72

Signed-off-by: Fabrice Normandin <[email protected]>

Remove walrus (:=) because build uses old python

25cee53

Signed-off-by: Fabrice Normandin <[email protected]>

Handle potentially different block lengths

83fcb30

Signed-off-by: Fabrice Normandin <[email protected]>

Shift the pages up one level + add header

0f5ddd0

Signed-off-by: Fabrice Normandin <[email protected]>

breuleux reviewed Aug 3, 2023

View reviewed changes

lebrice added 2 commits August 7, 2023 12:05

Simplify the block length, assume 2

806c59d

Signed-off-by: Fabrice Normandin <[email protected]>

Remove trailing whitespace in header.rst

0e32122

Signed-off-by: Fabrice Normandin <[email protected]>

breuleux approved these changes Aug 7, 2023

View reviewed changes

lebrice merged commit 6c4797b into mila-iqia:master Aug 7, 2023

satyaog added a commit to satyaog/mila-docs that referenced this pull request Aug 16, 2023

Fix for mila-iqia#200

028a5c5

satyaog added a commit to satyaog/mila-docs that referenced this pull request Aug 16, 2023

Fix for mila-iqia#200

c363469

satyaog added a commit to satyaog/mila-docs that referenced this pull request Aug 16, 2023

Fix for mila-iqia#200

1dc28c5

satyaog added a commit to satyaog/mila-docs that referenced this pull request Aug 16, 2023

Fix for mila-iqia#200

2204f79

satyaog added a commit to satyaog/mila-docs that referenced this pull request Aug 16, 2023

Fix for mila-iqia#200

667b0fa

satyaog added a commit to satyaog/mila-docs that referenced this pull request Aug 16, 2023

Fix for mila-iqia#200

3792d9d

satyaog added a commit to satyaog/mila-docs that referenced this pull request Aug 16, 2023

Fix for mila-iqia#200

c159806

satyaog added a commit to satyaog/mila-docs that referenced this pull request Sep 21, 2023

Fix for mila-iqia#200

c3846de

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructure the minimal examples section [DOC-144] #200

Restructure the minimal examples section [DOC-144] #200

lebrice commented Jul 5, 2023 •

edited

Loading

satyaog left a comment •

edited

Loading

satyaog Jul 12, 2023

lebrice Jul 18, 2023

lebrice Jul 31, 2023 •

edited

Loading

lebrice Jul 31, 2023 •

edited

Loading

lebrice Aug 2, 2023

breuleux Aug 1, 2023

lebrice Aug 2, 2023 •

edited

Loading

breuleux Aug 1, 2023

lebrice Aug 2, 2023

breuleux Aug 2, 2023

lebrice Aug 2, 2023

breuleux Aug 1, 2023

lebrice Aug 2, 2023

breuleux Aug 3, 2023

lebrice Aug 7, 2023

		elif line.startswith(".. literalinclude:: ") or not line:
		break

Restructure the minimal examples section [DOC-144] #200

Restructure the minimal examples section [DOC-144] #200

Conversation

lebrice commented Jul 5, 2023 • edited Loading

satyaog left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lebrice Jul 31, 2023 • edited Loading

Choose a reason for hiding this comment

lebrice Jul 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lebrice Aug 2, 2023 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lebrice commented Jul 5, 2023 •

edited

Loading

satyaog left a comment •

edited

Loading

lebrice Jul 31, 2023 •

edited

Loading

lebrice Jul 31, 2023 •

edited

Loading

lebrice Aug 2, 2023 •

edited

Loading