-
Notifications
You must be signed in to change notification settings - Fork 29
CI/CD Overhaul #143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CI/CD Overhaul #143
Conversation
d822914 to
e30983b
Compare
|
@karosc Nice work! |
- implement stable abi (>=3.9)
- migrate to scikit-build-core
- update cibuildwheel to use native builds for all archs
- move shared libs into package dir
- control stable abi compile definition with env var
e30983b to
4b50572
Compare
|
So I ran the following benchmark simulations on linux (debian trixie) using a medium size (1167/1151 links/nodes) RTK SWMM model with a month long simuation:
I attached the two scripts that run the model with and without swmm-toolkit getter/setter calls to show the extent of the getter and setter usage (lots of calls). Below is a table showing the runtime in seconds of each simulation in triplicate:
It seems that adding the ABI does add some amount of overhead, slowing the simulation down by 3%. That being said, the standard deviation in ABI CTL runtime (27s) is about half the average slowdown (45s). That, and I think we`re doing heck of a lot of getting and setting in these sims, more than the typical workload......so we may still call this a win. Right now this compilation of the stable ABI targets python 3.9 and these simulations were all run with 3.11. I wanted to check if targeting newer versions of the stable ABI would increase performance. In other words, if I increase the Py_LIMITED_API compile definition, does the stable ABI increase in performance? Below is a table with those added simulations. TLDR, it does not affect things.
|
ebf4676 to
cbf133a
Compare
|
Build wheels workflow: https://github.com/karosc/swmm-python/actions/runs/17532877034 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @karosc! I've said it before and I'll say it again, moving this build to Python's stable ABI is a big win for simplicity! I thought your performance analysis was very thoughtful. The degradation observed is expected and acceptable.
The only question I have is, what about the source distribution? We probably ought to post one when we publish to pypi.
Thanks for your work on this! It's greatly appreciated
There are three components to this PR. This may be quite a substantial set of changes to stomach, so consider this branch experimental until we get thorough testing of performance and stability from the maintainers. 🧪🧪
scikit-build-core migration
This is to move away from the deprecated build system scikit-build and into the more modern scikit-build-core backend that leverages pyproject.toml metadata. This largely amounted to moving most of our setup.py into a pyproject.toml file. However, there was one other change made to better accommodate the newer build-system:
__init__.pyfor the windows platform is no longer necessaryStable ABI Overhaul
This explores restricting our build process to the python stable ABI. The main benefit of restricting to the stable ABI is we only need one wheel per system and architecture, it does not matter what python version you have. This means that future releases of python will be compatible with past releases of swmm-toolkit! 🥳🥳
Theoretically, all one needs to do to enable the stable api is set the compile time variable
Py_LIMITED_API=0x03090000to constrain our python api calls to those that are available in python>=3.9. However, in reality there were several other changes that were required:The python wrapper extension had to be modified to use abi3 instead of Python3_SOABI....with the exception of windows, which does not seem to support the abi3 extension and consequently has none. Using
_solver.abi3.pydyielded the following error:ImportError: cannot import name '_solver' from 'swmm.toolkit' (C:\Users\stanley\Downloads\swmm\.venv\Lib\site-packages\swmm\toolkit\__init__.py)Removing the.abi3component of the file name resolved the issue.Remove explicit linking of python when using MSVC. When
$<$<BOOL:$<C_COMPILER_ID:MSVC>>:Python3::Module>, cmake does not link to the stable ABI libpython3.liband instead forces a link to the version-specific libUpdates to the build_wheel.yml workflow
Since I last touched this file, github has released native arm64 runners for both macos and linux. I modified this workflow to use the native runners rather than cross compile. Additionally, since this branch uses the python stable ABI, it only builds once for each platform, but runs the unit tests for each version of python 3.9 through 3.14 to check compatibility.
Other notes
openmp libraries are now explicitly included in the distribution since I don't expect all our users to have the dlls. Consequently, I also am explicitly including the openmp-llvm license in our wheel to be safe.
If you don't want to compile a stable ABI binary and instead want to target your own python version (perhaps for optimizations?) just build the project with the environment variable
NO_STABLE_ABI=0