Python wheels #2731

akoen · 2023-10-17T18:14:22Z

Description

Adds a CI step to build python wheels with cibuildwheel. Currently only builds for CPython 3.11 but can be expanded to encompass 3.7--3.12 with a one-line change.

Discussion at: https://openmc.discourse.group/t/alpha-python-wheels-for-openmc-including-apple-sillicon/3518

Checklist

I have performed a self-review of my own code
I have run clang-format on any C++ source files (if applicable)
I have followed the style guidelines for Python source files (if applicable)
I have made corresponding changes to the documentation (if applicable)
I have added tests that prove my fix is effective or that my feature works (if applicable)

fix: typo fix: typo fix: dir fix fix fix fix fix fix ci: add libpng ci: add libpng fix

shimwell · 2023-10-17T19:49:59Z

I'm sure @hassec will be keen to take a look at this PR

hassec

@shimwell thanks for tagging me!😊
And great to see this PR @akoen!

I think its worth considering to enhance the setup.py such that a pip install correctly builds and installs the project.
I'd suggest to add a job to test these wheels as well to ensure that everything is packaged & relocated correctly.

hassec · 2023-10-18T19:15:25Z

.github/workflows/build-wheels.yml

+          CIBW_BUILD: "cp311-${{ matrix.cibw-arch }}"
+          CIBW_ARCHS_MACOS: x86_64 arm64
+          CIBW_ARCHS_LINUX: x86_64
+          CIBW_MANYLINUX_X86_64_IMAGE: manylinux_2_28
+          CIBW_ENVIRONMENT_LINUX: LD_LIBRARY_PATH="/usr/local/lib64:${LD_LIBRARY_PATH}" PIP_ONLY_BINARY="numpy"
+          CIBW_BEFORE_ALL_LINUX: |


Have you considered moving the cibuildwheel configuration into the pyproject.toml?
I've found that it makes testing the build locally easier.

hassec · 2023-10-18T19:16:53Z

.github/workflows/build-wheels.yml

@@ -0,0 +1,63 @@
+name: Build Wheels
+
+on: [push, workflow_dispatch]


Might just be because it's still draft but this probably doesn't need to run on every push. I'd suggest to run on every tag creation.

hassec · 2023-10-18T19:27:47Z

CMakeLists.txt

+add_custom_command(TARGET openmc POST_BUILD
+  COMMAND ${CMAKE_COMMAND} -E copy
+  $<TARGET_FILE:openmc>
+  ${CMAKE_CURRENT_SOURCE_DIR}/openmc/lib/$<TARGET_FILE_NAME:openmc>
+  COMMENT "Copying openmc binary to Python module directory")
+


I know that the libopenmc library already gets relocated into the source tree, but IMHO that's always felt a bit "unclean" to put build artifacts into the source tree.

This might be a chance to move away from that approach.
If you are interested, in my attempt at creating a pip package I had made changes to the setup.py to include the build step by implementing a setuptools.Extension for OpenMC which builds in a temporary directory.

That's one way, I'm sure there is more 🤷

