Link to libcuvs dynamically in wheel builds - #8250
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds libcuvs==26.8.* to packaging with CUDA-specific wheel variants and an extra-index-url, removes depends_on_libcuvs from py_rapids_build_cuml includes, drops CUML_USE_CUVS_STATIC from CMake and CI args, calls libcuvs.load_library() during cuML preload, and tightens linkage test handling. Changeslibcuvs Dependency Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
bdice
left a comment
There was a problem hiding this comment.
See also: rapidsai/build-planning#293
|
Don't we need to add |
|
@jcrist good catch! |
| # librmm.so and librapids_logger.so may not be found | ||
| # but are loaded dynamically at runtime, skip these | ||
| if lib_name in ["librmm.so", "librapids_logger.so"]: | ||
| continue |
There was a problem hiding this comment.
These libraries were never in the expected_libs dictionary that only tests CUDAtoolkit dependencies whose rpath is modified. As we explicitly load RAPIDS dependencies, we don't have to test for these linkages.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci/build_wheel_libcuml.sh (1)
46-53:⚠️ Potential issue | 🟠 MajorAdd
libcuvs*toauditwheel repairexclusions inci/build_wheel_libcuml.sh
ci/build_wheel_libcuml.shrunspython -m auditwheel repairwith anEXCLUDE_ARGSlist that does not excludelibcuvs*, unlikeci/build_wheel_cuml.shwhich excludeslibcuvs.so. This can causeauditwheelto bundlelibcuvsinto thelibcumlwheel, negating the intended dynamic runtime dependency model.Add matching
--exclude "libcuvs.so.*"/--exclude "libcuvs_c.so.*"(or equivalent patterns used by yourcuvsartifacts) toEXCLUDE_ARGSinci/build_wheel_libcuml.sh.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/build_wheel_libcuml.sh` around lines 46 - 53, The auditwheel repair invocation in ci/build_wheel_libcuml.sh omits libcuvs from EXCLUDE_ARGS, allowing libcuVS to be bundled into the libcuml wheel; update the EXCLUDE_ARGS array used by the python -m auditwheel repair call to include patterns excluding libcuvs (e.g., --exclude "libcuvs.so.*" and/or --exclude "libcuvs_c.so.*") so auditwheel does not repair/bundle libcuvs into the wheel when running the command that uses EXCLUDE_ARGS and writes to ${RAPIDS_WHEEL_BLD_OUTPUT_DIR}.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ci/build_wheel_libcuml.sh`:
- Around line 46-53: The auditwheel repair invocation in
ci/build_wheel_libcuml.sh omits libcuvs from EXCLUDE_ARGS, allowing libcuVS to
be bundled into the libcuml wheel; update the EXCLUDE_ARGS array used by the
python -m auditwheel repair call to include patterns excluding libcuvs (e.g.,
--exclude "libcuvs.so.*" and/or --exclude "libcuvs_c.so.*") so auditwheel does
not repair/bundle libcuvs into the wheel when running the command that uses
EXCLUDE_ARGS and writes to ${RAPIDS_WHEEL_BLD_OUTPUT_DIR}.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e3625e54-c88b-4276-a336-9e62c62a68fb
📒 Files selected for processing (1)
ci/build_wheel_libcuml.sh
|
/merge |
Now that
libcuvswheels are available on PyPI, we do not need to maintain the static link in our wheel builds. This should make CI jobs faster and eliminatelibcuvsrelated cache misses.