-
Notifications
You must be signed in to change notification settings - Fork 580
Python wheels #2731
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
base: develop
Are you sure you want to change the base?
Python wheels #2731
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| name: Build Wheels | ||
|
|
||
| on: [push, workflow_dispatch] | ||
|
|
||
| jobs: | ||
| build_wheels: | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| - runs-on: ubuntu-20.04 | ||
| cibw-arch: manylinux_x86_64 | ||
| - runs-on: macos-latest | ||
| cibw-arch: macosx_x86_64 | ||
| - runs-on: macos-latest | ||
| cibw-arch: macosx_arm64 | ||
|
|
||
| name: Build wheels on ${{ matrix.cibw-arch }} | ||
| runs-on: ${{ matrix.runs-on }} | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| submodules: "recursive" | ||
|
|
||
| - name: Build wheels | ||
| uses: pypa/[email protected] | ||
| env: | ||
| CIBW_BUILD: "cp311-${{ matrix.cibw-arch }}" | ||
| CIBW_ARCHS_MACOS: x86_64 arm64 | ||
| CIBW_ARCHS_LINUX: x86_64 | ||
| CIBW_MANYLINUX_X86_64_IMAGE: manylinux_2_28 | ||
| CIBW_ENVIRONMENT_LINUX: LD_LIBRARY_PATH="/usr/local/lib64:${LD_LIBRARY_PATH}" PIP_ONLY_BINARY="numpy" | ||
| CIBW_BEFORE_ALL_LINUX: | | ||
|
Comment on lines
+30
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered moving the cibuildwheel configuration into the |
||
| set -eu | ||
| dnf install -y epel-release | ||
| dnf config-manager --enable epel | ||
| dnf install -y sudo cmake eigen3-devel gcc gcc-c++ wget hdf5-devel | ||
| bash build-deps.sh | ||
| echo "LD_LIBRARY_PATH: ${LD_LIBRARY_PATH}" | ||
| CIBW_ENVIRONMENT_MACOS: MACOS_DEPLOYMENT_TARGET=10.9 CMAKE_OSX_ARCHITECTURES=${{ matrix.cibw-arch == 'macosx_x86_64' && 'x86_64' || matrix.cibw-arch == 'macosx_arm64' && 'arm64' || '' }} PIP_ONLY_BINARY="numpy" | ||
| CIBW_BEFORE_ALL_MACOS: | | ||
| set -eu | ||
| if [ "$CMAKE_OSX_ARCHITECTURES" == "arm64" ]; then | ||
| # Uninstall libpng so we can get the correct version | ||
| dependent_packages=$(brew uses --installed libpng) | ||
| brew uninstall --ignore-dependencies $dependent_packages libpng | ||
| PACKAGES=(libpng libaec eigen hdf5) | ||
| for PACKAGE in "${PACKAGES[@]}" | ||
| do | ||
| brew fetch --force --bottle-tag=arm64_big_sur $PACKAGE | ||
| brew install $(brew --cache --bottle-tag=arm64_big_sur $PACKAGE) | ||
| done | ||
| else | ||
| brew install eigen hdf5 | ||
| fi | ||
| bash build-deps.sh | ||
| - uses: actions/upload-artifact@v3 | ||
| with: | ||
| path: ./wheelhouse/*.whl | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -570,6 +570,12 @@ add_custom_command(TARGET libopenmc POST_BUILD | |
| ${CMAKE_CURRENT_SOURCE_DIR}/openmc/lib/$<TARGET_FILE_NAME:libopenmc> | ||
| COMMENT "Copying libopenmc to Python module directory") | ||
|
|
||
| add_custom_command(TARGET openmc POST_BUILD | ||
| COMMAND ${CMAKE_COMMAND} -E copy | ||
| $<TARGET_FILE:openmc> | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/openmc/lib/$<TARGET_FILE_NAME:openmc> | ||
| COMMENT "Copying openmc binary to Python module directory") | ||
|
|
||
|
Comment on lines
+573
to
+578
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know that the This might be a chance to move away from that approach. That's one way, I'm sure there is more 🤷
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, what we have currently for libopenmc is very hacky, so I would not advocate for doubling down on it and handling the executable the same way. The approach you have there @hassec sounds a lot better. The other option that comes to mind is to use scikit-build (see #916). It looks like there are some other build backends out there that might merit a look. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW, @shimwell had a version that tried to use I don't have any experience with that package yet.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recently started tinkering with scikit build core (which I think now replaces scikit-build) on branch if anyone is interested develop...shimwell:openmc:trying-scikit-core |
||
| #=============================================================================== | ||
| # Install executable, scripts, manpage, license | ||
| #=============================================================================== | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||||||
| #!/bin/bash | ||||||||
| set -eu | ||||||||
|
|
||||||||
| # pip install numpy==1.25.1 cython==0.29.36 # required for MOAB install (silly) | ||||||||
|
|
||||||||
| # It's important to use LLVM clang rather than Apple clang | ||||||||
| # CC=/opt/homebrew/Cellar/llvm/16.0.6/bin/clang | ||||||||
| # CXX=/opt/homebrew/Cellar/llvm/16.0.6/bin/clang++ | ||||||||
|
|
||||||||
| NUM_CORES=2 | ||||||||
|
|
||||||||
| # 5.5.0 breaks with pyne std::isnan bug | ||||||||
| MOAB_TAG='5.3.0' | ||||||||
|
Comment on lines
+12
to
+13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 5.5.1 has been released, might fix the pyne std::isnan bug
Suggested change
|
||||||||
| MOAB_REPO='https://bitbucket.org/fathomteam/moab/' | ||||||||
|
|
||||||||
| DAGMC_TAG='v3.2.2' | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another version bump
Suggested change
|
||||||||
| DAGMC_REPO='https://github.com/svalinn/DAGMC' | ||||||||
|
|
||||||||
| # MOAB | ||||||||
| # WARNING: CMake often gets wrong python path. Check logs in case of error. | ||||||||
| git clone --single-branch -b ${MOAB_TAG} --depth 1 ${MOAB_REPO} | ||||||||
| pushd moab | ||||||||
| cmake -S . -B build \ | ||||||||
| -DCMAKE_BUILD_TYPE=Release \ | ||||||||
| -DENABLE_HDF5=ON \ | ||||||||
| -DENABLE_NETCDF=OFF \ | ||||||||
| -DBUILD_SHARED_LIBS=ON \ | ||||||||
| -DENABLE_FORTRAN=OFF \ | ||||||||
| -DENABLE_BLASLAPACK=OFF \ | ||||||||
| -DENABLE_PYMOAB=OFF | ||||||||
| cmake --build build -j $NUM_CORES | ||||||||
| sudo cmake --build build -t install | ||||||||
| popd | ||||||||
|
|
||||||||
| # DAGMC | ||||||||
| git clone --shallow-submodules --recurse-submodules --single-branch -b ${DAGMC_TAG} --depth 1 ${DAGMC_REPO} | ||||||||
| pushd DAGMC | ||||||||
| # MANUAL: Fix isnan problem in pyne.h and pyne.cpp, changing isnan to std::isnan | ||||||||
| cmake -S . -B build \ | ||||||||
| -DCMAKE_BUILD_TYPE=Release \ | ||||||||
| -DBUILD_TALLY=ON \ | ||||||||
| -DMOAB_DIR=/usr/local \ | ||||||||
| -DBUILD_STATIC_LIBS=OFF | ||||||||
| sudo cmake --build build -j $NUM_CORES -t install | ||||||||
| popd | ||||||||
|
|
||||||||
| # OpenMC | ||||||||
| # build dir is claimed by bdist_wheel | ||||||||
| cmake -S . -B bld \ | ||||||||
| -DCMAKE_BUILD_TYPE=Release \ | ||||||||
| -DCMAKE_BUILD_TESTS=OFF \ | ||||||||
| -DOPENMC_USE_DAGMC=ON | ||||||||
| cmake --build bld -j 8 | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| #!/usr/bin/env python3 | ||
| import os | ||
| import sysconfig | ||
| import sys | ||
|
|
||
| os.execv(sysconfig.get_path("platlib") + "/openmc/lib/openmc", sys.argv) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't know about this before. I was doing this with |
||
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.
Might just be because it's still draft but this probably doesn't need to run on every push. I'd suggest to run on every tag creation.