CI, BLD: Use cibuildwheel to build WASM NumPy wheels#26570
CI, BLD: Use cibuildwheel to build WASM NumPy wheels#26570rgommers merged 1 commit intonumpy:mainfrom
cibuildwheel to build WASM NumPy wheels#26570Conversation
|
Do we want an extra CI job for this, or only change the existing one? |
Ah, I should have mentioned it in the PR description. I plan to change the existing one to use P.S. This could have been incorporated into |
|
Sounds good to me, thanks. And yes, I'd prefer to keep it separate from |
agriyakhetarpal
force-pushed
the
cibuildwheel-pyodide
branch
2 times, most recently
from
b29bf4c to
f2d0d79
Compare
| # To enable this workflow on a fork, comment out: | ||
| if: github.repository == 'numpy/numpy' | ||
| env: | ||
| PYODIDE_VERSION: 0.26.0 |
There was a problem hiding this comment.
I feel that it would be better not to pin the Pyodide version for this workflow. Two reasons:
- The
pyodide-buildversion is coupled with the Pyodide version, which in-turn is set bycibuildwheel. In my earlier commits, I usedpipto installpyodide-build, which can fall out of date - NumPy is one of the fundamental packages and is extensively built and tested with Pyodide when the Emscripten toolchain version is bumped (i.e., when the ABI breaks). It's quite unlikely that building it would be broken for a released version of Pyodide, and owing to the experimental nature of the build system and this feature in general,
cibuildwheelwill not plan to provide WebAssembly builds for a tip-of-tree or unlisted Pyodide build – which would require its own CDN or distribution channel, anyway.
Do you have any thoughts about this, @rgommers?
There was a problem hiding this comment.
Yes, I think it's fine to remove this and always build/test with the latest releases. We have lots of unpinned test setups like this, and by now our Pyodide support has stabilized.
| - name: Upload wheel artifact(s) | ||
| uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3 |
There was a problem hiding this comment.
I see that other workflows upload artifacts on failure() events, while some upload them without conditions. I have added this for the sake of completeness/convenience, but it's not needed if we don't plan to share it across jobs, at least for now (the documentation builds can be adjusted to need them later). Happy to keep this or remove this based on further directions.
agriyakhetarpal
changed the title
cibuildwheel to build WASM NumPy wheelscibuildwheel to build WASM NumPy wheels
agriyakhetarpal
force-pushed
the
cibuildwheel-pyodide
branch
from
f2d0d79 to
fbe40a6
Compare
|
|
||
| [tool.cibuildwheel.pyodide] | ||
| config-settings = "build-dir=build setup-args=--cross-file=$PWD/tools/ci/emscripten/emscripten.meson.cross setup-args=-Dblas=none setup-args=-Dlapack=none" | ||
|
|
There was a problem hiding this comment.
This will use lapack-lite. Is there a better BLAS implementation available?
There was a problem hiding this comment.
It's supposed to use lapack-lite here (for now at least), that's what is happening in Pyodide in-tree as well. It's possible that will switch to openblas in the future, but it doesn't really matter here (performance isn't a high prio for things like interactive docs).
There was a problem hiding this comment.
Happy to make it try to use OpenBLAS or any alternative BLAS vendor in the future (not for now, though – indeed not a priority)
|
@rgommers is this ready for merging? |
|
I just noticed that there were conflicts here. I've merged the latest changes from |
This commit performs the following changes: 1. cibuildwheel version 2.20.0 is added for building and testing the Pyodide wheels for NumPy in its workflow for out-of-tree builds. 2. The workflow sets the CIBW_PLATFORM environment variable in order to point to Pyodide as the build target. 3. The version of Pyodide used is bumped to the recent version 0.26.1. 4. The TOML tables are updated to use cibuildwheel's provided overrides for the test command, the wheels' repair command, and the setup arguments needed at the time of building the wheel. [skip azp] [skip circle] [skip cirrus]
rgommers
force-pushed
the
cibuildwheel-pyodide
branch
from
51a90fc to
ad58c55
Compare
rgommers
left a comment
rgommers
left a comment
There was a problem hiding this comment.
I reviewed this, rebased it and made one more tweak (updating to cibuildwheel 2.20.0), and everything LGTM. Time to merge this. Thanks @agriyakhetarpal, and thanks for the review @mattip.
| # To enable this workflow on a fork, comment out: | ||
| if: github.repository == 'numpy/numpy' | ||
| env: | ||
| PYODIDE_VERSION: 0.26.0 |
There was a problem hiding this comment.
Yes, I think it's fine to remove this and always build/test with the latest releases. We have lots of unpinned test setups like this, and by now our Pyodide support has stabilized.
| config-settings = "setup-args=--vsenv setup-args=-Dallow-noblas=true build-dir=build" | ||
| repair-wheel-command = "" | ||
|
|
||
| [[tool.cibuildwheel.overrides]] |
There was a problem hiding this comment.
It indeed seems correct to have multiple sections with this exact name (see https://cibuildwheel.pypa.io/en/stable/options/#overrides). That design seems a bit questionable to me, but if it works it works.
3 participants
Description
This PR intends to invoke pypa/cibuildwheel#1456 for building and testing the Pyodide builds for NumPy. cc @rgommers who recommended testing out on a fork of NumPy first, which I did, and I got it to pass.
Update:
cibuildwheelversion 2.19.1 is out with the required support for Pyodide.Additional context
cibuildwheeldocumentation: https://cibuildwheel.pypa.io/en/latest/setup/#pyodide-webassembly-builds-experimental