Skip to content

Follow-up to recent cucascade fixes#22510

Closed
KyleFromNVIDIA wants to merge 3 commits into
rapidsai:mainfrom
KyleFromNVIDIA:cucascade-fix-followup
Closed

Follow-up to recent cucascade fixes#22510
KyleFromNVIDIA wants to merge 3 commits into
rapidsai:mainfrom
KyleFromNVIDIA:cucascade-fix-followup

Conversation

@KyleFromNVIDIA
Copy link
Copy Markdown
Member

Description

  • Vendor FindCUDAToolkit as of af0828cbbff5d9b111b7209b46242b9c9ba7865c
  • Require cuda-nvml-dev when building devcontainers

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@KyleFromNVIDIA KyleFromNVIDIA requested review from a team as code owners May 14, 2026 18:16
@KyleFromNVIDIA KyleFromNVIDIA requested a review from msarahan May 14, 2026 18:16
@KyleFromNVIDIA KyleFromNVIDIA added bug Something isn't working non-breaking Non-breaking change labels May 14, 2026
@github-actions github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels May 14, 2026
Copy link
Copy Markdown
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Are we missing cuda-nvml-dev in the conda recipe? Or is it not needed there?

- cuda-nvrtc-dev

@KyleFromNVIDIA
Copy link
Copy Markdown
Member Author

Well, it looks like none of my attempted fixes have worked so far. I need to do some local investigation and will push once I have something that works.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6db18774-93aa-45be-98a4-a0f94ae74a98

📥 Commits

Reviewing files that changed from the base of the PR and between d7c7891 and 27019f1.

📒 Files selected for processing (6)
  • conda/environments/all_cuda-129_arch-aarch64.yaml
  • conda/environments/all_cuda-129_arch-x86_64.yaml
  • conda/environments/all_cuda-132_arch-aarch64.yaml
  • conda/environments/all_cuda-132_arch-x86_64.yaml
  • cpp/cmake/Modules/FindCUDAToolkit.cmake
  • dependencies.yaml

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated CUDA development toolkit dependencies to include NVML packages across all supported CUDA versions and processor architectures.
    • Enhanced CUDA toolkit detection and build configuration to prevent duplicate target definitions and improve compatibility.

Walkthrough

CUDA NVML development package support is added across multiple conda environment definitions by updating the source dependency manifest and generated environment files for CUDA 12.9 and 13.2 on both aarch64 and x86_64 architectures. CMake CUDA toolkit detection is simultaneously improved to capture nvcc exit status and conditionally create imported targets only when they do not already exist.

Changes

CUDA NVML Development Support and CMake Improvements

Layer / File(s) Summary
Dependency source configuration
dependencies.yaml
The CUDA dependency group adds cuda-nvml-dev to the conda package list as a common CUDA development dependency.
Generated conda environments
conda/environments/all_cuda-129_arch-aarch64.yaml, conda/environments/all_cuda-129_arch-x86_64.yaml, conda/environments/all_cuda-132_arch-aarch64.yaml, conda/environments/all_cuda-132_arch-x86_64.yaml
Each of the four generated environment manifests for CUDA 12.9 and 13.2 across aarch64 and x86_64 architectures includes cuda-nvml-dev in the dependencies list.
CMake CUDA toolkit improvements
cpp/cmake/Modules/FindCUDAToolkit.cmake
CUDA toolkit version detection now captures the exit status of the nvcc --version command; CUDA::nvml_static and CUDA::sanitizer imported-target creation is made conditional, executed only when the targets do not already exist, with CUDA::sanitizer also receiving interface include directory configuration derived from its IMPORTED_LOCATION.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

improvement

Suggested reviewers

  • msarahan
  • gforsyth
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title refers to a follow-up to cucascade fixes, which aligns with the PR's actual changes: vendoring FindCUDAToolkit and adding cuda-nvml-dev requirement.
Description check ✅ Passed The description clearly outlines the two main changes: vendoring FindCUDAToolkit and requiring cuda-nvml-dev for devcontainers, directly related to the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@KyleFromNVIDIA
Copy link
Copy Markdown
Member Author

Closing in favor of rapidsai/rapidsmpf#1035

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

Labels

bug Something isn't working CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants