Fix notebook tests

[jakobkruse1]

Description

Currently, many notebooks tests fail due to timeouts or other bugs.
This branch aims to solve this by doing the following:

• Increase speed of the notebooks or increase timeout
• Make successful notebook tests mandatory
• Ensure that the entire job does not run for too long to not annoy developers (same runtime as test jobs)

Pytest has its timeout set to 300 seconds, running all 10 notebooks in 300 might be nice. Other option is to split the notebook-tests like for the regular tests.

Fixed notebook errors

• data_oob: Timeout
• influence_imagenet Timeout (only locally, not in CI)
• influence_sentiment_analysis: RuntimeError and too slow in CI
• least_core_basic: ValueError when solver does not converge (NaN)
• least_core_basic: Too slow
• msr_banzhaf_spotify and shapley_basic_spotify: KeyError 'Billie Eilish' happens sometimes (randomness)
• shapley_utility_learning: Timeout

Checklist

• If notebooks were added/changed, added boilerplate cells are tagged with "tags": ["hide"] or "tags": ["hide-input"]

[mdbenito]

@jakobkruse1 It's awesome that notebooks run again! Thanks for this.

I guess you'll have noticed, but just in case:

• Notebooks should always run with less resources in CI. We check with os.getenv('CI') e.g. to retrieve just a fraction of the data, but we might also want to change some parameters, like number of iterations to a bare minimum. If a notebook fails to do this, it must be fixed.
• Results from automatically ran notebooks don't really matter because we must manually run them with the whole data (could take hours for one notebook), and commit them rendered (which reminds me that we must move them to LFS once and for all, something that we should've done since the beginning 😒)

So, maybe one can instead check for CI or PYTEST_RUNNING (which we set with a flag when calling pytest) or something like that. Of course this should be a function in the support module for the notebooks, etc.

[mdbenito]

Thanks for this. Note that we use pytest-split for the other tests. Is it also active for the notebook tests? And if yes, shouldn't we update the .test_durations file?

[jakobkruse1]

@mdbenito I am not using pytest-split for the notebooks. I am using os.getenv("CI") which keeps the runtime of the notebooks acceptable. Therefore, splitting is not required as of now. In the future, splitting may be necessary to keep the runtimes acceptable.

[mdbenito]

What is "acceptable"? Wouldn't it make sense to have add pytest-split while you are at it. We will have more notebooks, probably many of them.

[jakobkruse1]

This PR is ready to merge from my side now @mdbenito @AnesBenmerzoug. Could somebody review it again? Also, I can't merge this, somebody else would have to do that :)

[AnesBenmerzoug]

@jakobkruse1 I just had a quick look at the CI run times for the notebook tests and they seem to vary wildly. You should run the notebook tests using tox -e notebook-tests -- --store-durations in order to record the duration of each notebook run so that they're split more evenly (See this section in the contributing docs for more information).

Other than that, the only thing I would look into would be the rendering of the notebooks in the documentation (You could check that by building the docs locally using mkdocs serve and navigate to the Examples section)

[jakobkruse1]

@jakobkruse1 I just had a quick look at the CI run times for the notebook tests and they seem to vary wildly. You should run the notebook tests using tox -e notebook-tests -- --store-durations in order to record the duration of each notebook run so that they're split more evenly (See this section in the contributing docs for more information).

Other than that, the only thing I would look into would be the rendering of the notebooks in the documentation (You could check that by building the docs locally using mkdocs serve and navigate to the Examples section)

Done @AnesBenmerzoug

[AnesBenmerzoug]

Looks good to me! I will merge it later this afternoon if Miguel doesn't leave any other comment.