build: windows wheels

[lgray]

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

c.f. for in-progress MRs:

• https://gitlab.com/fastjet/fastjet/-/merge_requests/25
• https://gitlab.com/fastjet/siscone/-/merge_requests/15
• cmake-based build for fastjet-contrib (cmake requires fastjet >=3.5.0, siscone >=3.1.1 also built with cmake) cms-externals/fastjet-contrib#19

We'll merge this once all that is in releases, except for contrib...

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 2.4MB / wheel in windows, ~2.0MB in other OSes)

@matthewfeickert @henryiii

[lgray]

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]

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]

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]

@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]

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

[lgray]

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]

@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]

[lgray]

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

[gavinsalam]

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]

@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]

Cool - windows tests pass.

Now to shoehorn it into cibuildwheel.

[lgray]

@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]

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]

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

[lgray]

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

Will propagate it later to the wheels.

[henryiii]

You can add the Rust toolchain (see actions/partner-runner-images#77), and I think it's coming soon anyway. I'd skip older Python's on Win ARM, there really aren't legacy users there anyway.

[lgray]

Thanks @henryiii, let's see if it blends.

[lgray]

Yikes, now it dies on building pyarrow.

[lgray]

What I've decided to do in the mean time is just skip tests on windows-11-arm64. So much of the infrastructure is missing or won't build at the moment, and the library itself builds fine and tests fine in x64 windows and arm64 linux/macos. So we're very likely safe.

[lgray]

This is now waiting for the release of fastjet-3.5.1. I'm going to flip it out of draft mode.

[lgray]

OK - this is now using fastjet-3.5.1

[lgray]

@gavinsalam I shall require your blessing to merge this. :-)

I think the only point of concern is using the cmake patchset atop fastjet-contrib, but it is localized entirely to the build step of this project. It's not presenting it to users for wider consumption, so it's much less of a risk.

[gavinsalam]

@gavinsalam I shall require your blessing to merge this. :-)

I think the only point of concern is using the cmake patchset atop fastjet-contrib, but it is localized entirely to the build step of this project. It's not presenting it to users for wider consumption, so it's much less of a risk.

This is blessing with regard to the Windows wheels, or specifically the fjcontrib changes? Still haven't had a chance to go through those (what bandwidth there was on our end went to 3.5.1)

[lgray]

@gavinsalam The windows wheels require the fjcontrib changes, so technically both!

The patches are summarized here in 000x-*.patch:
https://github.com/conda-forge/fastjet-contrib-feedstock/tree/main/recipe

The big ones are:

• 0003 for the injected CMake build using a for loop over auto-detected contrib dirs (using the same logic as in your configure script, but in CMake)
• 0004 for the various DLL flags which are all now <CONTRIB>_WINDLL following the convention in fastjet/siscone

Perhaps the thing you may take issue with in these are the auto-generated <contrib>_defines.h files that hold the dll flag definition macros? That required adding includes to many files, but was necessary. It also means that new contribs would need to mind this, if adopted centrally. For now it's just there to get the build chugging along and isn't really publicly exposed.

0001 and 0002 are thread safety and maintenance patches from the CMS collaboration that have been in https://github.com/cms-externals/fastjet-contrib/tree/cms/v1.101 and CMSSW for quite some time.

No other changes aside from that.

Thanks for your time!

[lgray]

And, please, no need to rush this. I'm only on top of it because the faster I get this wrapped up the faster I can give it to someone else to maintain, as we will be in a considerably more steady state from then on out.

[lgray]

Just to keep this moving along, I will merge this August 1st regardless of blessing.
Everything works correctly and we are willing to support issues that may crop up from our changes which are either just some defines or the build system.

... We resemble but are legally distinct from the lollipop guild.

[gavinsalam]

Hi Lindsey, we looked briefly at the FJContrib part some weeks ago. It works at our end, in terms of compiling with CMake. One "feature" that we noticed is that the per-contrib CMakeLists.txt files get added to the source directory (not ideal, still need to think whether it's actually serious) and with hard-coded paths (would prevent multiple distinct builds from the same source directory). Not a showstopper probably for the wheels, but it would prevent us including this as-is in fjcontrib. We hoped to find time to look at it in more detail and wanted to do so before getting back to you, but in the end we hit the usual bandwidth problem...

[lgray]

Hi Gavin - thanks for the quick response and having looked at it!

Those comments were things I was thinking to clean up eventually, but indeed not really necessary for the wheel builds in this current state. I think I know how to do it all still with the top-level-only cmake, it's just more going around one's elbow to get to the matter at hand. However, I understand the issue on your end. I'll continue iterating on that and making it cleaner so we can properly upstream it to fj-contrib.

With your present commentary and feeling OK about the wheels I'll merge so we can get this all shipped.

I'll follow up in the coming month or so for the rest!