build(wheel): split libcuopt into per-component wheels - #1929
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
dec8e57 to
f8bf4e1
Compare
c515d8a to
cbb552e
Compare
f8bf4e1 to
d97768f
Compare
cbb552e to
439b881
Compare
d97768f to
c6ca78a
Compare
439b881 to
d075240
Compare
c6ca78a to
51524a6
Compare
d075240 to
76bc6f4
Compare
76bc6f4 to
edea594
Compare
CI Test Summary✅ All 32 test job(s) passed. |
edea594 to
7102c92
Compare
3fd937e to
5112f48
Compare
68932d6 to
d1f8b53
Compare
The build scripts existed but nothing invoked them, so CI never built the component wheels and the split had no effect on what gets published. pr.yaml gains a build job per component on the same matrix filter as libcuopt, and build.yaml gains the matching build and publish jobs. wheel-build-cuopt now waits on all three, since the cuopt wheel resolves through them rather than through libcuopt alone, and pr-builder gates on them so a failure blocks the PR instead of passing silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All four component wheel builds failed at the last step with "unrecognized package_dir: 'python/libcuopt_client'". The wheels built and auditwheel repaired them; validate_wheel.sh rejects any package_dir it does not know, and the three new ones were never added. mathopt and routing take libcuopt's existing limits, which are a safe ceiling since each is strictly smaller than the combined wheel was. The client gets 50Mi: it has no CUDA kernels, so anything near the other limits would mean something was vendored into it that should not have been. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds return annotations and documents the contract for the three load_library functions and the cuopt_cli entry point: the empty-list result, that a missing library warns rather than raising, and that main never returns because execv replaces the process image. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
libcuopt stages only the ld script and cuopt_grpc_server, but its dependencies were still the CUDA stack it no longer ships, and it named none of the component wheels that now carry the libraries. pip install libcuopt would have pulled cudss and nccl while getting no engine at all. The earlier edit to python/libcuopt/pyproject.toml did not survive, because dependencies.yaml is the source of truth and py_run_libcuopt was never updated; the generator rewrote the file back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wheel-build-cuopt failed with "No matching distribution found for libcuopt-client-cu12": libcuopt is now a metapackage over the component wheels, which are new packages no index carries, so pip resolved its dependencies against PyPI and found nothing. The pip output leads with a hash-mismatch line, which is why I first read this as a network flake -- the real error is further up. ci/utils/download_libcuopt_components.sh downloads the three artifacts, constrains pip to them and exposes their paths. Sourced from the four scripts that install libcuopt: build_wheel_cuopt, test_wheel_cuopt, test_wheel_cuopt_server and test_self_hosted_service, all of which would have hit the same resolution failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cuopt-config.cmake exports one _IMPORT_PREFIX for all three libraries, so splitting them across sibling wheels leaves find_package(cuopt) unable to resolve cuopt::client / ::mathopt / ::routing, which python/cuopt links against. That is what broke wheel-build-cuopt. libcuopt therefore stages every component and keeps its own dependencies and its cuopt_cli console script, exactly as before this PR. The component wheels are still built, published and installable on their own, which is the part that stands without the CMake work. Reverts the metapackage dependencies and the constraint helper that existed only to resolve them. Trimming libcuopt needs per-component CMake packages so each resolves from its own prefix; that is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each component's load_library() loads libcuopt_client before its own library, so glibc satisfies the DT_NEEDED from the already-loaded object and never searches a path for it. The entries were redundant even when the libraries were split, and libcuopt stages all three again, so $ORIGIN/../lib64 covers them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wheel-build-cuopt failed configuring the cuopt wheel: cub-config.cmake:9 Unknown CMake command "libcudacxx_update_language_compat_flags" Naming the components explicitly drops everything installed without one. rapids-cmake writes its helper find-modules to lib64/rapids/cmake/ with no COMPONENT, so the wheel lost them and CMake fell back to the CUDA toolkit's libcudacxx rather than the vendored one librmm's cub-config expects. A passing run on another PR resolves libcudacxx inside the libcuopt wheel; this PR's resolved it under /usr/local/cuda. The C++ install partitions cleanly across the named components, which is why this was not visible there: those helper files only exist in the wheel build, where the dependencies are fetched rather than found. libcuopt keeps everything for now, so the setting has no purpose. The file is now functionally identical to main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reverting libcuopt to ship every component left several pieces behind. python/libcuopt/libcuopt/load.py still delegated to libcuopt_client and the other component packages, which libcuopt no longer depends on, so 'import libcuopt; libcuopt.load_library()' -- which ci/test_wheel_cuopt.sh asserts -- would have raised ModuleNotFoundError. Reverted. ci/build_wheel_libcuopt.sh excluded the component libraries from vendoring on the grounds that sibling wheels provide them; this wheel ships them itself. Reverted. The three component scripts keep their excludes, where that reasoning still holds. python/libcuopt/CMakeLists.txt kept its own copy of the body extracted to python/cmake/cuopt_wheel_build.cmake, so the same 83 lines existed twice and could drift. It now includes the shared file like the other three. Verified the include is reached, rather than skipped by the cuopt_FOUND early return: configuring with -DCMAKE_DISABLE_FIND_PACKAGE_cuopt=ON generates the full build graph. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ilds build_wheel_cuopt.sh downloads only the libcuopt wheel -- it names the component libraries solely in auditwheel excludes, which is unchanged from main -- so waiting on the three component jobs bought nothing and delayed the cuopt wheel by a full build. pr-builder still gates on them, so a component failure blocks the PR either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rapids-build-backend writes .pyproject.toml.rapids-build-backend.bak while building a wheel and restores from it afterwards; an interrupted local build left one behind and 'git add -A' swept it in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MANIFEST.in and libcuopt/GIT_COMMIT are both written by rapids-build-backend during a wheel build -- the MANIFEST.in even says so -- and local builds left them behind for 'git add -A' to pick up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both declare libcuopt-client as a dependency, so publishing either first leaves a clean install unable to resolve it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wheel-build-libcuopt failed with FileNotFoundError: 'libcuopt_cu13/GIT_COMMIT' rapids-build-backend rewrites pyproject.toml in place during a build, applying the CUDA suffix to the project name, and restores it afterwards. An interrupted local build left the rewritten copy behind and it was committed. _write_git_commits derives the directory from the name it reads, so it looked for libcuopt_cu13/ rather than libcuopt/. Third artifact from the same source, after the .bak file and MANIFEST.in, and the only one that broke a build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Splitting libcuopt across wheels breaks find_package(cuopt): cuopt-config.cmake exports a single _IMPORT_PREFIX for all three libraries, derived from where the config sits, so with the libraries in sibling wheel directories none of cuopt::client / ::mathopt / ::routing resolve. python/cuopt links those, and wheel-build-cuopt fails with 'Target "data_model_wrapper" links to cuopt::mathopt but the target was not found'. So far: - each component installs to its own export set, so install(EXPORT) writes one targets file per component and each computes _IMPORT_PREFIX from its own install location - the umbrella config includes whichever component targets files it finds, beside itself or through CMAKE_PREFIX_PATH - cuopt::cuopt is synthesised in the config rather than exported, since an exported target may not reference targets outside its own export set Does not configure yet: rapids_export(BUILD) still exports the build tree, where the umbrella's BUILD_INTERFACE links to the components are live and they are no longer members of cuopt-exports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…them cuopt-config.cmake exported one _IMPORT_PREFIX for all three libraries, taken from where the config sits, so find_package(cuopt) only resolved while they shared a tree. Split across wheels it defined none of cuopt::client / ::mathopt / ::routing, which python/cuopt links against. Each component now exports to its own set, so install(EXPORT) writes one targets file per component and each computes its own prefix. The umbrella carries no component links -- CMake will not export a target referencing targets outside its export set -- and the installed config rebuilds them from whichever components are present. ConfigureTest, the only in-tree consumer of those links, names the components directly. Verified from a consumer project: find_package(cuopt) resolves cuopt::cuopt, ::client, ::mathopt and ::routing, with the umbrella linking all three. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b8deb05 to
bd1bccb
Compare
|
Do we have tests for individual components? These are the Agent reviewed concerns:
|
…heir siblings Two gaps the component split opened, both found in review. cudss_mtlayer_cuopt installed with COMPONENT runtime, which no wheel and no conda output stages, so libcudss_mtlayer_cuopt.so reached neither. The barrier solver dlopens it by bare soname and falls back to single-threaded cuDSS when that fails, so the loss was a silent slowdown rather than an error. It now installs with mathopt, the only component that loads it. cuopt_cli has a DT_NEEDED on libcuopt_client.so, ships in libcuopt-mathopt, and lost its sibling RPATH entry in 2335024. That entry is unnecessary for the libraries, since load_library() loads them in dependency order and glibc then satisfies each DT_NEEDED from an already-loaded object, but an executable has no Python in the process. The entries come back for the executables alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both concerns are real. Checked them against the built artifacts rather than the intent, and fixed both in 0bea735. 1.
Fixed by restoring the sibling entries for the executables only, leaving the libraries as 2335024 left them — the distinction being whether there is a Python interpreter in the process. 2.
Worth noting the failure mode is graceful, not fatal — Moved to On tests for individual components — partly, and the gap is exactly where these two slipped through. conda has a per-output isolation test: each loads its own The wheels have nothing. They are built and published but never installed or imported in CI, and no test runs Happy to add a wheel smoke-test job here — install each component wheel in a clean env, |
Iroy30
left a comment
There was a problem hiding this comment.
thanks for addressing the reviews!
Splitting the components into their own export sets left cuopt-exports holding only the umbrella. install(EXPORT) writes a targets file per component, but nothing wrote them for the build tree, and both exports share one FINAL_CODE_BLOCK that includes those files by name. A build-tree or CPM consumer therefore got cuopt_FOUND=1, cuopt::cuopt defined, no components, and an empty INTERFACE_LINK_LIBRARIES -- a find_package that succeeds and fails later at link time. Before the split all four targets were in the single cuopt-targets.cmake, so this was a regression. export(EXPORT) beside each install(EXPORT) restores it. Verified with a consumer project against both the build tree and an install prefix: both now report cuopt::cuopt links = 'cuopt::client;cuopt::mathopt;cuopt::routing'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Correct, and it's a regression. Fixed in e85ad59. A consumer against the build tree got It worked before this PR: the components were all in Fix is an |
|
/merge |
Adds a wheel per component, using the install components from #1926 via scikit-build-core's
install.components.libcuopt-clientlibcuopt_client.solibcuopt-mathoptlibcuopt_mathopt.so,cuopt_clilibcuopt-routinglibcuopt_routing.socuda-toolkit[cublas,cudart]onlylibcuoptcuopt_grpc_serverA routing-only install no longer pulls cudss, nccl or nvjitlink. Dependencies were split from each library's measured
DT_NEEDEDrather than by dividing the existing list.libcuoptno longer bundles the libraries; it depends on them, and itsload_librarydelegates to the component packages, client first since mathopt and routing both have aDT_NEEDEDon it.pip install libcuoptis unchanged for users.Also folds in the CMake export restructure that was #1963: each component gets its own export set, since CMake will not export a target that references targets outside its own export set, and a single umbrella export left every component's targets file computing the wrong
_IMPORT_PREFIX. The installed config rebuilds the umbrella's links from whichever components are present. The two changes only work together -- without it the component wheels cannot resolve each other -- so they are reviewed as one.Each wheel configures the whole C++ tree, so this trades CI build time for install size, with sccache absorbing the repeats. Building once and packaging three times would need
build_wheel.shrestructured — worth doing separately if the cost shows up.🤖 Generated with Claude Code