Skip to content

CI fix CircleCI failures by making it run sequencially #25809

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

glemaitre
Copy link
Member

Investigate the CircleCI failure (i.e. EOFError and OSError) that could be linked to some hardware limitation (i.e. memory).

The first thing to try is to limit the number of jobs to build the documentation. This could limit memory consumption.

Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

For the CI, I think it's better to set SPHINX_NUMJOBS in https://github.com/scikit-learn/scikit-learn/blob/main/.circleci/config.yml itself. This way, the sphinx build is still parallel on local development environments.

@adrinjalali
Copy link
Member

I'd be curious if having it serial locally would also make it faster. For skops tests, I've realized setting NUM_THREADS to 1 makes our persistence tests to run much faster. We might be oversubscribing in a few places.

Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

We might be oversubscribing in a few places.

There is some oversubscribing, but it does speed things up on CircleCI. I recall benchmarking on CircleCI and -j2 was ~50% faster than -j1.

In any case, I think it's okay to make it one anyways. Parallelizing Sphinx has lead to issues in the past.

@adrinjalali adrinjalali changed the title CI investigate CircleCI failure CI fix CircleCI failures by making it run sequencially Mar 10, 2023
@adrinjalali adrinjalali enabled auto-merge (squash) March 10, 2023 15:58
@adrinjalali adrinjalali merged commit ac24ebb into scikit-learn:main Mar 10, 2023
lesteve added a commit to lesteve/scikit-learn that referenced this pull request Mar 13, 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