Skip to content

Audit Wheel Dependencies#3091

Open
tarang-jain wants to merge 5 commits into
NVIDIA:mainfrom
tarang-jain:cmake-audit
Open

Audit Wheel Dependencies#3091
tarang-jain wants to merge 5 commits into
NVIDIA:mainfrom
tarang-jain:cmake-audit

Conversation

@tarang-jain

@tarang-jain tarang-jain commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Drops depends_on_nccl from the libraft wheel's build and runtime
requirement groups (py_rapids_build_libraft, py_run_libraft) in
dependencies.yaml.

More changes on 07/22/2026:
libraft and libraft-static dont need to depend on the complete libraft-headers (there are no multi-gpu targets etc). Further raft-dask seems like the only target that needs the cmake flag for nccl

Addresses #2977.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

LibRAFT dependency groups, wheel build arguments, and conda recipes are updated. NCCL runtime wheel configuration is restricted to raft-dask, header-only package requirements are adopted, and obsolete dependency declarations are removed.

Changes

LibRAFT packaging updates

Layer / File(s) Summary
Update LibRAFT dependency includes
dependencies.yaml
LibRAFT dependency groups replace depends_on_nccl with depends_on_librmm and depends_on_rapids_logger.
Scope wheel build arguments
ci/build_wheel.sh
The NCCL runtime wheel CMake argument is exported only when building raft-dask; the SPDX header is updated.
Update conda package requirements
conda/recipes/libraft/recipe.yaml, conda/recipes/pylibraft/recipe.yaml
The libraft and libraft-static packages switch to exact libraft-headers-only requirements, remove cuda-profiler-api, and update the pylibraft recipe to remove libraft-headers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: msarahan

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title matches the main theme of changing wheel and dependency requirements.
Description check ✅ Passed The description is clearly related to the dependency and build-flag changes in the PR.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@tarang-jain tarang-jain added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 21, 2026
@tarang-jain
tarang-jain requested a review from a team as a code owner July 22, 2026 20:58
@tarang-jain tarang-jain changed the title Remove NCCL from the libraft wheel dependencies Audit Wheel Dependencies Jul 22, 2026
@tarang-jain tarang-jain self-assigned this Jul 23, 2026
- ${{ compiler("c") }}
host:
- ${{ pin_subpackage("libraft-headers", exact=True) }}
- ${{ pin_subpackage("libraft-headers-only", exact=True) }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this changed? What is the specific difference in host/run pinnings that you are aiming to produce?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of the impact from AI:

libraft-headers depends on libraft-headers-only, so selecting it also installs the latter's cuda-version, librmm, and rapids-logger dependencies. Relative to that shared closure, libraft-headers additionally brings cuda-profiler-api, libcublas-dev, libcurand-dev, libcusolver-dev, libcusparse-dev, nccl >=2.19, and ucxx 0.52.\*.

Consequently, replacing libraft-headers with libraft-headers-only avoids the four CUDA development packages plus nccl and ucxx. The runtime CUDA libraries are not removed from libraft or libraft-static: both packages continue to directly require libcublas, libcurand, libcusolver, and libcusparse. cuda-profiler-api is also directly removed from those two packages in PR 3091, rather than being removed solely through the header-package substitution.

So basically this means that depending on libraft is no longer enough to build other libraries against libraft, because you don't have CUDA math library dev packages. This is a regression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that the libraft package only contains a few source files in the raft_runtime directory:

add_library(
    raft_objs OBJECT
    src/raft_runtime/random/rmat_rectangular_generator_int64_double.cu
    src/raft_runtime/random/rmat_rectangular_generator_int64_float.cu
    src/raft_runtime/random/rmat_rectangular_generator_int_double.cu
    src/raft_runtime/random/rmat_rectangular_generator_int_float.cu
    src/raft_runtime/solver/lanczos_solver_int64_double.cu
    src/raft_runtime/solver/lanczos_solver_int64_float.cu
    src/raft_runtime/solver/lanczos_solver_int_double.cu
    src/raft_runtime/solver/lanczos_solver_int_float.cu
  )

I think any downstream package should depend on libraft-headers and not libraft

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the important question is -- what do we need the libraft pkg to represent -- only the compiled source files or all the packages required to include any header in this repo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the description here:
https://pypi.org/project/libraft-cu12/#:~:text=RAFT%20is%20a,GPU%20with%20Dask.

I think this PR is in the right direction -- libraft's shared library should be C++ only. I'll wait for others / PICs to weigh in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants