Modernize pyalps build (standalone nanobind wheel build) - #107
skilledwolf wants to merge 49 commits into
Conversation
|
Apologies, I know this PR will be annoying to review, but I didn't see a way to land this in smaller units. Please let me know what you think, and whether the implicit decisions are reasonable / compatible with your long term vision, I'm happy to make any changes! |
skilledwolf
changed the title
|
@egull Could you weigh in on this? Nanobind is a modern replacement for Boost.Python, and it has a STABLE_ABI flag which allows one binary to work across multiple python versions. Under the hood, I believe it simply inserts C-wrappers around Cpython code to make the structs defining core python objects agree (e.g. strips away attributes of a struct so the memory layout is the same between versions). That example was somewhat of a simpilification... but I think this is worth looking into for ALPS 3.0 release. |
|
@marcusr2ML I know too little to have an informed opinion. Boost python is a historical choice, it made sense when these bindings were in their infancy and this was one of the sort-of ok tools on the market. Things have changed a lot so I'm fully supportive of the idea of upgrading to a newer version of bindings. However, I haven't used the python parts recently, i haven't followed the binding developments, and I'm really not informed enough to know what exactly to do. |
|
I guess only one comment: can we delay this change until after the next release is out? |
|
Hi everyone! Delaying until after the 3.0 release would make sense. It's a fairly invasive PR with changes to the source build and the CI/CD pipeline with little impact on the intended users (who are supposed to use spack and pip anyway, source build is likely the exception for the average user). It should have no impact on any usage or functional behavior of the compiled binaries and also no impact on usage or output of the python codes. The main upsides will be easier source build, maintenance and distribution, and eventually more easily adding new interfaces to the python package. I'd strongly advocate that we merge this or a similar PR eventually, but the timing is not crucial. If everyone is on board with the switch to nanobind, I think it can ride along as non-breaking minor release in the 3.x series. PS: No doubt that historically Boost.Python was the way to go, and ALPS handled the python interface and build really well within that context. I think pybind11 has only existed as modern replacement since around 2015 and nanobind since around 2022. |
|
@skilledwolf Waiting makes sense. The source install is riddled with ABI errors so it'd be nice to have. Just putting it on the radar. |
ee7f079 to
59469d3
Compare
|
@skilledwolf Before commiting, we should go through a list I compiled. I listed every issue that could occur when replacing Python.Boost with nanobind, then I had claude go through all of ALPS to find any place in the source where we might hit these landmines. I need to retry using this PR head since you could have fixed these. There were something like 90 issues total though, about 20 being MUST be changed. I am out of tokens till a bit later, so I will rerun then. |
|
@marcusr2ML Yes, the last commits fixed some important oversights I noticed over the last few days (some missing bindings). I'll resolve the conflict later today and might tidy up the new python CI/CD pipe a bit, but it shouldn't affect your plan. The blast radius of this PR is necessarily pretty significant, so I'm looking forward to find out if there's anything I still missed! |
|
@skilledwolf Jumping off from this head, Claude found 43 potential issues that could occur on migrating to nanobind. This is down from the 90 found earlier. I posted the PDFs (for you) and the .md files (for claude). Use "findings" and see if you can query Claude to double check your branch for these issues. I am thinking we can ignore many of these issues till later, focusing on the "hard blockers" and the "silent runtime breakage" sections. |
|
@marcusr2ML Thanks! Most of it is accurate, but a few aren't, see below. I'll address it accordingly. @egull There's actually something your audit surfaced that I came across earlier this week: the CMake project is accidentally hardcoded to only ever compile with C++14, no matter what external flags are set. This is particularly bad in our CI, which never builds with C++17/20/23 even though it claims to do so -- and worse, it turns out that it can't compile for those standards at all. We should really fix that prior to the release. I suggest we review and merge #112 asap, and create a follow-up PR right after, I already fixed it locally. |
|
That’s really bad.
Let’s merge it then deal with the fallout. I’m pretty sure that I managed to build it with the different compilers (perhaps manually?)
I’m about to board a plane to Europe, will be offline until tomorrow.
Best,
Emanuel
… On Aug 18, 2026, at 4:23 PM, Tobias Wolf ***@***.***> wrote:
skilledwolf
left a comment
(ALPSim/ALPS#107)
<#107 (comment)>
@marcusr2ML <https://github.com/marcusr2ML> Thanks! Most of it is accurate, but a few aren't, see below. I'll address it accordingly.
@egull <https://github.com/egull> There's actually something your audit surfaced that I came across earlier this week: the CMake project is accidentally hardcoded to only ever compile with C++14, no matter what external flags are set. This is particularly bad in our CI, which never builds with C++17/20/23 even though it claims to do so -- and worse, it turns out that it can't compile for those standards at all. We should really fix that prior to the release. I suggest we review and merge #112 <#112> asap, and create a follow-up PR right after, I already fixed it locally.
audit_response.md <https://github.com/user-attachments/files/31195976/audit_response.md>
—
Reply to this email directly, view it on GitHub <#107?email_source=notifications&email_token=ABW32RNMOBADDLHYJX665ZD5KS3LBA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZTGM3DCOJZG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5333619970>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABW32RNOMPNISMN3WOOCBZD5KS3LBAVCNFSNUABFKJSXA33TNF2G64TZHM4DIOJZG43DEMRXHNEXG43VMU5TIOJVGI2DKNRZHEZKC5QC>.
You are receiving this because you were mentioned.
|
|
Safe travels! And I made it sound worse than it is, there's a simple name collision with standard libraries from C++17 and up somewhere in the library, the fix is to append the namespace at a few call sites, I'll create the PR later EDIT: done. the build honors requested C++ standards now, and compiles successfully for C++14 and up |
|
@marcusr2ML I think I addressed everything, can you review/audit it again? |
|
@skilledwolf I got this from the audit. It seems to report more issues, some of which are covered above. |
|
@marcusr2ML Great :) Note that I just pushed a fix that I forgot to merge back from my fork last night. I'll take my hands off it for now and will focus on other things until you've had a chance to look at it. I'm still not sure if it makes sense to merge it before friday. It would be very cool to have and it looks good on the face of it, but I'm a bit worried because it reached a complexity where we need AI assistence to review it. 🫠 |
90f7acc to
a491873
Compare
Finish the Python 3 and archive compatibility repairs, bundle the ALPS runtime and applications once, retain ordinary per-version wheels for downstream nanobind interoperability, and follow master's DWA removal.
a491873 to
e9930d0
Compare
|
@marcusr2ML Hey, it took a bit longer than expected, but here are the latest fixes. Let me know what you find :) |
Six follow-ups from assessing the nanobind migration against master. Each was
found by running something the migration had left unexecuted.
archive[path] = object could not store any ALPS extension type. The gate in
ngs/hdf5.cpp accepted only bound methods whose type name is "method" -- those
defined in Python -- and its comment claimed registered types kept a "native
save path" that does not exist, so params, observables, results and the rest
fell through to extract_from_pyobject and were reported as unsupported even
though obj.save(archive) worked when called directly. This is a Python-2-era
capability: Boost.Python's bound methods really were CPython methods, which
Python 2 named "instancemethod", so master's gate matched them and stopped
matching anything the day pyalps moved to Python 3.
Rather than enumerate the types in hdf5.cpp -- there are sixteen, ten of them
accumulator template instantiations owned by another translation unit -- the
capability is now declared where each class is bound, via
pyalps::mark_archive_savable in cpp/archive_savable.hpp. The archive module
needs no includes for and no knowledge of any of them, and a type added later
works without touching it. MCScalarData/MCVectorData deliberately do not
declare it, since their save() takes (filename, observable_name); they now
raise a TypeError naming the spelling that does work.
Container children were bypassing the dispatch entirely: the list and dict
branches of the save visitor recursed straight into extract_from_pyobject, so
archive[p] = {'Energy': observable} failed while archive[p] = observable
worked. They now recurse through python_hdf5_save, which is what makes a
checkpoint -- a mapping of observables -- writable at all.
str() raised TypeError on all ten accumulator and result types: __str__ is
bound as a std::string-returning function but ngs/accumulator.cpp never
included nanobind/stl/string.h. Invisible for as long as the test that would
have caught it was uncollectable by pytest.
Also:
- Delete src/alps/python/make_copy.hpp, an unused nanobind duplicate of the
Python-free src/alps/utility/make_copy.hpp that was installed as a public
header of an SDK documented to have no Python dependency.
- Keep one save_observable_to_hdf5.hpp: pyalea.cpp had been including a stale
bindings-local copy rather than the cleaned SDK one.
- Stop baking the build machine's absolute paths into every wheel.
pyalps_config.py's fallbacks are for builds that do not bundle their
resources, so they are empty when the package does bundle them -- the
default, and what every published wheel is. tools.py treats an absent
fallback as absent instead of exporting an empty path.
- Replace the three test files pytest never collected. accumulators.py used a
flat import that only resolved under the old in-tree build layout;
numpylarge.py wrote 4 GiB, asserted nothing and littered the source tree;
loadobs.py and loadobs.cpp both read a fixture neither created and that
ships nowhere. The replacements assert, run in tmp_path, and build their own
fixtures. loadobs_test.py also pins down the encoding rule both originals
got wrong: list_children returns names as stored, already encoded, and
re-encoding them double-encodes anything containing '/' or '&'.
- Give tutorials/ngs/{6_python_native,7_python_extend} smoke tests, run from
the pytest suite so cibuildwheel exercises them against every wheel. Both
needed repairs to checkpoint code that had never executed: save/load defined
twice in tutorial 6 with the first pair shadowed, a load() that wrote to the
archive instead of reading from it, double() in both, tutorial 7 reading its
checkpoint from paths save() does not write, and a bare ArchiveNotFound in
both main.py files that would have raised NameError.
Suite goes from 43 passed to 80. Numeric fidelity re-verified after the
changes: master's own test scripts still reproduce master's reference output
byte for byte.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Marking mcbase archive-savable interacts with its deliberately non-virtual bound save(), which exists so a Python subclass calling super().save(ar) does not re-enter its own override. If a downstream C++ simulation inherited that base save through the marker, `archive['/'] = sim` would silently checkpoint base state only -- a partial write where the previous behaviour was a loud "Unsupported type". It does not: export_sim_to_python binds save() on the derived type, so getattr finds the derived binding and both spellings write the same tree. Verified by building tutorials/ngs/5_export_python and comparing archive trees entry by entry -- 30 entries, identical, including the simulation's own checkpoint/sweeps and checkpoint/state. That comparison is now part of that tutorial's smoke test, which the gated downstream test already runs, so the guarantee cannot quietly lapse if the exporter stops binding save(). Also: the fall-through message no longer asserts that an unmarked type's save() takes a file name -- true for every such type in the tree, but misleading for a binding that merely forgot the marker -- and archive_savable.hpp records that it is pyalps-internal by design, with the reason a downstream module does not need it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The numpy output path allocated numpy.empty(shape, dtype=...) through the
cached numpy module and memcpy'd into it. Every caller had already built a
std::vector, so each conversion cost two copies and a Python round trip.
nb::ndarray<nb::numpy, T> over a heap vector owned by a capsule costs one.
Measured, best of seven, macOS arm64:
MCVectorData.mean n=1000 0.55 -> 0.36 us -35%
MCVectorData.mean n=100000 10.20 -> 9.67 us -5%
archive load 1-D n=100000 232 -> 212 us -9%
archive load 2-D 300x300 230 -> 205 us -11%
Modest: the Python round trip was a fixed cost, so small arrays gain most,
and archive loads sit on a ~170us floor set by HDF5 itself.
Integers deliberately keep the old dtype-named route. nb::ndarray describes
its buffer through DLPack, which encodes only "signed 64-bit" and so cannot
distinguish `long` from `long long`. NumPy can: different dtype objects,
different .char ('l' vs 'q'), different repr. Routing them through DLPack
turned archive["/ints"] from int64 into longlong -- identical bits, but a
visible change in what pyalps returns, which this migration has no business
making. It was caught by pyhdf5io_test.py no longer reproducing master's
reference output, and a compile-time remap cannot fix it because the
distinction is erased at the DLPack boundary. Floating point and complex have
one unambiguous dtype each, so they take the zero-copy route.
Three tests added: returned arrays stay writable, C-contiguous and
non-aliasing; an array outlives every C++ reference and the archive it came
from, across heap churn and forced collection; and integer dtypes match
element for element on .char and repr, not merely by == (which cannot see the
long/long long difference that broke the reference output).
Numeric fidelity re-verified: master's own test scripts still reproduce
master's reference output byte for byte.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cxx.py caught ImportError around its thirteen `from ._ext import <name>` lines and retried them as top-level modules -- the spelling that worked in the old in-tree build, where every .so sat side by side on PYTHONPATH. No installed layout can satisfy it, so the branch could only ever turn a real import failure into a second, more confusing one. Replaced with an explicit list and one importlib loop, so the module names are stated once rather than implied by a scan of locals() for keys ending in "_c". The sys.modules registration stays, and is now documented as load-bearing rather than incidental: pyalps' own alea, alea_detail, hdf5, pytools and ngs modules all import through `from .cxx.<name> import ...`, which is submodule syntax and cannot resolve from attributes alone. Also drops `from __future__ import print_function` from mpi.py, a no-op on every interpreter pyalps supports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Everything built fine. Ran the tutorials too. I only see some known issues (old DMRG code, tutorial script diverged from source code, incorrect escape sequences). I'll make a PR for these issues (I already have a catalog of these), but things look like they are working. |
pyalps carried its own hard-coded version in pyproject.toml, a second number to bump beside ALPS_VERSION.txt and one that had already drifted from it: ALPS_VERSION.txt read 2.3.4 while pyproject read 2.3.4b1, with nothing tying the two together. The numeric version now comes from ALPS_VERSION.txt -- the same file cmake/ALPSVersion.cmake reads for ALPS_VERSION_CORE -- through an in-tree scikit-build-core dynamic-metadata provider. A prerelease label cannot live in that file, because project(VERSION ...) rejects a non-numeric version and neither find_package() matching nor the library SOVERSION has a notion of prerelease ordering; CMake therefore takes it from the ALPS_VERSION_PRERELEASE cache variable, and the Python build now takes it from the environment variable of the same name, using the same vocabulary translated to PEP 440 (beta.1 -> b1, rc.2 -> rc2, dev.3 -> .dev3). An unrecognised label fails the build rather than producing a surprising version. The wheels CI sets ALPS_VERSION_PRERELEASE=beta.1 in [tool.cibuildwheel.environment], so published wheels keep the 2.3.4b1 identity they have today; clearing that line publishes a final release. A plain local build now produces 2.3.4 rather than silently claiming to be a beta. The sdist is rooted at the pyalps directory and cannot reach the repository root, so it force-includes its own copy of ALPS_VERSION.txt; the provider looks in both places, the same repo-or-vendored split the CMake side already handles for the application sources. Verified in all three layouts: a repo wheel build (2.3.4, and 2.3.4b1 with the label), an sdist that derives its version with no repository above it, and a wheel built from that sdist. pyalps.__version__ is added, read from the installed distribution metadata so that it too is not a second copy. test_wheel_payload.py fails if the installed version and ALPS_VERSION.txt ever disagree -- checked by temporarily setting the file to 9.9.9 and confirming the test goes red. One consequence is worth stating, and is documented in the pyalps README: because the number is inherited, a Python-only API change cannot be signalled in the pyalps version alone. It would take a bump of ALPS_VERSION.txt, which moves the whole project. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 participants
Summary
Replace Boost.Python with nanobind and build
pyalpsas a standalonescikit-build-core project under
bindings/python/pyalps. Port the 16 extensionmodules present on master: 13 core modules plus MaxEnt, CT-HYB and CT-INT.
Preserve the legacy import aliases, Python simulation interface, packaged
applications/resources and downstream C++ extension workflow.
The C++ SDK remains independent of Python. Binding-owned parameter providers
preserve NumPy arithmetic, shared-reference mutation, Python metadata and
checkpoint decoding while supplying checked values to native consumers.
Compatibility and correctness
parameters. Convert mixed lists element by element to the C++ consumer's
requested type and preserve comma-separated name-list text. Native-only
checkpoint reload/re-save handles Boolean/mixed scalar lists. Exercise reads
and final destruction on native threads.
type metadata; retain the old interpretation for unmarked legacy data.
Handle nested values, attributes, archive contexts and overwrites.
order, wildcard matching, cancellation, partial completion and all request
helpers for large messages, including large self-sends.
pop,clearandreload; support replacement and safe iteration across mapping mutations.
load()implementations, includingdefault results and scalar/vector collections at custom archive paths.
Failed loads leave prior values intact. Unary operations/deepcopy preserve
their input, and empty-result operations raise instead of crashing.
convergence returns, mean-only vector construction and count-only inputs.
prerelease versions when rebuilding sdists, and exercise the advertised
Boost versions in CI. Handle Boost 1.76's include-order issue and older
GCC's vector/scalar overload resolution.
Validation
All 45 checks pass on
cb9cc3e76: the 34-job compiler matrix and wheels, installation and MPI workflow. The tag-only publishing job is correctly skipped.3.10 with NumPy 1.26, SciPy 1.13 and nanobind 2.10. Both compiled downstream
extension checks are explicitly enabled, including the Ising simulation's
run/save/load cycle. The repaired wheel also passes the complete 204-test run.
a 4 MiB complex NumPy array, every request helper, ordering and cancellation.
using identical Python/NumPy versions: 384 identical; the remaining 19 are
individually classified bug fixes, added methods and removed Boost indexing
implementation types. Additional compiled old/new parameter readers verify
mixed-list conversions beyond Python getters.
loading; separately compiled native parameter ownership, precision and
thread/GIL checks.
spinmc,loop,dirloop_sse,sparsediag,fulldiaganddmrg, plus the smallMaxEnt, CT-HYB and CT-INT runs in the binding suite.
from the source distribution using the matching SDK.
Supported scope
CPython 3.10–3.14 is supported with per-version wheels. Stable-ABI and
free-threaded operation without the GIL are not claimed. Downstream extensions
must rebuild against the matching SDK revision because parameter layout
changed. The supported export example is
tutorials/ngs/5_export_python.pyalps[mpi]uses mpi4py's protocol. Boost.MPI's C++/Python serialization bridge,skeleton/content API and Boost-specific exception interface are not reproduced;
programs relying on those require adaptation. Experimental non-count
accumulators remain scalar-valued. Python metadata can be retained even when
it is unsuitable for a particular C++ scalar/vector consumer;
Nonehas noALPS HDF5 representation. Legacy unmarked signed-byte/Boolean data remains
intrinsically ambiguous. Native readers of new Boolean/mixed list checkpoints
need the updated SDK; the old/new cross-reading checks cover ordinary scalar
parameters and numeric arrays.
These checks establish compatibility for the tested workflows, not every solver
setting, historical example or downstream application. Platform CI passes;
the repository's required human review remains the merge gate.
This addresses the Python packaging portion of #86.