fix: declare libcudf wheel CUDA runtime dependencies#23414
Conversation
3a4e5a8 to
dc5279d
Compare
|
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. |
|
If CI passes, this should be retargeted to Also note that CI may fail until rapidsai/kvikio#1013 is merged. |
jameslamb
left a comment
There was a problem hiding this comment.
Makes sense to me, and the smoke test is wonderful. Thanks!
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR updates CUDA 12 and 13 wheel dependency selection, adds CUDA 13 NVRTC requirements, enables CUDA wheel selection for libcudf builds, and validates libcudf-related wheels in dedicated virtual environments. ChangesCUDA wheel updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ci/test_wheel_cudf_streaming.sh`:
- Around line 25-30: Update the install command in the wheel smoke test to
install the matching local libcudf, librapidsmpf, and librmm runtime wheels
alongside libcudf_streaming, using the same RAPIDS_PY_CUDA_SUFFIX and wheelhouse
selection. Ensure load_library() resolves these locally built dependencies
rather than falling back to system installations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 69634d51-afe5-4f2a-b231-936ef8229244
📒 Files selected for processing (1)
ci/test_wheel_cudf_streaming.sh
| rapids-pip-retry install \ | ||
| -v \ | ||
| --prefer-binary \ | ||
| --constraint "${PIP_CONSTRAINT}" \ | ||
| "$(echo "${LIBCUDF_STREAMING_WHEELHOUSE}"/libcudf_streaming_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" | ||
| python -c "import libcudf_streaming; libcudf_streaming.load_library()" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
. libcudf-streaming-env/bin/activate
python - <<'PY'
from importlib import metadata
for name in ("libcudf_streaming", "libcudf", "librapidsmpf", "librmm"):
try:
dist = metadata.distribution(name)
except metadata.PackageNotFoundError:
print(f"{name}: MISSING")
else:
print(f"{name}: {dist.version} ({dist.locate_file('')})")
print("libcudf_streaming requirements:")
for requirement in metadata.metadata("libcudf_streaming").get_all("Requires-Dist") or []:
print(requirement)
PY
deactivateRepository: rapidsai/cudf
Length of output: 227
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'ci/test_wheel_cudf_streaming.sh' 'ci/test_wheel_cudf.sh' 'python/libcudf_streaming/libcudf_streaming/load.py' 'python/libcudf_streaming/pyproject.toml' 'python/libcudf_streaming/setup.py'
printf '\n== Outline: ci/test_wheel_cudf_streaming.sh ==\n'
ast-grep outline ci/test_wheel_cudf_streaming.sh --view expanded || true
printf '\n== Outline: ci/test_wheel_cudf.sh ==\n'
ast-grep outline ci/test_wheel_cudf.sh --view expanded || true
printf '\n== Outline: python/libcudf_streaming/libcudf_streaming/load.py ==\n'
ast-grep outline python/libcudf_streaming/libcudf_streaming/load.py --view expanded || true
printf '\n== Read relevant slices ==\n'
sed -n '1,220p' ci/test_wheel_cudf_streaming.sh
printf '\n--- ci/test_wheel_cudf.sh ---\n'
sed -n '1,220p' ci/test_wheel_cudf.sh
printf '\n--- load.py ---\n'
sed -n '1,220p' python/libcudf_streaming/libcudf_streaming/load.py
printf '\n--- metadata files ---\n'
sed -n '1,220p' python/libcudf_streaming/pyproject.toml 2>/dev/null || true
sed -n '1,220p' python/libcudf_streaming/setup.py 2>/dev/null || trueRepository: rapidsai/cudf
Length of output: 12787
Install the matching runtime wheels in this smoke test. This step only installs libcudf_streaming, but libcudf_streaming.load_library() imports libcudf, librapidsmpf, and librmm, and can fall back to system paths if those imports are missing. Add the matching local runtime wheels here, or assert the expected wheel metadata, so this job actually exercises the wheel set produced by the build.
🤖 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/test_wheel_cudf_streaming.sh` around lines 25 - 30, Update the install
command in the wheel smoke test to install the matching local libcudf,
librapidsmpf, and librmm runtime wheels alongside libcudf_streaming, using the
same RAPIDS_PY_CUDA_SUFFIX and wheelhouse selection. Ensure load_library()
resolves these locally built dependencies rather than falling back to system
installations.
|
/ok to test 9d7fdd5 |
|
/ok to test 9dae08d |
Description
This fixes some wheel packaging issues in libcudf / cudf. The main goal is to declare the CUDA runtime libraries loaded by
libcudfwheels: NVRTC and nvJitLink.Previously we included an
nvcccomponent which was needed bynumba-cuda, but that is now handled through our dependency onnumba-cuda[cu12]which includes the necessary CUDA wheels.In #23392, we added a dependency on cuFile, but this was incorrect. cuDF doesn't use cuFile directly, it is only used through KvikIO. rapidsai/kvikio#1013 supplies the corresponding cuFile metadata for KvikIO, so we don't need that here.
This PR also adds an isolated
libcudf.load_library()wheel smoke test beforepylibcudftest extras are installed, so that test dependencies cannot mask missinglibcudfwheel requirements. This kind of test revealed the issue in KvikIO's packaging, so we should add similar tests here.Checklist