netcdf4-python icon indicating copy to clipboard operation
netcdf4-python copied to clipboard

add cibuildwheel GHA

Open ocefpaf opened this issue 1 year ago • 21 comments

This is just a proof of concept to see if we can build wheels here. We probably want to build out own hdf5 and netcdf-c b/c the ones from CentOS are quite old probably the source of the failures we are seeing in the tests here. See https://github.com/MacPython/netcdf4-python-wheels/blob/master/config.sh#L13-L26 for the versions used in the muiltbuild.

xref.: #1204

ocefpaf avatar Oct 16 '23 17:10 ocefpaf

Ubuntu wheels are using netcdf 4.3.3.1 and hdf5 1.8.12 is failing with:

 netCDF4 v1.6.5.
  .......E..........F.........s........................s...........................................
  ======================================================================
  ERROR: test_get (tst_fancyslicing.VariablesTestCase)
  testing 'fancy indexing'
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/project/test/tst_fancyslicing.py", line 147, in test_get
      data = v[ibx,iby,ibz]
    File "src/netCDF4/_netCDF4.pyx", line 4948, in netCDF4._netCDF4.Variable.__getitem__
    File "/tmp/tmp.QW23YtRUJ7/venv/lib/python3.8/site-packages/netCDF4/utils.py", line 429, in _StartCountStride
      start[...,i] = np.apply_along_axis(lambda x: e*x, i, np.ones(sdim[:-1]))
    File "<__array_function__ internals>", line 200, in apply_along_axis
    File "/tmp/tmp.QW23YtRUJ7/venv/lib/python3.8/site-packages/numpy/lib/shape_base.py", line 376, in apply_along_axis
      raise ValueError(
  ValueError: Cannot apply_along_axis when any iteration dimensions are 0
  
  ======================================================================
  FAIL: runTest (tst_masked.PrimitiveTypesTestCase)
  testing auto-conversion of masked arrays and packed integers
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/project/test/tst_masked.py", line 164, in runTest
      assert var2[:].mask.any() == False
  AssertionError

macOS is passing with the libs hdf5 1.14.2 and netcdf 4.9.2_1

ocefpaf avatar Oct 16 '23 17:10 ocefpaf

One option, I'm not sure if it's the best:

What about setting the cibuildwheel image to use manylinux_2_28? manylinux2014 is based on CentOS 7, which has less than a year left before its end-of-life (2024-06-30). manylinux_2_28 is based on Alma Linux 8, which uses netCDF 4.7.0.

But, in the existing wheel generation process (https://github.com/MacPython/netcdf4-python-wheels), netCDF and its dependencies are compiled from source, so currently netCDF 4.9.2 is being bundled for the wheels on PyPI.

So maybe a better option is to follow suit and build the dependencies from source in here as well instead of using the ancient versions in the EPEL 7 repos.

EDIT: I didn't read in the original description that you already suggested building the dependencies from source

richli avatar Oct 16 '23 23:10 richli

I've done something similar for my nc-complex package: CI workflow, cibuildwheel config in the pyproject.toml, example workflow run

I used manylinux_2_28 in order to get the more recent HDF5, and then only had to build netcdf-C from source.

Note that I use my fork of netcdf-C because I'm waiting for PR #2762 to fix finding HDF5 correctly from CMake. Once that's in, using the lastest main of netcdf-C would be good because it will have compatibility with h5netcdf.

ZedThree avatar Oct 19 '23 12:10 ZedThree

I'll try to focus on building hdf5 and netcdf-c over the next week There are a few options I want to try:

  1. Use a more modern manylinux image and try the system libraries again. Advantage: easy. Disadvantage: little to no control over the hdf5 and netcdf-c;
  2. Build using conda-forge packages for hdf5 and netcdf-c. This GitHub gave me this idea. Advantage: looks easier than the alternative 3 but harder than 1, uses community packages* that are curated by many maintainers who are also active here. Disadvantage: not 100% imediate control over the c-libs b/c someone needs to send a PR to conda-forge first;
  3. Build both hdf5 and netcdf-c from source here. Advantage: 100% control over the builds. Disadvantage: hard to implement and maintain (IMO).

As a conda-forge founder, core member, and maintainer of many geo/met/ocean packages, I'm heavily biased towards 2. What do you think @ZedThree, @jswhit, and @dopplershift?

* we already do something similar for the Windows packages and I want to move the current Windows builds to this, regardless of the Linux and macOS choice, to keep things in a single repository.

ocefpaf avatar Oct 20 '23 14:10 ocefpaf

I suggest using manylinux_2_28 with the system HDF5 (which is 1.10) and netcdf-C from source (because the system version is only 4.7.0).

I would definitely try avoiding building HDF5 from scratch if possible, as you might find you end up building a lot more of the stack!

ZedThree avatar Oct 20 '23 15:10 ZedThree

I suggest using manylinux_2_28 with the system HDF5 (which is 1.10) and netcdf-C from source (because the system version is only 4.7.0).

Looks like their hdf5 has libaec, that is a good start. I recall some issues with openssl in the past and that required folks to compiling their own. I'm not sure if that is still true. Let's call this suggestion would be "easy" to implement b/c I can copy your setup but would give 100% control only over the netcdf-c library. Somewhere between options 1 and 3.

ocefpaf avatar Oct 20 '23 15:10 ocefpaf

What about the approach of using conda built libs? (https://github.com/ocefpaf/netcdf4-win-wheels/pull/6)

If you want to enable all the features of hdf5/netcdf-c it requires a lot of dependencies to be built - don't think we want to do all that here.

jswhit avatar Oct 20 '23 15:10 jswhit

What about the approach of using conda built libs? (ocefpaf/netcdf4-win-wheels#6)

If you want to enable all the features of hdf5/netcdf-c it requires a lot of dependencies to be built - don't think we want to do all that here.

Option 2 is a "modern" version of that where the main advantage is that it uses cibuildwheel+conda-forge clibs, and keep things in this repo, no need for third party ones. I'll try that and see if it works.

ocefpaf avatar Oct 20 '23 16:10 ocefpaf

Windows wheels using conda package is working 🎉

I'll try the same for Linux and macOS next week. Time to clock out a bit.

ocefpaf avatar Oct 20 '23 23:10 ocefpaf

@jswhit everything is passing but the devil is in the details as usual. I'd love for more eyes here. We could also try to consolidate some of the many environment variables and options, netcdf-c version, etc but I prefer to leave that for another PR.

ocefpaf avatar Oct 23 '23 14:10 ocefpaf

This looks great! Thank you @ocefpaf and @ZedThree. I'd like to get this merged soon so we can get a 1.6.5 release out with 3.12 wheels. The old MacPython wheel building repo is broken, and it looks like the multibuild package it relies on is no longer actively maintained.

jswhit avatar Oct 23 '23 23:10 jswhit

The old MacPython wheel building repo is broken, and it looks like the multibuild package it relies on is no longer actively maintained.

Just to say - for the record - we (the multibuild maintainers) think the build is broken because your multibuild is not up to date (see https://github.com/MacPython/netcdf4-python-wheels/issues/23) - and we are maintaining multibuild - but no complaints from us for switching away.

matthew-brett avatar Oct 24 '23 07:10 matthew-brett

This looks great! Thank you @ocefpaf and @ZedThree. I'd like to get this merged soon so we can get a 1.6.5 release out with 3.12 wheels.

Things we need to consider before shipping these wheels (we can merge it and keep iterating on them though):

  1. Size of the wheels, are they larger?
  2. Plugins and untested features, while the tests are passing do wheels users expect any bells and whistles in the hdf5/netcdf-c that is in multi-build but not here?
  3. Does the build matrix here* covers the same builds as in the multi-build setup?

* Here we have:

  • Linux: manylinux_2_28, system hdf5 1.10.5 4.el8 and custom netcdf-c v4.9.2
  • Windows is virtually the same as we had before
  • macOS: both arm64 and x86_64 are built but as separate wheels, no unversal2, to reduce the file size, arm64 are untested b/c we are cross-compiling and we do not have access to M1/M2 machines
  • All are built for Pythons 3.8, 3.10, 3.11, and 3.12.

This PR:

netCDF4-1.6.5-cp310-cp310-macosx_10_9_x86_64.whl 2.8M
netCDF4-1.6.5-cp310-cp310-macosx_11_0_arm64.whl 596K
netCDF4-1.6.5-cp310-cp310-manylinux_2_28_x86_64.whl 9.2M
netCDF4-1.6.5-cp310-cp310-win_amd64.whl 6.6M
netCDF4-1.6.5-cp311-cp311-macosx_10_9_x86_64.whl 2.8M
netCDF4-1.6.5-cp311-cp311-macosx_11_0_arm64.whl 598K
netCDF4-1.6.5-cp311-cp311-manylinux_2_28_x86_64.whl 9.5M
netCDF4-1.6.5-cp311-cp311-win_amd64.whl 6.6M
netCDF4-1.6.5-cp312-cp312-macosx_10_9_x86_64.whl 2.8M
netCDF4-1.6.5-cp312-cp312-macosx_11_0_arm64.whl 588K
netCDF4-1.6.5-cp312-cp312-manylinux_2_28_x86_64.whl 9.4M
netCDF4-1.6.5-cp312-cp312-win_amd64.whl 6.6M
netCDF4-1.6.5-cp38-cp38-macosx_10_9_x86_64.whl 2.8M
netCDF4-1.6.5-cp38-cp38-macosx_11_0_arm64.whl 590K
netCDF4-1.6.5-cp38-cp38-manylinux_2_28_x86_64.whl 9.3M
netCDF4-1.6.5-cp39-cp39-macosx_10_9_x86_64.whl 2.8M
netCDF4-1.6.5-cp39-cp39-macosx_11_0_arm64.whl 596K
netCDF4-1.6.5-cp39-cp39-manylinux_2_28_x86_64.whl 9.2M
netCDF4-1.6.5-cp39-cp39-win_amd64.whl 6.6M

With multi-build

netCDF4-1.6.4-cp310-cp310-macosx_10_9_x86_64.whl 6.7M
netCDF4-1.6.4-cp310-cp310-macosx_11_0_arm64.whl 3.2M
netCDF4-1.6.4-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl 5.0M
netCDF4-1.6.4-cp310-cp310-manylinux_2_17_i686.manylinux2014_i686.whl 5.3M
netCDF4-1.6.4-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 5.4M
netCDF4-1.6.4-cp310-cp310-win_amd64.whl 6.6M
netCDF4-1.6.4-cp311-cp311-macosx_10_9_x86_64.whl 6.7M
netCDF4-1.6.4-cp311-cp311-macosx_11_0_arm64.whl 3.2M
netCDF4-1.6.4-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl 5.0M
netCDF4-1.6.4-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 5.4M
netCDF4-1.6.4-cp311-cp311-win_amd64.whl 6.6M
netCDF4-1.6.4-cp38-cp38-macosx_10_9_x86_64.whl 6.7M
netCDF4-1.6.4-cp38-cp38-macosx_11_0_arm64.whl 3.2M
netCDF4-1.6.4-cp38-cp38-manylinux_2_17_i686.manylinux2014_i686.whl 5.3M
netCDF4-1.6.4-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 5.4M
netCDF4-1.6.4-cp38-cp38-win_amd64.whl 6.6M
netCDF4-1.6.4-cp39-cp39-macosx_10_9_x86_64.whl 6.7M
netCDF4-1.6.4-cp39-cp39-macosx_11_0_arm64.whl 3.2M
netCDF4-1.6.4-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl 5.1M
netCDF4-1.6.4-cp39-cp39-manylinux_2_17_i686.manylinux2014_i686.whl 5.3M
netCDF4-1.6.4-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 5.4M
netCDF4-1.6.4-cp39-cp39-win_amd64.whl 6.6M

Edit:

One can download the latest wheel build from this PR in https://github.com/ocefpaf/netcdf4-python/actions/runs/6617950814

The size of those macosx arm64 builds worries me but I cannot test them. @dopplershift do you have a M1/M2 machine? Could you test those?

ocefpaf avatar Oct 24 '23 13:10 ocefpaf

1.6.5 multi-build wheels now available

jswhit avatar Oct 24 '23 21:10 jswhit

1.6.5 multi-build wheels now available

Awesome! I'm not confident in switching to this just yet. I'll work on reducing the file size and test the macOS builds in the days. BTW, I did test both Linux and Windows they are working as expected.

ocefpaf avatar Oct 24 '23 21:10 ocefpaf

The old MacPython wheel building repo is broken, and it looks like the multibuild package it relies on is no longer actively maintained.

Just to say - for the record - we (the multibuild maintainers) think the build is broken because your multibuild is not up to date (see MacPython/netcdf4-python-wheels#23) - and we are maintaining multibuild - but no complaints from us for switching away.

Apologies @matthew-brett - as expected, it was simply a dumb mistake on my part. I really appreciate all the work you've done on multi-build, we've used it here for a long time.

jswhit avatar Oct 24 '23 22:10 jswhit

@jswhit - no problem - and I'm sure I speak for @mattip too when I say we take absolutely no offence if you'd like to switch to cibuildwheel.

matthew-brett avatar Oct 24 '23 22:10 matthew-brett

1.6.5 multi-build wheels now available

Awesome! I'm not confident in switching to this just yet. I'll work on reducing the file size and test the macOS builds in the days. BTW, I did test both Linux and Windows they are working as expected.

@ocefpaf The 3.12 wheel is working fine here on my M2 in my 3.12 environment. Not sure what's up with the file size.

1.6.5 multi-build wheels now available

@jswhit Anything beyond getting things well-tested before these wheels appear on PyPI?

dopplershift avatar Oct 25 '23 00:10 dopplershift

@ocefpaf So while my MetPy test suite passed with no unexpected failures, I see the reason for the difference in file sizes:

❯ unzip -l netCDF4-1.6.5-cp312-cp312-macosx_10_9_x86_64.whl
Archive:  netCDF4-1.6.5-cp312-cp312-macosx_10_9_x86_64.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
     2679  10-24-2023 19:23   netCDF4-1.6.5.dist-info/RECORD
     1056  10-24-2023 19:23   netCDF4-1.6.5.dist-info/LICENSE
      111  10-24-2023 19:23   netCDF4-1.6.5.dist-info/WHEEL
      116  10-24-2023 19:22   netCDF4-1.6.5.dist-info/entry_points.txt
        8  10-24-2023 19:22   netCDF4-1.6.5.dist-info/top_level.txt
     1784  10-24-2023 19:23   netCDF4-1.6.5.dist-info/METADATA
  1558672  10-24-2023 19:23   netCDF4/_netCDF4.cpython-312-darwin.so
     1455  10-24-2023 19:04   netCDF4/__init__.py
    37383  10-24-2023 19:04   netCDF4/utils.py
   313939  10-24-2023 19:04   netCDF4/_netCDF4.pyx
        0  10-24-2023 19:04   netCDF4/plugins/empty.txt
   173216  10-24-2023 19:23   netCDF4/.dylibs/libhdf5_hl.200.dylib
   259024  10-24-2023 19:23   netCDF4/.dylibs/libidn2.0.dylib
  1886752  10-24-2023 19:23   netCDF4/.dylibs/libnetcdf.19.dylib
    71504  10-24-2023 19:23   netCDF4/.dylibs/libaec.0.dylib
   583248  10-24-2023 19:23   netCDF4/.dylibs/libssl.3.dylib
    88112  10-24-2023 19:23   netCDF4/.dylibs/libbrotlidec.1.1.0.dylib
  4425184  10-24-2023 19:23   netCDF4/.dylibs/libcrypto.3.dylib
   168128  10-24-2023 19:23   netCDF4/.dylibs/libbrotlicommon.1.1.0.dylib
    52688  10-24-2023 19:23   netCDF4/.dylibs/libsz.2.dylib
   162352  10-24-2023 19:23   netCDF4/.dylibs/libintl.8.dylib
   154496  10-24-2023 19:23   netCDF4/.dylibs/libzip.5.5.dylib
   218032  10-24-2023 19:23   netCDF4/.dylibs/libnghttp2.14.dylib
   645824  10-24-2023 19:23   netCDF4/.dylibs/libcurl.4.dylib
   201664  10-24-2023 19:23   netCDF4/.dylibs/liblzma.5.dylib
  1848896  10-24-2023 19:23   netCDF4/.dylibs/libunistring.5.dylib
  3960848  10-24-2023 19:23   netCDF4/.dylibs/libhdf5.200.dylib
   150320  10-24-2023 19:23   netCDF4/.dylibs/librtmp.1.dylib
   867440  10-24-2023 19:23   netCDF4/.dylibs/libzstd.1.5.5.dylib
  1652608  10-24-2023 19:23   netCDF4/.dylibs/libblosc.1.21.1.dylib
---------                     -------
 19487539                     30 files
❯ unzip -l netCDF4-1.6.5-cp312-cp312-macosx_11_0_arm64.whl
Archive:  netCDF4-1.6.5-cp312-cp312-macosx_11_0_arm64.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
     1550  10-24-2023 19:42   netCDF4-1.6.5.dist-info/RECORD
     1056  10-24-2023 19:42   netCDF4-1.6.5.dist-info/LICENSE
      110  10-24-2023 19:42   netCDF4-1.6.5.dist-info/WHEEL
      116  10-24-2023 19:41   netCDF4-1.6.5.dist-info/entry_points.txt
        8  10-24-2023 19:41   netCDF4-1.6.5.dist-info/top_level.txt
     1784  10-24-2023 19:42   netCDF4-1.6.5.dist-info/METADATA
  1438032  10-24-2023 19:42   netCDF4/_netCDF4.cpython-312-darwin.so
     1455  10-24-2023 19:26   netCDF4/__init__.py
    37383  10-24-2023 19:26   netCDF4/utils.py
   313939  10-24-2023 19:26   netCDF4/_netCDF4.pyx
        0  10-24-2023 19:26   netCDF4/plugins/empty.txt
   171728  10-24-2023 19:42   netCDF4/.dylibs/libhdf5_hl.200.dylib
  1891296  10-24-2023 19:42   netCDF4/.dylibs/libnetcdf.19.dylib
    87616  10-24-2023 19:42   netCDF4/.dylibs/libaec.0.dylib
    68880  10-24-2023 19:42   netCDF4/.dylibs/libsz.2.dylib
   551424  10-24-2023 19:42   netCDF4/.dylibs/libcurl.4.dylib
  3887200  10-24-2023 19:42   netCDF4/.dylibs/libhdf5.200.dylib
  1427664  10-24-2023 19:42   netCDF4/.dylibs/libblosc.1.21.1.dylib
---------                     -------
  9881241                     18 files

Looks like quite a few libraries (like libcrypto, libzstd, liblzma) aren't in the arm64 wheel.

dopplershift avatar Oct 25 '23 00:10 dopplershift

@dopplershift the netcdf-c library included in the multi-build wheels has support for compression plugins (blosc, zstd, lzma, etc), but the plugins themselves are not bundled. Without the plugins, I'm not sure how useful this is - otherwise I think these wheels are functionally equivalent.

I've just uploaded 1.6.5 multi-build wheels to pypi, so there's no rush on this PR

jswhit avatar Oct 25 '23 15:10 jswhit

ocefpaf So while my MetPy test suite passed with no unexpected failures, I see the reason for the difference in file sizes.

Thank for testing! And for figuring out the size difference. I guess that the system hdf5 there is probably the reason for this. I did like the small size though 😁

@jswhit I agree, no rush. I'll try to reduce the Linux wheel size, build hdf5 from source, and check what is needed for the plugins. That last one I don't really know what to do but it will be fun to figure it out.

ocefpaf avatar Oct 25 '23 20:10 ocefpaf

@ocefpaf what's the status of this PR? I'd like to release 1.7.0 soon, and would like to use this instead of multibuild. Having the plugins working is not necessary.

jswhit avatar Jun 07 '24 22:06 jswhit

@ocefpaf what's the status of this PR? I'd like to release 1.7.0 soon, and would like to use this instead of multibuild. Having the plugins working is not necessary.

I can work on it newt Tuesday. (Sorry, looong weekend here in Sweden and no daycare until then.)

If you are ok with the current hdf5, instead of building one from source, then this is virtually ready, I only need to refresh the branch and update the GitHub Actions used.

ocefpaf avatar Jun 09 '24 15:06 ocefpaf

@ocefpaf I'm fine with the current approach for hdf5. Hope you enjoyed the long weekend!

jswhit avatar Jun 10 '24 14:06 jswhit

@ocefpaf I'm fine with the current approach for hdf5. Hope you enjoyed the long weekend!

@jswhit I'm done here! I guess that, the only future change I'd make is to build everything, hdf5, curl, openssl, etc from ourselves. But I'll leave that for the future when we have more time.

ocefpaf avatar Jun 11 '24 07:06 ocefpaf