Revamped notebook testing infrastructure

[agriyakhetarpal]

Description

Adds pytest plus the pytest-xdist and nbmake plugins to execute notebooks in parallel, along with associated and ancillary changes to the documentation and infrastructure

Fixes #3165

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02% ⚠️

Comparison is base (2eb5c59) 99.57% compared to head (a22166b) 99.56%.
Report is 3 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3344      +/-   ##
===========================================
- Coverage    99.57%   99.56%   -0.02%     
===========================================
  Files          253      253              
  Lines        19571    19546      -25     
===========================================
- Hits         19488    19461      -27     
- Misses          83       85       +2     

Files Changed	Coverage Δ
pybamm/simulation.py	99.54% <100.00%> (-0.46%)	⬇️

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

[agriyakhetarpal]

Even if there isn't a huge improvement shown in the CI with pytest-xdist because Linux GitHub Actions runners have two cores (it is still faster by ~5-7 minutes than the current job though), it is great to have for local development. On my machine with 8 physical cores, I can execute all the notebooks in 11 minutes, which is much faster than the previous ~30 minutes. The times without nbmake and with non-parallel nbmake are nearly identical, with nbmake being slightly faster. We should be able to achieve sub-30 minute CI times by splitting the doctests into another job and reorganising some of the redundant job steps like the installation of PyBaMM dependencies (a job which can get repeated often).

Another thing to be noted is that nbmake is a notebook executor and runs notebooks in a way that resembles how they are run manually, either through Jupyter or VS Code. Our previous method for testing notebooks was to convert them to Python scripts with nbconvert and run them with Python or the ipython kernel, which was not the best way and has accounted for some errors that were displayed and then fixed, like the one in the simulation class example notebook. We did not go forward with alternatives like nbval because it is more of a tool to test notebooks by comparing their outputs to the stored ones, which would not be apt because many PyBaMM features can be stochastic in their nature and thus fail such tests. Reducing CI time for notebook execution was also a factor in consideration, which meant we did not opt for other tools like papermill and testbook, both of them seem to be less actively maintained than before. We looked at another notebook testing toolkit called ploomber-engine with programmatic access to testing notebooks and some advanced features such as parameterising them; however, nbmake as a command-line application seems easier to use for our limited use-case and will also provide pytest as a developer dependency for the possible migration of the test suite from unittest to pytest some time in the future.

[agriyakhetarpal]

Also, I'm not sure about this but nbmake has an Apache license, do we need to include its license file in our source code?

[agriyakhetarpal]

Update: after the failures were fixed, the notebooks step in particular completed in 12 minutes, which looks like it is less than half of the previous times! Now our notebook tests will take roughly the same time as the unit tests or integration tests jobs.

[Saransh-cpp]

Thanks, @agriyakhetarpal! A fantastic step towards migrating to pytest!

[agriyakhetarpal]

Apparently, this was an error that wasn't caught. I updated the argument to match what the documentation says (or said, rather).

[Saransh-cpp]

Looks good, thanks @agriyakhetarpal!