-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Require libgomp
on Linux
#220
Require libgomp
on Linux
#220
Conversation
To satisfy the OpenMP dependency on Linux too, add `libgomp` to the requirements.
Rebuild packages with `libgomp` dependency on Linux.
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
…nda-forge-pinning 2023.05.23.18.17.57
Hi @jakirkham, thank you for contributing. Have you observed any problems at runtime with the current build (e.g. no thread parallelism?). |
Thanks Julien! 🙏 A colleague was seeing a rather strange error message in relation to scinkit-learn usage ( dask-contrib/dask-sql#1144 (comment) ), which when digging more deeply can be caused by GNU OpenMP either being missing or not found correctly ( for example: opencv/opencv#14884 (comment) ) As LLVM OpenMP is included here for macOS and the usual recommendation suggests requiring GNU OpenMP on Linux too, thought we could give this change a try It's possible the issue encountered has other causes, but this would at least rule out one (while improving the package here as well) Would be curious to hear your thoughts on all of this? 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for providing context.
scikit-learn's builds for Linux distributed on conda-forge has been depending on libgomp
implicitly since the support of OpenMP but I think that it is appropriate specifying it explicitly for portability (since it allows switching to llvm-openmp
if needed).
I am waiting for another maintainer's approval before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change makes sense to me. LGTM!
Thanks Julien & Thomas! 🙏 |
Thank you for resolving problems across packages in the ecosystem, John. I hope to see you at the Scientific Python Developer Summit. 🌳 |
Ofc! Hoping to come over there today 😄 |
Looks like OpenMP is used for the builds on macOS, but the dependency is not included on Linux. This adds
libgomp
on Linux to provide OpenMP support on that platform.Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)