Add support for building Python wheels for Linux & MacOS by kylc · Pull Request #1103 · ompl/ompl

kylc · 2023-10-22T21:29:49Z

This PR adds support for building Python wheels for the OMPL package, along with Github CI workflows to do so automatically. The result is that users can install the OMPL Python bindings, with precompiled OMPL shared libraries and all dependencies, like this:

pip install https://github.com/ompl/ompl/releases/download/prerelease/ompl-1.6.0-cp310-cp310-manylinux_2_28_x86_64.whl

Along with just about any CPython version from the cibuildwheel table, supported build targets are:

Linux x86_64 (via manylinux_2_28_x86_64)
MacOS (both x86_64 and arm64)

This is done using the cibuildwheel package to automate building for various operating systems, CPU architectures, and Python versions. On Linux, the official manylinux Docker images are used to ensure glibc backwards compatibility in accordance with standard PyPA packaging rules.

Usage

Users can build wheels locally by installing cibuildwheel and invoking it from the ompl/ root directory like this:

CIBW_BUILD="cp312-manylinux_x86_64" cibuildwheel --platform linux py-bindings/

Correct format of the CIBW_BUILD variable can be found in the cibuildwheel docs. If cross-compiling on MacOS, you must also set OMPL_BUILD_ARCH to the target architecture, e.g. arm64.

Note that cibuildwheel will make changes to your local system on MacOS. On Linux it uses Docker to containerize the build, but on MacOS the commands are performed on the host system. Be careful!

Changes

NOTE: I don't have access to a MacOS development machine, so it's entirely possible that some of the Mac-specific changes below aren't required. Additional testing on that platform would be appreciated.

py-bindings/setup.py is modified to invoke CMake and build the OMPL shared libraries. This allows the cibuildwheel tool (or users) to pip install . to build locally.
Github workflow files are added under .github to automatically run wheel build jobs on new commits.
CMakeLists.txt changes:
- Custom shared library outputs paths are removed to make it easier for setup.py to find the resulting shared libraries
- We remove direct linking to ${PYTHON_LIBRARIES} in accordance with PEP513 recommendations; it will be loaded at runtime anyway.
- When building on Mac, we pass extra linker arguments -undefined suppress -flat_namespace to prevent the linker complaints about missing Python symbols (due to the above).
- When building on Mac, we define _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION=1 to bring back some deprecated stdlib types which are removed in recent XCode versions (see Xcode 15 Release Notes).

Known Issues

CRITICAL: On MacOS, there seems to be some issue with multiple definitions of the same symbol. This can be observed by importing multiple OMPL namespaces and watching for a warning to the effect, e.g. import ompl.base; import ompl.util. These symbols aren't compatible with each other even though it is plain to see that they refer to the same thing. This breaks some Python code with cryptic errors like "expected argument LogLevel, got LogLevel".

RuntimeWarning: to-Python converter for ompl::msg::LogLevel already registered; second conversion method ignored.

Future work

Reformat some of what is written above into a documentation page in the OMPL repository.
Submit to PyPI so that users can simply pip install ompl (or similar).
Build universal2 wheels for MacOS, which bundle both x86_64 and arm64 compatible binaries into a single wheel.
Bundle optional dependencies like FLANN, if worthwhile?
Windows builds?

kylc

Pre-review

kylc · 2023-10-22T21:32:35Z

.github/workflows/wheels.yaml

+          CIBW_BUILD: cp310-macosx_{x86_64,arm64} cp310-manylinux_x86_64
+          # NOTE: Many combinations of OS, arch, and Python version can be built
+          # depending on your patience. For example:
+          # CIBW_BUILD: cp3{10,11,12}-macosx_{x86_64,arm64} cp3{9,10,11,12}-manylinux_x86_64


Which combinations should we build for in CI? It's only a matter of patience and not exceeding Github's CI job timeout.

Maybe:

Linux cp3{10,11,12} to support Ubuntu 22.04+

Mac OS cp12 to match the latest Python version in Brew

Maybe also Linux cp39? Some Noetic environments still use Python 3.9 iirc.

