Skip to content

CI "no OpenMP" build environment actually has OpenMP #24694

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.

Sign up for GitHub

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

Jump to bottom
Closed
lesteve opened this issue Oct 18, 2022 · 3 comments · Fixed by #25391
Closed

CI "no OpenMP" build environment actually has OpenMP #24694

lesteve opened this issue Oct 18, 2022 · 3 comments · Fixed by #25391
Labels
Build / CI

Comments

@lesteve
Copy link
Member

lesteve commented Oct 18, 2022

This avoided catching a regression where an unprotected cimport openmp was introduced.

As a side-comment: Pyodide build needs to be built without OpenMP.

From #24682 (comment), there is OpenMP in the build environment:

❯ ag openmp build_tools/azure/pylatest_conda_mkl_no_openmp_osx-64_conda.lock 
9:https://repo.anaconda.com/pkgs/main/osx-64/intel-openmp-2021.4.0-hecd8cb5_3538.conda#65e79d0ffef79cbb8ebd3c71e74eb50a
15:https://repo.anaconda.com/pkgs/main/osx-64/llvm-openmp-14.0.6-h0dcd299_0.conda#b5804d32b87dc61ca94561ade33d5f2d

From #24682 (comment)

Looking at why we get OpenMP in the "no OpenMP" build:

So it seems like if we want an "no OpenMP" build we need it to be Linux or Windows. Not sure whether there was a good reason to have it on OSX originally.
@lesteve lesteve mentioned this issue Oct 18, 2022
Merged
@thomasjpfan
Copy link
Member

Not sure whether there was a good reason to have it on OSX originally.

The original reason for disability OpenMP was because the default compiler on OSX did not have OpenMP: #15174

I think using linux to test no OpenMP is sufficient and +1 on moving the CI over.

This was referenced Jan 12, 2023
Merged
Merged
@lesteve
Copy link
Member Author

lesteve commented Jan 13, 2023 •
edited
Loading

In #25391 an unprotected cimport openmp will make the no-OpenMP build fail. I think this fixes the main issue.

Also, it does make sense to keep the OSX with system clang use case build, since this may be something that happens in practice for some users and we prefer that the build works with a warning. That means (at least for now) that the no-OpenMP build has OpenMP, which is a bit weird but oh well 🤷.

For completeness I could not find a way to create a conda environment without OpenMP. For example on Linux as soon as you have libgcc-ng you will get libgomp, so for example creating an environment with python will bring in libgomp.

In a not so far future, we may have a Pyodide build, which will be a "no-OpenMP whatsoever" build.

@jeremiedbb
Copy link
Member

jeremiedbb commented Jan 13, 2023 •
edited by lesteve
Loading

That means (at least for now) that the no-OpenMP build has OpenMP, which is a bit weird but oh well 🤷.

To be precise, there's openmp in the env, but scikit-learn is not built against openmp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Build / CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

CI Make no-OpenMP build fail with unprotected cimport openmp lesteve/scikit-learn
3 participants
@lesteve @thomasjpfan @jeremiedbb