search

thaler-lab / Wasserstein

Python/C++ library for computing Wasserstein distances efficiently.
https://thaler-lab.github.io/Wasserstein
Other
26 stars 9 forks source link

EnergyFlow HDF5 dependency causes build test fails on Windows and Linux #66

Open j-s-ashley opened 3 weeks ago

j-s-ashley commented 3 weeks ago

Wheel testing on both Windows and Linux currently fails.

This appears to be caused by the EnergyFlow dependency HDF5 being unable to load necessary libraries (errors in Windows and Linux builds).

matthewfeickert commented 3 weeks ago

@j-s-ashley Yeah. For older Pythons h5py didn't ship the underlying hdf5 library, and so for those Pythons it will be necessary to use the cibuildwheel CIBW_BEFORE_ALL option to use the OS specific package manager to install the library before the build.

So below

https://github.com/thaler-lab/Wasserstein/blob/7f658fd6c62d13d9e2f40f6f015fd302bbfc9249/pyproject.toml#L104

something like

[tool.cibuildwheel.linux]
before-all = "yum install -y hdf5"

(which on RHEL Linux distros gets you

$ find /usr/ -type f -iname "libhdf5.so*"
/usr/lib64/libhdf5.so.200.1.0

)

and then the equivalent for Windows. (Off the top of my head I don't remember how to also specify Python version there.)

edit:

er...maybe it is more complicated than that as I can do

$ docker run --rm -ti python:3.7 bash
root@18dbc785ef23:/# python -m venv venv && . venv/bin/activate && python -m pip --quiet install --upgrade pip
(venv) root@18dbc785ef23:/# python -m pip install 'h5py!=3.11.0,>=2.9.0'
Collecting h5py!=3.11.0,>=2.9.0
  Downloading h5py-3.8.0-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (2.5 kB)
Collecting numpy>=1.14.5 (from h5py!=3.11.0,>=2.9.0)
  Downloading numpy-1.21.6-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl.metadata (2.1 kB)
Downloading h5py-3.8.0-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (4.3 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 4.3/4.3 MB 3.6 MB/s eta 0:00:00
Downloading numpy-1.21.6-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl (15.7 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 15.7/15.7 MB 8.7 MB/s eta 0:00:00
Installing collected packages: numpy, h5py
Successfully installed h5py-3.8.0 numpy-1.21.6
(venv) root@18dbc785ef23:/# python -c 'import h5py; print(h5py)'
<module 'h5py' from '/venv/lib/python3.7/site-packages/h5py/__init__.py'>
(venv) root@18dbc785ef23:/#

without issue. So I'm not fully clear on why cibuildwheel is building the wheel, but it seems that if it is going to build it it needs the library installed first.

henryiii commented 3 weeks ago

Maybe force only binary installs? https://github.com/scikit-hep/boost-histogram/blob/69a9399e0d97c7d4e95678715b59232a0f4ee2a3/pyproject.toml#L173-L174

matthewfeickert commented 3 weeks ago

Maybe force only binary installs? https://github.com/scikit-hep/boost-histogram/blob/69a9399e0d97c7d4e95678715b59232a0f4ee2a3/pyproject.toml#L173-L174

That used to not work as POT didn't have Python 3.12 wheels. However, that seems to have changed last week (2024-11-05) with pot v0.9.5 so I think that will work now. :+1:

henryiii commented 3 weeks ago

You could just specify h5py for only binary if needed instead of :all:.

matthewfeickert commented 3 weeks ago

https://github.com/thaler-lab/Wasserstein/pull/67#issuecomment-2469872887