In particular, I know that the RoboStack conda channel (https://robostack.github.io/index.html) uses 3.9 for Noetic.

cp38 and I believe ISAAC Sim is tied to cp37 may be worthwhile targets... but that may be pushing the amount of time we have on the CI job.

cp37-cp312 might be possible. I think the default Github CI limit is 6hrs. I'll hold off until we're done with reviews and then test it out in CI to avoid making the builds take forever during the review process.

Given #1103 (review), is now the time to test the wider range of Python versions?

Yep, I enabled for: cp3{10,11,12}-macosx_{x86_64,arm64} cp3{7,8,9,10,11,12}-manylinux_x86_64. Seems to only take about 2hrs on CI, so plenty of time to spare.

kylc · 2023-10-22T21:33:38Z

CMakeLists.txt


-set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin")
-set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/lib")
-


This might break some existing expectations, but it also makes it easier for the setup.py script to find the resulting binaries.

Perhaps to allow setup.py to override these, they could be wrapped in if(NOT DEFINED ...) checks?

https://github.com/ompl/ompl/pull/1103/files#diff-60f7d754a0c159e4789a1ac359dc6552c6187ec903af6e6c3af43232be7449a8R102-R104

Also want to make sure this doesn't break / interfere with #1101

py-bindings/pyproject.toml

kylc · 2023-10-22T21:37:05Z

py-bindings/setup.py

+
+        if sys.platform.startswith("darwin"):
+            # TODO: Move these out to configuration
+            cmake_args += ["-DCMAKE_CXX_COMPILER=/usr/local/opt/llvm@16/bin/clang++"]


I don't have a Mac machine to test changing this, but this path probably shouldn't be hardcoded. Maybe someone can experiment with setting this via an environmental variable in before_build.sh? Maybe brew provides a method for identifying this path?

I have a Mac to test against, I'll see what needs to be changed here.

kylc · 2023-10-22T21:37:25Z

py-bindings/setup.py

+        if sys.platform.startswith("darwin"):
+            # TODO: Move these out to configuration
+            cmake_args += ["-DCMAKE_CXX_COMPILER=/usr/local/opt/llvm@16/bin/clang++"]
+            cmake_args += ["-DCMAKE_OSX_DEPLOYMENT_TARGET=13.0"]


Not sure how to correctly set this value.

This is typically set to the lowest supported OS version. The host OS needs to have the SDK for that version. Do you need to set this at all?

Without it I was seeing lots of dylib was built for newer macOS version than being linked warnings on the macos-13 Github runner. Maybe it's OK to leave it as-is until someone has a chance to verify that it does indeed build with a lower version.

kylc · 2023-10-22T21:38:47Z

py-bindings/ompl/base/__init__.py

@@ -1,2 +1 @@
-from ompl import util


This change "avoids" the major issue on Mac I mention in the PR description, but doesn't solve it if the user imports ompl.util or other packages themselves.

I didn't notice any negative side effects from removing this.

wbthomason · 2023-10-23T14:13:36Z

Thanks very much for this! I want to tag kylc/ompl-wheels#1 here - I think the critical issue is not only an issue for macOS.

zkingston · 2023-10-23T14:31:41Z

Thanks a ton for making a PR for this! I would like to make sure #1100 gets merged first as I believe it fixes some of the issues for MacOS and modern Linux systems.

kylc · 2023-10-23T16:37:45Z

Thanks for testing it out @wbthomason, apologies for not noticing your post earlier. It seems like there is some odd behavior depending on the ordering of imports. For example, if I import ompl.tools in a fresh interpreter then I get an error like yours, but if I first import ompl.util; import ompl.tools then it runs fine.

Removing the import change here seems to resolve the issue for me, but I thought it was causing some issues on MacOS relating to py-converters already being registered. For now I've reverted this change and I guess we'll see.

If you want to test updated wheels then you can find them on my OMPL fork after CI finishes running: https://github.com/kylc/ompl/tree/build-wheels

Regarding the libboost shared library issue, can you provide some more details about your Linux setup? Did you build the wheels locally or install from the CI job? It seems odd that it's looking for 1.78 since we build a much more recent version and link to that in the CI job. Maybe you can unzip the whl you installed and see which libboost shared libraries are bundled in there?

For example, if I unzip the latest build, I see that ompl.libs/libboost_serialization-15a60f1e.so.1.83.0 exists and ldd shows that OMPL is linking directly to that file.

wbthomason · 2023-10-25T19:45:52Z

I unfortunately deleted the actual .whl file I was testing with before, but I'll try to reproduce the issue in the next few days. In the meanwhile:

I'm on Arch Linux, using Python 3.11.5
I installed the wheel from the CI job on the day I filed the issue, using pip 23.3.1

I'll try re-installing and (assuming the issue repros) unzipping/checking with ldd later this week.

wbthomason · 2023-10-26T04:01:56Z

I did a bit more digging here - I found the original .whl file. The errors in kylc/ompl-wheels#1 may have been red herrings - I had another version of OMPL installed on my system, and this was found first by the call dll_loader('ompl', dirname(abspath(__file__))) in ompl/__init__.py, line 17. This old version of OMPL linked to the old, missing version of Boost.

So, there's still a bug with the library loading, but I don't think it's related to this PR - at the least, I definitely don't think that it should block this.

I can still reproduce the original ompl.tools import error with this old .whl; I cannot reproduce the libboost error after uninstalling my system OMPL.

kylc · 2023-10-31T15:18:03Z

That's good to hear @wbthomason. Still odd that Python doesn't somehow give priority to shared libs distributed in the wheel over an old one on the system path somewhere.

The ompl.tools import issue should be fixed in the latest wheels built in my fork of this repo. For this discussion, the ompl-wheels repo should be considered deprecated.

zkingston · 2023-11-10T21:21:43Z

Before finishing this PR, I'd like to get these issues with building on Ubuntu 20/22 fixed, will require a modest reversion of some of the CMake logic to pull in the old versions of libraries:

It would also be nice to get the PR which fixes compilation for modern builds/Apple M-series chips in as well: #1100, but that can wait until after.

zkingston · 2023-11-28T18:37:50Z

Sorry about the kerfuffle, didn't realize this was set to use the M1 branch as base - that's merged now after some additional testing.

zkingston · 2023-11-28T20:41:57Z

@kylc @wbthomason @mamoll are there any outstanding comments on this pull? If not, let's rebase and merge in.

kylc · 2023-11-30T19:13:09Z

The build is currently broken on main, but is fixed by #1119. For now I've rebased onto that PR and we should merge it first. Otherwise, I think the only items are:

Enable Github Actions on the repo and kick off a job to make sure everything is up and running
Someone with Apple M1 hardware should take a look at the issue in the original PR description

I just pushed up a few changes to only upload wheels for builds on the main branch, fix some issues with the Brew steps on MacOS, and specify pygccxml==2.2.1.

zkingston

Thanks again @kylc for all of your work - I think we are good to merge now.

kylc · 2023-11-30T22:16:12Z

Great! I rebased and enabled builds for all the Python versions we discussed in the comments above. Ship it!

simonschmeisser · 2024-01-31T15:42:58Z

All of them seem to fail since about a month, do you have any idea why?

kylc · 2024-02-02T16:44:37Z

Hey @simonschmeisser. I've noticed that and have a fix sitting on my fork at https://github.com/kylc/ompl, but haven't had time to open a PR. I'll get around to it this weekend I think.

* Modify native build for better MacOS support. * Add wheel generation scripts and CI jobs. * Don't remove ompl.util import from ompl.base. * Don't delete tag every time. * Only tag prerelease if on main branch. * Don't auto-update brew. * Specify pygccxml==2.2.1. * Build all the Python versions!

kylc commented Oct 22, 2023

View reviewed changes

wbthomason mentioned this pull request Oct 26, 2023

Failure to import some submodules kylc/ompl-wheels#1

Open

zkingston mentioned this pull request Oct 26, 2023

Add a CMake export step for config packages. #1067

Merged

5 tasks

zkingston force-pushed the apple-m1-python-bindings branch from 52914ec to cef11bf Compare October 26, 2023 17:58

zkingston force-pushed the apple-m1-python-bindings branch from cef11bf to 4dc9fa7 Compare November 28, 2023 18:06

zkingston deleted the branch ompl:main November 28, 2023 18:35

zkingston closed this Nov 28, 2023

zkingston reopened this Nov 28, 2023

zkingston changed the base branch from apple-m1-python-bindings to main November 28, 2023 18:37

kylc force-pushed the build-wheels branch 2 times, most recently from 7e21cc7 to 33d1e0f Compare November 30, 2023 19:09

kylc mentioned this pull request Nov 30, 2023

Fix for ompl::geometric Python bindings #1119

Merged

zkingston approved these changes Nov 30, 2023

View reviewed changes

kylc added 7 commits November 30, 2023 15:09

Modify native build for better MacOS support.

bf5adb3

Add wheel generation scripts and CI jobs.

1b340c8

Don't remove ompl.util import from ompl.base.

60eaa54

Don't delete tag every time.

acaf71f

Only tag prerelease if on main branch.

433989b

Don't auto-update brew.

d010c0e

Specify pygccxml==2.2.1.

b75f481

Build all the Python versions!

c55be61

kylc force-pushed the build-wheels branch from 33d1e0f to c55be61 Compare November 30, 2023 22:10

zkingston merged commit df058b7 into ompl:main Nov 30, 2023

joao-pm-santos96 mentioned this pull request Dec 5, 2023

Segmentation Fault in Python using Development Build Wheels with existing OMPL installation #1121

Closed

wbthomason mentioned this pull request Dec 12, 2023

PyPI upload for OMPL wheels #1125

Closed

2 tasks


		set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/bin")
		set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/lib")

Conversation

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

Uh oh!

Usage

Changes

Known Issues

Future work

Uh oh!

kylc 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!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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

Uh oh!

Uh oh!

zkingston commented Oct 23, 2023

Uh oh!

kylc commented Oct 23, 2023

Uh oh!

wbthomason commented Oct 25, 2023

Uh oh!

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

Uh oh!

Uh oh!

kylc commented Oct 31, 2023

Uh oh!

zkingston commented Nov 10, 2023

Uh oh!

zkingston commented Nov 28, 2023

Uh oh!

zkingston commented Nov 28, 2023

Uh oh!

kylc commented Nov 30, 2023

Uh oh!

zkingston left a comment

Choose a reason for hiding this comment

Uh oh!

kylc commented Nov 30, 2023

Uh oh!

simonschmeisser commented Jan 31, 2024

Uh oh!

kylc commented Feb 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

kylc commented Oct 22, 2023 •

edited

Loading

kylc Oct 22, 2023 •

edited

Loading

wbthomason commented Oct 23, 2023 •

edited

Loading

wbthomason commented Oct 26, 2023 •

edited

Loading