Add a Python wheel build for the imath module (#432) - #596
mshooter-ilm wants to merge 4 commits into
Conversation
a844f88 to
413a951
Compare
…n#432) Signed-off-by: Moira Shooter <mshooter@ilm.com>
…demySoftwareFoundation#515), fail-fast off Signed-off-by: Moira Shooter <mshooter@ilm.com>
…SoftwareFoundation#515); /bigobj for imath_module on MSVC Signed-off-by: Moira Shooter <mshooter@ilm.com>
413a951 to
41139ad
Compare
|
This looks great, thanks! The FetchContent looks like a cleaner approach than what I was imagining. Although it adds some complexity directly to the cmake configuration rather than the python wheel CI, that could be generally helpful, since it means anyone can build PyImath without needing to first install Boost.Python. This does introduce a lot of long CI jobs, since Boost.Python gets built for each combination of arch and python version. I did a little investigating, and I think it should be possible to introduce a cache using actions/cache keyed on OS/arch + the pinned Boost version, so the tarball is downloaded/extracted once and reused across the CPython builds. Can you try asking Claude to explore this approach? If it works, great, but if not, the current setup is fine. |
| def equalWithAbsErrorScalar(x1, x2, e): | ||
| return abs(x1 - x2) < e | ||
|
|
||
| # Architectures on which array quaternion operations and the equivalent |
There was a problem hiding this comment.
This fix for the test would be better in an independent PR. It's necessary for the wheel build/test to succeed, but it's a separate issue, so it would be better tracked separately.
Could you move this change to a new branch and submit it as a separate PR?
Also, I would recommend switching the comparison to the fuzzy variety for all platforms, not just the ones that currently fail. After studying it in a bit more depth, I think it's actually a bit arbitrary that the existing bit-exact comparisons happen to work given what's going on (vectorized vs. non-vectorized computations). Just eliminate the platform test in quatEqual.
So:
- copy the changes to pyImathTest.py to a separate brance/commit, eliminate the platform conditional, and submit as a separate PR.
- revert the changes to pyImathTest.py on this branch. That will mean the CI test for this PR will fail, but we'll merge the other PR, then update this branch, and effectively be back where we are now.
Thanks!
…ademySoftwareFoundation#597 Signed-off-by: Moira Shooter <mshooter@ilm.com>
Hi @cary-ilm , thanks for the review.
What would actually make the job short: build the five Python versions in parallel, as separate CI jobs, instead of one after the other on the same machine. Same total work, but the run finishes in about 15 minutes instead of an hour. It's a workflow-only change; the cost is 30 small jobs instead of 6 big ones in the checks list. On top of that, a compile cache (sccache) can make re-runs almost free when the code hasn't changed: I tested it locally and a rebuild went from 222 seconds to 4. That needs a bit more plumbing, so I'd do it as a follow-up PR. Which would you like: the parallel jobs in this PR, the Boost cache, both, or leave it as is? |
|
Thanks! I merged the test pr, so you can rebase the branch on that and pick up the change. Thanks also for the speed analysis. I had the impression that Boost took longer to build, so that's good to know. Let's split it out as 30 jobs, one per pair of architecture/python version. And I don't think the cache is worth it. Imath doesn't change very often, so the run time is not actually all that critical. |
|
And one more thing: you should copy over the python-wheels-publish.yml and python-wheels-publish-test.yml from OpenEXR. Those are the workflows that run when a release candidate is tagged and when a release is published, which actually upload the wheels to PyPI and PyPI.test. Those are hard to test, since you have to fake a release. I'll handle that. There may be some additional setup with the Imath PyPI project. I'll look into that as well. But the first step is to replicate for Imath exactly what OpenEXR does. Thanks! |
Adds a cibuildwheel workflow and
pyproject.tomlfor the Boost.Pythonimathmodule, modeled on OpenEXR's. Boost.Python is downloaded and builtfrom source inside the CMake build (
IMATH_FORCE_INTERNAL_BOOST, pinned toboost-1.92.0), so the wheel is a single self-contained module with no
dependencies. Scope as discussed with @cary-ilm:
imathonly, static; noimathnumpy, no publish workflows.Green on the full OpenEXR matrix (Linux x64/arm64, macOS x86_64/arm64/universal2,
Windows x64/arm64; CPython 3.9–3.13):
https://github.com/mshooter-ilm/Imath/actions/runs/34525829207
Notes for review:
IMATH_FORCE_INTERNAL_BOOST,IMATH_BOOST_URL/IMATH_BOOST_URL_HASH,PYIMATHNUMPY. Defaults leave normal buildsunchanged; CI and CI-vfx pass on this branch.
pyImathTest.py:testQuatArraysuses a small tolerance on arm64-typearchitectures (issue testQuatArrays tests fail on some arches #515), exact elsewhere. Should probably be separate PRs
and submit the tolerance first, and then the wheel.
imathisalready taken by an unrelated package.
In collaboration with Claude AI