Skip to content

WIP on building wheel variants #1

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

Closed
wants to merge 44 commits into from
Closed

Conversation

mgorny
Copy link
Collaborator

@mgorny mgorny commented Mar 21, 2025

This is intended to be developed in parallel with wheelnext/meson-python#1.

The rough plan is as follows:

  1. Get minimal mkl/openblas variant builds with explicit -Dvariant-name and meson args.
  2. Implement -Dvariant and switch over to that.
  3. Implement CPU baseline variants.
  4. Move logic to plugins?

Sorry, something went wrong.

@mgorny
Copy link
Collaborator Author

mgorny commented Mar 22, 2025

Ok, the first demo is ready now: it builds openblas and mkl variants, with x86_64 v1 through v4 CPU baseline. The relevant options are enabled via variant, with only C/C++ flags being set directly in the CI job.

Open questions right now:

  1. Should we alter cpu-dispatch too? I.e. should x86-64-v2 wheel permit dispatching higher features, or do we assume people with capable CPUs would use v3 wheel instead? The latter may involve some corner cases of CPUs implementing feature set between v2 and v3.
  2. What API should plugins use to affect the build environment? I suppose the x86-64 plugin could just set os.environ["CFLAGS"] — but that would hardwire the assumption that the plugin will be called in the same process as the final build. Perhaps we could explicitly define a protocol specifically for passing compiler flags from plugin to the build system — but I'm not sure if we can do it cleanly, and particularly how to deal with different compilers.

Also, plugin work is currently blocked on wheelnext/variantlib#5

Copy link

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good. Shall we enable CI jobs on this fork so we can see it in action more easily?

Also note that all GHA CI jobs are anyway guarded so they run on numpy/numpy by default, so there's no need to delete them I think (may make browsing the diff easier).

Should we alter cpu-dispatch too? I.e. should x86-64-v2 wheel permit dispatching higher features,

I think that makes sense. Disabling dispatching for v1-v3 seems fine, since it reduces binary size and should only affect performance for some not-very-relevant corner cases. Only the highest variant (v4) should probably keep dispatching.

Perhaps we could explicitly define a protocol specifically for passing compiler flags from plugin to the build system

I don't think that is possible. I think the plugin should have an API to query what the desired flags are, and the build backend should call that API and then pass on the flags to the build system (it'll be build backend specific).

@mgorny
Copy link
Collaborator Author

mgorny commented Mar 26, 2025

Good points. I've reverted the irrelevant changes and leftovers from earlier experiments to keep the diff clean.

@mgorny
Copy link
Collaborator Author

mgorny commented Mar 26, 2025

Hmm, looks like my CI pipelines are failing because they've cached old meson-python commit. I'll check again tomorrow.

@rgommers
Copy link

Isn't everything just skipped? I think the fix is changing these lines:

if: github.repository == 'numpy/numpy'

@mgorny
Copy link
Collaborator Author

mgorny commented Mar 26, 2025

I mean in the added variants-build.yml workflow: https://github.com/mgorny/numpy/actions/runs/14091739554

mgorny added 7 commits April 14, 2025 08:13
@mgorny
Copy link
Collaborator Author

mgorny commented Apr 14, 2025

I've enabled artifact publishing on the workflows, in the fork: https://github.com/mgorny/numpy/actions/runs/14452129682

mgorny added 2 commits April 15, 2025 16:37
This reverts commit d32be73.
That shouldn't be necessary, I was getting confused by meson output.
@mgorny
Copy link
Collaborator Author

mgorny commented Apr 28, 2025

This is now wheel-variants branch on this fork.

@mgorny mgorny closed this Apr 28, 2025
@rgommers
Copy link

rgommers commented Apr 29, 2025

The build logs look quite good, seems like this is ready for demo-ing indeed!

There is one minor funky thing with AVX2 support it looks like. E.g., from this mkl-x86_64-v3 log:

Test features "SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42 AVX F16C FMA3 AVX2" : Supported 
Configuring npy_cpu_dispatch_config.h using configuration
Message: 
CPU Optimization Options
  baseline:
    Requested : AVX2
    Enabled   : SSE SSE2 SSE3 SSSE3 SSE41 POPCNT SSE42 AVX F16C FMA3 AVX2
  dispatch:
    Requested : 
    Enabled   : 
...

 Generating multi-targets for "_umath_tests.dispatch.h" 
  Enabled targets: baseline
Generating multi-targets for "argfunc.dispatch.h" 
  Enabled targets: baseline
Generating multi-targets for "x86_simd_argsort.dispatch.h" 
  Enabled targets: AVX2

It may be a quick in NumPy's SIMD support, or it may be something like the order of flags that come in as build options through what's implementing variant: ['x86_64 :: level :: v3'] is somehow different than what NumPy expects.

That's the only thing I noticed, everything else looks as I expected.

@rgommers
Copy link

Except, as we just discussed, that the wheels aren't redistributable yet because of no auditwheel. We'll follow up on that. We shouldn't be manually running auditwheel, but rather just use the actual cibuildwheel config that NumPy upstream uses, with minimal tweaks (only adding -Csetup-args=variant.... in cibuildwheel config in pyproject.toml I'd think.

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

Successfully merging this pull request may close these issues.

None yet

2 participants