Skip to content

adding python wheel builder#23

Open
shimwell wants to merge 25 commits intousing-sci-kit-buildfrom
building_wheels_2
Open

adding python wheel builder#23
shimwell wants to merge 25 commits intousing-sci-kit-buildfrom
building_wheels_2

Conversation

@shimwell
Copy link
Owner

@shimwell shimwell commented Dec 13, 2022

this new github action should build python wheels that contain the openmc executable for different OS versions

@shimwell
Copy link
Owner Author

once wheels are built they can be uploaded to pypi, here is an example
https://github.com/pypa/cibuildwheel/blob/main/examples/github-deploy.yml

@shimwell
Copy link
Owner Author

The wheels are not building and the error causing the failure appears to be related to cmake not finding hdf5

Could NOT find HDF5 (missing: HDF5_LIBRARIES HDF5_INCLUDE_DIRS
          HDF5_HL_LIBRARIES C HL) (found version "")

@shimwell shimwell mentioned this pull request Dec 14, 2022
@hassec
Copy link

hassec commented Dec 15, 2022

@shimwell I've been playing around a bit locally by using CIBW_BUILD_VERBOSITY=3 cibuildwheel --only cp310-manylinux_x86_64

Just dumping my findings here in case they are useful

I can get one step further by installing hdf5-devel and then cmake finds hdf5.

diff --git a/pyproject.toml b/pyproject.toml
index f980ce49a..a58988c10 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -17,3 +17,4 @@ skip = ["cp36-*"]
 
 [tool.cibuildwheel.linux]
 skip = ["cp36-*"]
+before-all = "yum install hdf5-devel -y"

I'm not sure if there is a way to use the pip install of h5py but I don't think that would come with the headers for HDF5.
And the default FindHDF5.cmake works by looking for the h5cc or h5pcc binary.
So for that cmake module to work, one needs a proper hdf5 install.

The above patch only finds hdf5 though, but for that specific docker image, the installed version is 1.8.12 which is too old.
In this case it fails because it can't find the H5free_memory function which was only added in 1.8.13... Sooo close 😅

So one might have to build hdf5 from source. But maybe one could just reuse what h5py does ?

@shimwell
Copy link
Owner Author

Thanks for taking a look, perhaps we can update the docker image

@shimwell
Copy link
Owner Author

shimwell commented Dec 15, 2022

the macos 11 build was failing so I removed it for now

it was failing with the familar

Could NOT find HDF5 (missing: HDF5_LIBRARIES HDF5_INCLUDE_DIRS
          HDF5_HL_LIBRARIES C HL) (found version "")

since removing one matrix entry the CI appears to have got confused when I removed the mac os

it appears to have worked for linux

@hassec
Copy link

hassec commented Dec 16, 2022

I looked a bit more into how h5py does it, and they actually provide a docker image that is manylinx + hdf5 -> https://github.com/h5py/hdf5-manylinux

They use that to build their Linux wheels, but for macos they build HDF5 from source. So I imagine that openmc would have to do the same.

Instead of adding the yum command I've tried their docker image by using:

diff --git a/pyproject.toml b/pyproject.toml
index f980ce49a..db49ba225 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -17,3 +17,5 @@ skip = ["cp36-*"]
 
 [tool.cibuildwheel.linux]
 skip = ["cp36-*"]
+# before-all = "yum install hdf5-devel -y"
+manylinux-x86_64-image = "ghcr.io/h5py/manylinux2014_x86_64-hdf5"

and that allowed me to build a python 3.10 wheel via CIBW_BUILD_VERBOSITY=3 cibuildwheel --only cp310-manylinux_x86_64

Using that wheel in a plain docker image to test it, threw an error on openmc.run().
That's because executor.py assumes that openmc is just on the path. But that's not true in this case. The wheel includes the executable, in ..../python3.10/site-packages/openmc/bin, so an extra export PATH=... fixed it. Maybe the executor.py script should be adapted to first look for an exectuable inside it's packaged python env, before falling back to a plain openmc and hoping that finds something?

That would also make it safer to have e.g. different versions of openmc installed. Because right now I can use any openmc python version, but the actual openmc executable used is the one that is found by the system PATH resolution.

@shimwell
Copy link
Owner Author

Thanks for looking into this @hassec I've added your suggestion on the docker image.

For the access to the executable from the command line we could do something like pdfx does with a combination on console_scripts in the setup and argparse usage in a python launcher
https://github.com/metachris/pdfx/blob/9e6864c5f9bcc8801e12c63a64d6efdfd1960494/setup.py#L109-L113

https://github.com/metachris/pdfx/blob/master/pdfx/cli.py

But perhaps there are other solutions

@shimwell
Copy link
Owner Author

perhaps the easy option is to add bin/openmc to the scripts arg
https://stackoverflow.com/questions/36364610/python-module-distribution-installs-executable-to-path

@shimwell
Copy link
Owner Author

Latest error is with the py3.11 build on Mac

setup.py:63: RuntimeWarning: NumPy 1.21.1 may not yet support Python 3.11.

@hassec
Copy link

hassec commented Dec 19, 2022

Seems like the that old numpy version is only picked up because the pyproject.toml pins numpy<1.22. Is there a specific reason why that is required?

I think I've a solution for the bin/openmc problem, but need to cleanup before posting a diff, hopefully later today 😉

edit: @shimwell I've opened #24 that includes the mentioned changes.

@shimwell
Copy link
Owner Author

I think the numpy version is limited due to some tests failing with the newer versions of numpy.

openmc-dev#2190

I guess openmc will move to the latest numpy version at some point so if it is causing issues here then we can change it.

@shimwell
Copy link
Owner Author

Also wanted to mention the latest commit from this PR removes the numpy limitation and aims to get the CI working for the latest version of numpy

Co-authored-by: Christoph Hasse <hassec@users.noreply.github.com>
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.

2 participants