Skip to content
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

[CI] use CIBW overrides to build arm64 macOS wheels #428

Merged

Conversation

psobolewskiPhD
Copy link
Contributor

@psobolewskiPhD psobolewskiPhD commented Apr 3, 2023

At present numcodecs doesn't have wheels for arm64 macOS.
It builds from source just fine, but not everyone may have development tools.
Wheels make things potentially safer and of course faster.

closes: #342

In this PR I add use of CIBW_ARCHS_MACOS to the existing workflow, setting 'x86_64 arm64'. This results in wheels cross compiled for arm64 in addition to the x86 ones. The runners are x86, so the arm64 wheels won't be tested.
However, this also requires using overrides to ensure that both AVX2 and SSE2 are disabled.
As a result, the ENVIROMENT aspects are moved to pyproject.toml

I tested this on my fork: psobolewskiPhD#2
I dowloaded the arm64 wheels, pip installed locally, and ran pytest:

====== 583 passed, 33 skipped, 36 xfailed, 1 xpassed, 5 warnings in 8.84s ======

Another option is to use universal2 and build a FAT wheel for x86 and arm64. The existing wheels arn't too big, so this isn't so bad? But I assume arm64 is still in the minority, so maybe separate wheels are better?

Also from a CI time PoV, it's worth considering whether full pytest needs to run on wheels on top of the CI tests for each PR/push, see: #427 (comment)

BTW: this same approach may be a cleaner way to implement #315

TODO:

  • Unit tests and/or doctests in docstrings
  • Tests pass locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • Docs build locally
  • GitHub Actions CI passes
  • Test coverage to 100% (Codecov passes)

@joshmoore
Copy link
Member

joshmoore commented Apr 4, 2023

Thanks for this, @psobolewskiPhD. Workflows launched.

The primary worry to date has been on further extending the build time since we are approaching limits. Do you have a feeling for the overall increase here? (and/or options for keeping it under control)

@psobolewskiPhD
Copy link
Contributor Author

@joshmoore
based on my fork, the macOS wheel step will take ~60% longer.
One strategy to reduce time would be to not duplicate testing, as I mention here:
#427 (comment)

Another option is to not build wheels on PRs, but there are some advantages to that...

@psobolewskiPhD
Copy link
Contributor Author

psobolewskiPhD commented Apr 4, 2023

Thinking about this specific PR, I assume one universal2 wheel would be faster than building twice. But I'm not sure how to deal with SSE2 then, because x86 macOS should have that (I'd assume most x86 macs have AVX2 as well but 🤷‍♂️), but arm64 not.

@psobolewskiPhD
Copy link
Contributor Author

Both PRs finished. Compared to the concurrency one (no arm64)
https://github.com/zarr-developers/numcodecs/actions/runs/4600452760/jobs/8150420382?pr=427
21.5 min
here 26 min, so not so bad?
Testing of the macOS ones is ~1 min per wheel...

@psobolewskiPhD
Copy link
Contributor Author

Gentle bump—I should have some time to return to this topic.

@joshmoore
Copy link
Member

Thanks again for the ping. I've updated the branch to check the current state and will go over the code now.

@joshmoore
Copy link
Member

@jakirkham: anything from your side here? I'm inclined to get it in (if still green) and then we let @psobolewskiPhD try to speed the builds up.

@joshmoore
Copy link
Member

I just rebuilt the failing job. Now all are green. I'm going to merge this to look at your gh-453. We can then start considering all the changes together for a release. Thanks again, @psobolewskiPhD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

arm64 wheel for Mac M1?
2 participants