Yes, what we have currently for libopenmc is very hacky, so I would not advocate for doubling down on it and handling the executable the same way. The approach you have there @hassec sounds a lot better. The other option that comes to mind is to use scikit-build (see #916). It looks like there are some other build backends out there that might merit a look.

FWIW, @shimwell had a version that tried to use scikit-build, and I started out with that approach in the beginning but couldn't really get it to work as well as with the setuptools.Extension which was more flexible and easier to setup as scikit-build had limited docs. But there as been a lot of recent activity on the new scikit-build-core.

I don't have any experience with that package yet.

I recently started tinkering with scikit build core (which I think now replaces scikit-build) on branch if anyone is interested develop...shimwell:openmc:trying-scikit-core

hassec · 2023-10-18T19:30:18Z

scripts/openmc

+import sysconfig
+import sys
+
+os.execv(sysconfig.get_path("platlib") + "/openmc/lib/openmc", sys.argv)


Didn't know about this before. I was doing this with Popen, but this is much neater 👍

akoen · 2023-10-19T01:43:49Z

Thanks @hassec and @paulromano for the feedback! I hadn't realized that others had made similar efforts. A few thoughts:

This is very much a proof-of-concept. My main goal was just to build wheels for macos arm64 since some of my colleagues were having trouble building from source. Hence the big focus on cross-compilation.
I deliberately avoided building openmc itself in setup.py for a few reasons.
1. Those with an existing system-wide installation of openmc might not want to install it locally just to install the python package in a new virtual environment.
2. When cibuildwheel builds wheels for many python versions in a single workflow, it's much faster to only build openmc once instead of for each python version
3. I think it makes sense to include DAGMC by default in a python wheel, and that + MOAB must be installed independently regardless.
scikit-build is likely the cleanest way to do this, but I had trouble getting cross-compilation working and got the setuptools based approach working first.

I'm happy to clean this PR up and address some of this feedback, but a rework with scikit-build is likely not something I'll have time for since I'm nearing the end of my internship.

hassec · 2024-02-05T14:10:16Z

Had a watch on an issue (pypa/auditwheel#340) over at auditwheel that is relevant to how packaging works for e.g. the openmc executable. So just wanted to leave an FYI here that this was recently fixed via pypa/auditwheel#443 in case it's helpful once this or a follow up PR gets going again

MicahGale · 2024-03-28T17:32:42Z

scikit-build is likely the cleanest way to do this, but I had trouble getting cross-compilation working and got the setuptools based approach working first.

Could you expand on this? Were not able to get it work on all environments that you asked for, or are you trying to run on one platform and build for another? If it's the latter I think just throwing different GHA runners at it should be good enough.

jon-proximafusion · 2024-07-03T15:03:45Z

build-deps.sh

+# 5.5.0 breaks with pyne std::isnan bug
+MOAB_TAG='5.3.0'


5.5.1 has been released, might fix the pyne std::isnan bug

Suggested change

# 5.5.0 breaks with pyne std::isnan bug

MOAB_TAG='5.3.0'

MOAB_TAG='5.5.1

jon-proximafusion · 2024-07-03T15:04:15Z

build-deps.sh

+MOAB_TAG='5.3.0'
+MOAB_REPO='https://bitbucket.org/fathomteam/moab/'
+
+DAGMC_TAG='v3.2.2'


another version bump

Suggested change

DAGMC_TAG='v3.2.2'

DAGMC_TAG='v3.2.3'

MicahGale · 2024-07-03T20:33:48Z

FYI: I was looking into how lxml builds wheels since they use a compiled backend and just seem to always work. It turns out they produce around 20+ wheels.

Looking at their GHA it is actually pretty straightfoward-ish. They most rely on cibuildwheel, which seems to do most of the work for them.

shimwell · 2024-07-09T15:36:41Z

Great to catch up at the meshing group @akoen

I just wanted to make note of something paulromano found.

This pypi package includes Python bindings for the MPI and includes MPICH binaries which is something we had not noticed before. Perhaps the first pypi package to include mpi binaries
https://pypi.org/project/mpi4py-mpich/

I notice this nice wheel you have made does not include MPI but perhaps with this package it might be able to

ci: build wheels

f9d4cd5

fix: typo fix: typo fix: dir fix fix fix fix fix fix ci: add libpng ci: add libpng fix

akoen force-pushed the akoen/wheels branch from 3d8d44b to f9d4cd5 Compare October 17, 2023 18:26

akoen changed the title ~~Akoen/wheels~~ Python wheels Oct 17, 2023

style: use single quotes

589eb14

hassec reviewed Oct 18, 2023

View reviewed changes

jon-proximafusion reviewed Jul 3, 2024

View reviewed changes

		@@ -0,0 +1,63 @@
		name: Build Wheels

		on: [push, workflow_dispatch]

	# 5.5.0 breaks with pyne std::isnan bug
	MOAB_TAG='5.3.0'
	MOAB_TAG='5.5.1

Python wheels #2731

Are you sure you want to change the base?

Python wheels #2731

Uh oh!

Conversation

akoen commented Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

shimwell commented Oct 17, 2023

Uh oh!

hassec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akoen commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hassec commented Feb 5, 2024

Uh oh!

MicahGale commented Mar 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MicahGale commented Jul 3, 2024

Uh oh!

shimwell commented Jul 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

akoen commented Oct 17, 2023 •

edited

Loading

akoen commented Oct 19, 2023 •

edited

Loading