-
Notifications
You must be signed in to change notification settings - Fork 4k
[ci] [python-package] add arm64 macOS wheels #6391
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
Conversation
|
tagging some others who I think might be interested in this approach / topic: |
guolinke
left a comment
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, LGTM!
|
Thanks for pinging me. I would like to adopt a similar approach for XGBoost so that we don't have to vendor libomp. |
|
Looks like this needs a bit more work on the R side. https://github.com/microsoft/LightGBM/actions/runs/9499965097/job/26181969335?pr=6391 I'll look later, but I suspect the issue is my unconditional use of Line 747 in f240bb7
It should probably be templating through LightGBM/R-package/src/install.libs.R Line 187 in 4e74403
I really like that those tests caught the use of |
| # echo "done running pre-commit checks" | ||
| echo "running pre-commit checks" | ||
| pre-commit run --all-files || exit 1 | ||
| echo "done running pre-commit checks" |
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.
I had commented this out in #6377 (comment), to keep merging things in preparation for the release.
Looks like the issues with filelock have been resolved by a new release of that project, and this now runs successfully again 🎉
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. |
Contributes to #5328
Contributes to #4229
Might help with #5764
arm64macOS wheels for the Python package (for the M1/M2/M3 Macs)lib_lightgbm.dylibso that apip-installedlightgbmwon't segfault when used in a conda environmentNotes for Reviewers
Proposed approach for building
arm64wheelsIt seems that Azure DevOps does not support
arm64macOS runners... only GitHub Actions: actions/runner-images#8971.So I'm proposing using GitHub Actions new
macos-14runners, which are arm64 (#5328 (comment)).Proposed approach for fixing loading errors
The problem "using a
pip-installedlightgbminside a conda environment segfaults" is not specific to arm64. It's been around inlightgbm's macOS support for a few years. I decided to try to fix it here to be able to test the new arm64 wheels.I'm proposing the following approach:
libomplib_lightgbm.dylibto be more forgiving about wherelibomp.dylibis foundIn short:
/opt/homebrew/opt/libomp/lib/libomp.dylibcurrently stored inlib_lightgb.dylibforces the macOS loader to load a library from that exact path, even if there is already alibomp.dylibloaded from some other source (like thellvm-openmpshipped y conda)@rpath/libomp.dylib, which means that at runtime oflightgbm, the loader will do the following (in order)libomp.dylibalready loaded, just use that/opt/homebrew/opt/libomp/lib//opt/homebrew/opt/libomp/lib/)/usr/local/liband/usr/libSee "References" section for the MANY resources I consulted to get to this point. Particularly:
How I tested this
downloaded the wheel built in CI (click me)
Got a run ID from CI, looking at the URL.
e.g. for this build: https://github.com/microsoft/LightGBM/actions/runs/9476495757/job/26109485388?pr=6391
the run id is
9476495757.Then, following https://docs.github.com/en/actions/managing-workflow-runs/downloading-workflow-artifacts?tool=cli, used the GitHub CLI to download the wheels. A fine-grained access token with just read-only permission on
Actionsshould be enough.From the root of the repo, on my M2 Mac laptop:
rm -rf ./whl-tmp mkdir -p ./whl-tmp RUN_ID=9476495757 gh run download \ --repo microsoft/LightGBM \ "${RUN_ID}" \ -n macosx-arm64-wheel \ --dir ./whl-tmpcreated a new conda environment (click me)
Created a conda environment.
conda create \ --name delete-me-lgb-dev \ --yes \ --file ./.ci/conda-envs/ci-core.txt \ python=3.10 \ pydistcheck source activate delete-me-lgb-devInspected the wheel with `pydistcheck` (click me)
pydistcheck \ --inspect ./whl-tmp/lightgbm-4.3.0.99-py3-none-macosx_14_0_arm64.whlInstalled from the wheel (on my M2 mac, using macOS 14.4.1).
pip install \ --no-deps \ ./whl-tmp/lightgbm-4.3.0.99-py3-none-macosx_14_0_arm64.whlRan the tests
Those all passed! Removed the brew-installed
libompon my system and ran that again.They passed again 🎉
Can
lightgbmreally safely link to anylibomp.dylib?This project's macOS wheels have already been doing that for years, by shipping with an absolute path that evaluates to "whatever
brew install libompinstalls", and no limits comparing the version linked against at build time to the one used at runtime.The changes in this PR are actually safer than the existing behavior of
lightgbm's macOS wheels because they avoid a well-known source of runtime issue (loading alibomp.dylibthat conflicts with the one loaded byscikit-learn).Why not just vendor
libomp.dyliblike other libraries do?Other projects like
lightgbmbundle a copy of LLVM OpenMP (libomp.dylib) in their wheels.numpyandscipy: https://pypackaging-native.github.io/key-issues/native-dependencies/blas_openmp/xgboost: [CI] Build Python wheels for MacOS (x86_64 and arm64) dmlc/xgboost#7621scikit-learn: https://github.com/scikit-learn/scikit-learn/blob/7e8ad632ff7aec09e99f0c4b8e493e13e19aef14/build_tools/wheels/build_wheels.sh#L20-L22That comes with a significantly reduced risk of "cannot find
libomp.dylib" types of runtime issues, but in exchange for:omp_set_num_threads()does not affect all the loaded OpenMP libraries, and so projects' parallelism can fight each other and lead to oversubscription)Since
lightgbmhas already been using dynamic linking + not vendoring for several years and since "you have tobrew install libomp" has generally not been a problem for macOS users, I'm proposing continuing those practices in this release.Will these CMake changes break the
lightgbmconda package?They shouldn't.
conda-buildrewrites paths in binaries it produces to correctly link between its packages. See "Making packages relocatable" (conda docs)References