Skip to content

CI Disable sphinx parallelism in CircleCI doc build #25832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Mar 13, 2023

Reference Issues/PRs

#25809 actually did not disable sphinx parallelism as you can see from the -j2 in the build log:

sphinx-build -b html -T -d _build/doctrees  -T -j2  . _build/html/stable

I think this is because we do export SPHINX_NUMJOBS=2 in build_tools/circle/build_doc.sh and in the Makefile SPHINX_NUMJOBS ?= 1 only happens if SPHINX_NUMJOBS is not already defined ...

What does this implement/fix? Explain your changes.

This is another attempt at disabling sphinx parallelism and see if it gets rid of the timeout and EOFError in CircleCI which seems to happen quite consistently recently, see https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn?branch=main.

cc @glemaitre

Comment on lines +64 to +65
# Disable sphinx parallelism to avoid EOFError or job stalling in CircleCI
- SPHINX_NUMJOBS: 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be nice to have it run sequencially on a local machine too though.

Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the PR merged, it seems that it was the proposal of @thomasjpfan but it got merged before I modify it.

@adrinjalali Since the issue should be related to limited hardware, it could still be nice to be able to run the sphinx with several jobs. You can always ask to set the environment variable for reproducibility.

@lesteve
Copy link
Member Author

lesteve commented Mar 13, 2023

I double-checked in the log that sphinx parallelism was disabled and it is the case indeed (-j1 instead of -j2):

sphinx-build -b html -T -d _build/doctrees  -T -j1  . _build/html/stable

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do a test and see how it works locally, but this is good to merge regardless.

@adrinjalali adrinjalali enabled auto-merge (squash) March 13, 2023 11:22
@adrinjalali adrinjalali merged commit 13d56e9 into scikit-learn:main Mar 13, 2023
@lesteve lesteve deleted the disable-sphinx-parallelism-in-ci branch March 13, 2023 11:55
@lesteve
Copy link
Member Author

lesteve commented Mar 13, 2023

FWIW it seems the latest doc build passed on main: https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn?branch=main

Veghit pushed a commit to Veghit/scikit-learn that referenced this pull request Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants