Skip to content

Conversation

@cwmeijer
Copy link
Member

@cwmeijer cwmeijer commented Dec 4, 2024

because timeout occurred for a cell which caused the notebook test to fail

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@cwmeijer cwmeijer marked this pull request as draft December 5, 2024 08:49
@cwmeijer
Copy link
Member Author

cwmeijer commented Dec 5, 2024

After increasing the time limit from 300s to 1800s, the same cell still times out. The cell contains the call to dianna in the land_atmosphere notebook. The notebook was running fine before (with 300s limit). We should investigate more.

@SarahAlidoost
Copy link
Member

SarahAlidoost commented Dec 19, 2024

After increasing the time limit from 300s to 1800s, the same cell still times out. The cell contains the call to dianna in the land_atmosphere notebook. The notebook was running fine before (with 300s limit). We should investigate more.

There are two cells in the notebook that take a few minutes to run due to large data. These cells are not suitable for execution with GitHub Action runners. Currently, the github workflow takes about 30 minutes, which is too long to be practical as a check for pull requests. The notebook workflow only runs pytest --nbmake, and the results are not used elsewhere. For the notebook kernelshap_tabular_land_atmosphere.ipynb, I have added an option to skip running time-consuming cells online. I suggest implementing the same thing for other tutorial notebooks if they also contain cells that require more runtime.

@SarahAlidoost SarahAlidoost marked this pull request as ready for review December 19, 2024 13:38
@SarahAlidoost
Copy link
Member

@cwmeijer running notebook lime_text_eulaw.ipynb has failed, see action here. The problem is new version of scikit-learn (1.6.x), see related issues dmlc/xgboost#11021 and scikit-learn/scikit-learn#30479.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants