Skip to content

build: windows wheels #340

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

build: windows wheels #340

wants to merge 1 commit into from

Conversation

lgray
Copy link
Collaborator

@lgray lgray commented Apr 7, 2025

Enable windows builds of fastjet wheels, pointing to in-progress MRs for fastjet and fastjet-contrib.

c.f. for in-progress MRs:

We'll merge this once all that is in releases.

Below is historical, documenting progress towards #342

This branch will be where we figure out pip install . and cibuildwheel for scikit-hep/fastjet using the in-progress cmake build branches for fastjet/siscone/fastjet-contrib.

I need to get the fastjet-contrib edits into a github repository so we can submodule it correctly.
Right now I have it working locally from the fastjet-contrib svn repository + patches.

At least on my laptop I can get the scikit-hep/fastjet extension compiling and linking.
It doesn't yet function at runtime, but it's only a matter of time.

I'll send some further notifications when it's ready for a more serious look.

One thing we need to solve:

  • MACOSX_DEPLOYMENT_TARGET=11.0 (instead of 14.0/13.0 which is what gets us all green right now)
  • Reduce windows wheel build time (presently 5-7m full build time 40m-1hr installing dependencies)
  • Reduce windows wheel build size (presently 16MB 4.3MB 2.8MB / wheel in windows, ~2.5MB in other OSes)

@matthewfeickert @henryiii

@lgray lgray marked this pull request as draft April 7, 2025 18:49
@lgray lgray changed the title build(WIP): use cmake build system in fastjet and fastjet-contrib build: use cmake build system in fastjet and fastjet-contrib Apr 7, 2025
@lgray
Copy link
Collaborator Author

lgray commented Apr 8, 2025

OK - the present setup builds on my mac using the following commands:

git clone [email protected]:scikit-hep/fastjet.git -b cmake-build-work --recursive
cd fastjet
mkdir build
cmake -S . -B build -DCMAKE_BUILD_TYPE="Release" -DFASTJET_ENABLE_PYTHON=ON -DFASTJET_ENABLE_CGAL=ON -DFASTJET_PYTHON_PACKAGE_NAME=fastjet_cxx
cmake --build build --clean-first --parallel
cd build
python -c 'import _ext; print(_ext)'

Depending on your circumstances you may need to specify pybind11_DIR or turn off CGAL (I have it in from conda).

I'm gonna leave it at that for now y'all can feel free to poke around on the PR with additions, etc.

@lgray
Copy link
Collaborator Author

lgray commented Apr 8, 2025

OK - cool so now it's at least compiling (but not finishing the install!) with scikit-build-core. CGAL is on in all the cases, installed via system libs.

The errors I'm seeing don't make a lot of sense, so would now definitely appreciate help in getting the CI green again :D

@lgray
Copy link
Collaborator Author

lgray commented Apr 8, 2025

hmmm it would be nice to be able to browse the packages available in manylinux online.

anyway leaving it here for now. more than decent progress.

@lgray
Copy link
Collaborator Author

lgray commented Apr 8, 2025

@henryiii

git clone [email protected]:scikit-hep/fastjet.git -b cmake-build-work --recursive
cd fastjet
pip install . -vv
python -c 'import fastjet; print(fastjet)'

@lgray
Copy link
Collaborator Author

lgray commented Apr 8, 2025

I'm also not sure what's up with this metadata error in the CI install. This doesn't happen locally.

@lgray
Copy link
Collaborator Author

lgray commented Apr 8, 2025

Nice, only the macos wheels are failing due to what looks like overlinking?

delocate.libsana.DelocationError: Already planning to copy library with same basename as: libgmp.10.dylib

Still, extremely positive and especially given how much this has simplified the installing and wheel building process.

@lgray
Copy link
Collaborator Author

lgray commented Apr 8, 2025

@gavinsalam FYI this is what is driving changes in the draft PRs over in fastjet and siscone. Once we get this building wheels in all cases then requirements are satisfied on my side. As you can see it's pretty close!

