aai-institute / pyDVL

pyDVL is a library of stable implementations of algorithms for data valuation and influence function computation
https://pydvl.org
GNU Lesser General Public License v3.0
100 stars 8 forks source link

Fix notebook tests #550

Closed jakobkruse1 closed 5 months ago

jakobkruse1 commented 6 months ago

Description

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

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

Checklist

mdbenito commented 6 months ago

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

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

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.

jakobkruse1 commented 6 months ago

@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 commented 6 months ago

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 commented 5 months ago

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 commented 5 months ago

@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).

image image

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 commented 5 months ago

@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).

image image

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