Skip to content

Commit

Permalink
Improvements to the documentation (#42)
Browse files Browse the repository at this point in the history
* Minor tweaks to algorithm_test.py

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

* Reorder fixtures and add docstrings

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

* Consolidate test stuff in a single `conftest.py`

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

* Reorganize the documentation files

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

* Add more stuff to the `testing.md` page

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

* Remove dynamic configs, use config files

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

* Use a decorator to parametrize algorithm_config

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

* Add a step in the CI to check that docs can build

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

* Add mermaid diagram to conftest, fixup docs

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

* Add a 'cards' panel in index.md

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

* Tweak icon

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

* Revert changes to the build.yml file (??)

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

* Tweak the fixture order in conftest.py

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

* Greatly simplify the generate_reference_docs.py

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

* Added automatic light/dark mode control

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

* Improve docstring of TestExampleAlgo

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

* Add a step to check docs in build.yml

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

* Remove unneeded strategy matrix for check_docs

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

* Add docstrings / tweak fixture diagram

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

* Various tiny tweaks, change instantiate_datamodule

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

---------

Signed-off-by: Fabrice Normandin <[email protected]>
  • Loading branch information
lebrice authored Aug 27, 2024
1 parent 3e1e522 commit 422ddba
Show file tree
Hide file tree
Showing 42 changed files with 1,317 additions and 1,120 deletions.
167 changes: 91 additions & 76 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ on:
branches:
- master
pull_request:
paths-ignore:
- 'docs/**'

permissions:
contents: read
Expand All @@ -28,89 +26,106 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-python@v4
with:
python-version: '3.10'
python-version: "3.10"
- run: pip install pre-commit
- run: pre-commit --version
- run: pre-commit install
- run: pre-commit run --all-files

check_docs:
needs: [linting]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Setup Rye with caching of venvs
uses: eifinger/setup-rye@v4
id: setup-rye
with:
enable-cache: true
github-token: ${{ secrets.GITHUB_TOKEN }}
cache-prefix: "3.10"
- name: Install dependencies
run: rye sync --no-lock --features=docs
- name: Build the documentation (strict mode)
run: rye run mkdocs build --strict

unit_tests:
needs: [linting]
runs-on: ${{ matrix.platform }}
strategy:
max-parallel: 4
matrix:
platform: [ubuntu-latest]
python-version: ['3.10']
python-version: ["3.10"]
steps:
- uses: actions/checkout@v4
- name: Setup Rye with caching of venvs
uses: eifinger/setup-rye@v4
id: setup-rye
with:
enable-cache: true
github-token: ${{ secrets.GITHUB_TOKEN }}
cache-prefix: ${{ matrix.python-version }}
- name: Pin python-version ${{ matrix.python-version }}
run: rye pin ${{ matrix.python-version }}
- name: Install dependencies
run: rye sync --no-lock
- name: Test with pytest (very fast)
env:
JAX_PLATFORMS: cpu
run: rye run pytest -v --shorter-than=1.0 --cov=project --cov-report=xml --cov-append --skip-if-files-missing
- name: Test with pytest (fast)
env:
JAX_PLATFORMS: cpu
run: rye run pytest -v --cov=project --cov-report=xml --cov-append --skip-if-files-missing

- name: Store coverage report as an artifact
uses: actions/upload-artifact@v4
with:
name: coverage-reports-unit-tests-${{ matrix.platform }}-${{ matrix.python-version }}
path: ./coverage.xml
- uses: actions/checkout@v4
- name: Setup Rye with caching of venvs
uses: eifinger/setup-rye@v4
id: setup-rye
with:
enable-cache: true
github-token: ${{ secrets.GITHUB_TOKEN }}
cache-prefix: ${{ matrix.python-version }}
- name: Pin python-version ${{ matrix.python-version }}
run: rye pin ${{ matrix.python-version }}
- name: Install dependencies
run: rye sync --no-lock
- name: Test with pytest (very fast)
env:
JAX_PLATFORMS: cpu
run: rye run pytest -v --shorter-than=1.0 --cov=project --cov-report=xml --cov-append --skip-if-files-missing
- name: Test with pytest (fast)
env:
JAX_PLATFORMS: cpu
run: rye run pytest -v --cov=project --cov-report=xml --cov-append --skip-if-files-missing

- name: Store coverage report as an artifact
uses: actions/upload-artifact@v4
with:
name: coverage-reports-unit-tests-${{ matrix.platform }}-${{ matrix.python-version }}
path: ./coverage.xml

local_integration_tests:
needs: [unit_tests]
needs: [unit_tests, check_docs]
runs-on: self-hosted
strategy:
max-parallel: 1
matrix:
python-version: ['3.10']
python-version: ["3.10"]
steps:
- uses: actions/checkout@v4
- name: Setup Rye with caching of venvs
uses: eifinger/setup-rye@v4
id: setup-rye
with:
enable-cache: true
github-token: ${{ secrets.GITHUB_TOKEN }}
cache-prefix: ${{ matrix.python-version }}
- name: Pin python-version ${{ matrix.python-version }}
run: rye pin ${{ matrix.python-version }}

- name: Install dependencies
run: rye sync --no-lock

- name: Test with pytest
run: rye run pytest -v --cov=project --cov-report=xml --cov-append --skip-if-files-missing
# TODO: this is taking too long to run, and is failing consistently. Need to debug this before making it part of the CI again.
# - name: Test with pytest (only slow tests)
# run: pdm run pytest -v -m slow --slow --cov=project --cov-report=xml --cov-append

- name: Store coverage report as an artifact
uses: actions/upload-artifact@v4
with:
name: coverage-reports-integration-tests-${{ matrix.python-version }}
path: ./coverage.xml
- uses: actions/checkout@v4
- name: Setup Rye with caching of venvs
uses: eifinger/setup-rye@v4
id: setup-rye
with:
enable-cache: true
github-token: ${{ secrets.GITHUB_TOKEN }}
cache-prefix: ${{ matrix.python-version }}
- name: Pin python-version ${{ matrix.python-version }}
run: rye pin ${{ matrix.python-version }}

- name: Install dependencies
run: rye sync --no-lock

- name: Test with pytest
run: rye run pytest -v --cov=project --cov-report=xml --cov-append --skip-if-files-missing
# TODO: this is taking too long to run, and is failing consistently. Need to debug this before making it part of the CI again.
# - name: Test with pytest (only slow tests)
# run: pdm run pytest -v -m slow --slow --cov=project --cov-report=xml --cov-append

- name: Store coverage report as an artifact
uses: actions/upload-artifact@v4
with:
name: coverage-reports-integration-tests-${{ matrix.python-version }}
path: ./coverage.xml

launch-slurm-actions-runner:
needs: [local_integration_tests]
runs-on: self-hosted
strategy:
max-parallel: 5
matrix:
cluster: ['mila'] #, 'narval', 'beluga']
cluster: ["mila"] #, 'narval', 'beluga']
outputs:
job_id: ${{ steps.sbatch.outputs.stdout }}
steps:
Expand Down Expand Up @@ -138,25 +153,25 @@ jobs:
matrix:
# TODO: this should be tied to the same setting in the `launch-slurm-actions-runner` job.
# cluster: ${{ needs.launch-slurm-actions-runner.strategy.matrix.cluster }}
cluster: ['mila']
cluster: ["mila"]
steps:
- uses: actions/checkout@v4
- name: Set up the repo using the setup script
run: scripts/mila_setup.sh
- name: Install dependencies
run: rye sync --no-lock
- name: Test with pytest
run: rye run pytest -v --cov=project --cov-report=xml --cov-append --skip-if-files-missing

# TODO: Re-enable this later
# - name: Test with pytest (only slow tests)
# run: pdm run pytest -v -m slow --slow --cov=project --cov-report=xml --cov-append

- name: Store coverage report as an artifact
uses: actions/upload-artifact@v4
with:
name: coverage-reports-slurm-integration-tests-${{ matrix.cluster }}
path: ./coverage.xml
- uses: actions/checkout@v4
- name: Set up the repo using the setup script
run: scripts/mila_setup.sh
- name: Install dependencies
run: rye sync --no-lock
- name: Test with pytest
run: rye run pytest -v --cov=project --cov-report=xml --cov-append --skip-if-files-missing

# TODO: Re-enable this later
# - name: Test with pytest (only slow tests)
# run: pdm run pytest -v -m slow --slow --cov=project --cov-report=xml --cov-append

- name: Store coverage report as an artifact
uses: actions/upload-artifact@v4
with:
name: coverage-reports-slurm-integration-tests-${{ matrix.cluster }}
path: ./coverage.xml

# https://about.codecov.io/blog/uploading-code-coverage-in-a-separate-job-on-github-actions/
upload-coverage-codecov:
Expand Down
7 changes: 3 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ repos:
require_serial: true
- id: check-yaml
require_serial: true
exclude: "mkdocs.yml"
- id: debug-statements
require_serial: true
- id: detect-private-key
Expand All @@ -35,10 +36,9 @@ repos:
rev: "v0.5.7"
hooks:
- id: ruff
args: ['--line-length', '99', '--fix']
args: ["--line-length", "99", "--fix"]
require_serial: true


# python docstring formatting
- repo: https://github.com/myint/docformatter
rev: v1.7.5
Expand Down Expand Up @@ -67,7 +67,7 @@ repos:
rev: 0.7.17
hooks:
- id: mdformat
exclude: 'SUMMARY.md'
exclude: "SUMMARY.md|testing.md"
args: ["--number"]
additional_dependencies:
- mdformat-gfm
Expand All @@ -80,7 +80,6 @@ repos:
- mdformat-mkdocs[recommended]>=2.1.0
require_serial: true


# word spelling linter
- repo: https://github.com/codespell-project/codespell
rev: v2.3.0
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ Why should you use this template (instead of another)?

Here are some of the advantages to using this template compared to [some of the other templates out there](https://mila-iqia.github.io/ResearchTemplate/related):

- ❗Support for both Jax and Torch with PyTorch-Lightning ❗
- ❗Can use both PyTorch and Jax with PyTorch-Lightning ❗
- Neat Configs thanks to [Auto-Generated YAML schemas](https://mila-iqia.github.io/ResearchTemplate/features/auto_schema)
- Easy development inside a [Development Container](https://code.visualstudio.com/docs/remote/containers) with [VsCode](https://code.visualstudio.com/)
- Tailor-made for ML researchers that run their jobs on SLURM clusters (with default configurations for the [Mila](https://docs.mila.quebec) and [DRAC](https://docs.alliancecan.ca) clusters.)
- Rich typing and documentation of all parts of the source code using Python 3.12's new type annotation syntax
Expand Down
33 changes: 0 additions & 33 deletions conftest.py

This file was deleted.

8 changes: 3 additions & 5 deletions docs/SUMMARY.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
* [Home](index.md)
* Overview
* overview/*.md
* Getting Started
* getting_started/*.md
* [Intro](intro.md)
* [Getting Started](install.md)
* Features
* [Magic Config Schemas](features/auto_schema.md)
* features/*.md
* Reference
* reference/*
* Examples
* examples/*
* [Tests](tests.md)
* [Related projects](related.md)
* [Getting Help](help.md)
* [Contributing](contributing.md)
2 changes: 2 additions & 0 deletions docs/features/auto_schema.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Auto Schema for Hydra Configs

> 🔥 NOTE: This is a feature that is entirely unique to this template! 🔥
This project template comes with a really neat feature: Your [Hydra](https://hydra.cc) config files automatically get a [Schema](https://json-schema.org/) associated with them.

This greatly improves the experience of developing a project with Hydra:
Expand Down
Loading

0 comments on commit 422ddba

Please sign in to comment.