So long as we're careful to reconcile your requirements with the requirements here we shouldn't run into too much trouble. This will also have a side effect of making the new fastjet/siscone/contrib cmake setup fairly robust; cibuildwheel is picky.

@lgray
Copy link
Collaborator Author

lgray commented Apr 9, 2025

Wheels Built

@lgray
Copy link
Collaborator Author

lgray commented Apr 9, 2025

Now that that particularly angry yak is shaved. Let's clean this up and ship it. :-) (after the fastjet/siscone/fastjet-contrib releases, etc.)

@lgray lgray closed this Apr 10, 2025
@lgray lgray reopened this Apr 10, 2025
@lgray lgray closed this Apr 21, 2025
@lgray lgray reopened this Apr 21, 2025
@gavinsalam
Copy link

if CGAL is providing too complicated on Windows, it's probably safe to leave it out for now. The switchover to CGAL happens only at relatively high multiplicities. The heuristics are given in
ClusterSequence::_best_strategy(), https://gitlab.com/fastjet/fastjet/-/blob/master/src/ClusterSequence.cc?ref_type=heads#L650

Specifically, at large R, the change happens for N=100k (anti-kt) or 40k (kt). For smaller R values the switch to NlnN is at even higher R. Very few users will have that kind of multiplicity, even with full HL-LHC pileup. The C/A algorith has lower thresholds but doesn't need CGAL for its NlnN strategy.

@lgray
Copy link
Collaborator Author

lgray commented May 8, 2025

@gavinsalam Thanks for the info! It's not CGAL that's causing problems here (though it does take a long time to install in windows, I have some ideas to mitigate that), it's weirdly strict standards compliance from MSVC + fastjet-contrib not having been completely vetted.

With a little more fiddling it should be working! Only two compilation errors left that I see. The only unfortunate thing is the hour long build time right now, but since I have it working I don't want to now remove CGAL. 😒

@lgray
Copy link
Collaborator Author

lgray commented May 9, 2025

Cool - windows tests pass.

Now to shoehorn it into cibuildwheel.

@lgray
Copy link
Collaborator Author

lgray commented May 10, 2025

@matthewfeickert @henryiii I've got the windows wheel matrix fully up and running. I've added two tasks that I would appreciate help on now that we've got working recipes. Any help you can provide is deeply appreciated in these directions, it'll vastly help with maintainability.

@lgray
Copy link
Collaborator Author

lgray commented May 10, 2025

A quick check of turning off debug info in the windows wheels brought it down to 4.5MB on my home desktop. That's reasonable already. I'll put it in the full wheel matrix if everything works out in the short-form CI.

@lgray
Copy link
Collaborator Author

lgray commented May 10, 2025

looks like things are failing due to gitlab getting angry. I'll start 'em again tomorrow.

@lgray lgray force-pushed the cmake-build-work branch from 85a46d1 to 141966a Compare May 12, 2025 14:31
@lgray
Copy link
Collaborator Author

lgray commented May 14, 2025

Not too bad, down to ~7 minutes for the windows build now.

Will propagate it later to the wheels.

@lgray lgray force-pushed the cmake-build-work branch 6 times, most recently from 44bbb2a to 25cfc2f Compare June 19, 2025 15:12
@lgray lgray force-pushed the cmake-build-work branch 16 times, most recently from 03aeb3f to c1e88d1 Compare June 27, 2025 17:05
@lgray
Copy link
Collaborator Author

lgray commented Jun 27, 2025

Unsurprising, but there are no numpy windows arm64 wheels yet. Fastjet itself actually finished compiling though, we'll be ready when it's available.

@lgray lgray force-pushed the cmake-build-work branch 4 times, most recently from cc7436c to 15b0706 Compare June 27, 2025 20:56
@lgray lgray force-pushed the cmake-build-work branch from 15b0706 to 19dbb3c Compare June 27, 2025 21:24
@lgray
Copy link
Collaborator Author

lgray commented Jun 27, 2025

OK - numpy does build win-arm64 wheels but only for py310 and greater. After that what breaks in the tests is some stuff that's missing the win-arm64 rust toolchain. Rather close to being there, should check again in a couple months.

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.

3 participants