fix(deps): add explicit cufile dependency#1552
Conversation
gforsyth
added
non-breaking
gforsyth
added
non-breaking
|
Ah, this is not necessary -- I just clocked that the |
📝 WalkthroughWalkthroughChangesThe dependency matrix now includes cuFile packages and CUDA toolkit components, while the Python package declares cuFile support
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@python/libcuopt/libcuopt/load.py`:
- Around line 49-53: Scope the librt.so.1 preload workaround in load_library()
to aarch64 only, using platform architecture detection, and wrap
ctypes.CDLL("librt.so.1", mode=ctypes.RTLD_GLOBAL) in a dedicated try/except
OSError. Allow loading to continue through existing fallbacks when the preload
is unavailable, while retaining the subsequent load_nvidia_dynamic_lib("cufile")
behavior.
- Around line 41-54: Separate the cuda.pathfinder import and cufile-specific
loading from the main core-library load block in the module’s load routine. Keep
rapids_logger, librmm, and libraft imports and load_library calls in the
existing guarded block, then add a subsequent guarded block for
load_nvidia_dynamic_lib, the librt workaround, and cufile loading so a missing
cuda-pathfinder does not prevent core libraries from loading.
🪄 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: cdc462e5-8865-4b5c-9b91-88306ae2dc2e
📒 Files selected for processing (3)
dependencies.yamlpython/libcuopt/libcuopt/load.pypython/libcuopt/pyproject.toml
| import libraft | ||
| import librmm | ||
| import rapids_logger | ||
| from cuda.pathfinder import load_nvidia_dynamic_lib | ||
|
|
||
| rapids_logger.load_library() | ||
| librmm.load_library() | ||
| libraft.load_library() | ||
| # We manually load librt.so.1 here to workaround a bad dynamic link | ||
| # in nvidia_cufile-1.15.1.6-py3-none-manylinux_2_27_aarch64.whl | ||
| # https://nvbugspro.nvidia.com/bug/6425783 | ||
| ctypes.CDLL("librt.so.1", mode=ctypes.RTLD_GLOBAL) | ||
| load_nvidia_dynamic_lib("cufile") | ||
| except ModuleNotFoundError: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: check if cuda-pathfinder is referenced anywhere outside dependencies.yaml (e.g. conda recipes)
rg -n "cuda-pathfinder|cuda_pathfinder" --type=yaml -g '!dependencies.yaml'
fd -a meta.yaml -x cat {}Repository: NVIDIA/cuopt
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== load.py ==\n'
sed -n '1,140p' python/libcuopt/libcuopt/load.py
printf '\n== dependencies.yaml matches ==\n'
rg -n -C 3 "cuda-pathfinder|cuda_pathfinder|requirements|pyproject|conda" dependencies.yaml
printf '\n== any packaging files mentioning cuda-pathfinder ==\n'
rg -n "cuda-pathfinder|cuda_pathfinder" -g '!dependencies.yaml' .Repository: NVIDIA/cuopt
Length of output: 21481
Move the cuda.pathfinder import out of the core load block. In python/libcuopt/libcuopt/load.py:41-54, a missing cuda-pathfinder turns the whole try into a no-op, so rapids_logger, librmm, and libraft never load on conda installs. Split the cufile-specific import/workaround into its own guarded block after the core loads.
🤖 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 `@python/libcuopt/libcuopt/load.py` around lines 41 - 54, Separate the
cuda.pathfinder import and cufile-specific loading from the main core-library
load block in the module’s load routine. Keep rapids_logger, librmm, and libraft
imports and load_library calls in the existing guarded block, then add a
subsequent guarded block for load_nvidia_dynamic_lib, the librt workaround, and
cufile loading so a missing cuda-pathfinder does not prevent core libraries from
loading.
| # We manually load librt.so.1 here to workaround a bad dynamic link | ||
| # in nvidia_cufile-1.15.1.6-py3-none-manylinux_2_27_aarch64.whl | ||
| # https://nvbugspro.nvidia.com/bug/6425783 | ||
| ctypes.CDLL("librt.so.1", mode=ctypes.RTLD_GLOBAL) | ||
| load_nvidia_dynamic_lib("cufile") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unscoped, unguarded librt.so.1 preload.
The comment explains this workaround is for nvidia_cufile-1.15.1.6-py3-none-manylinux_2_27_aarch64.whl, but ctypes.CDLL("librt.so.1", mode=ctypes.RTLD_GLOBAL) runs unconditionally on every architecture, and its OSError is not caught (only ModuleNotFoundError is handled by the surrounding except). If librt.so.1 is absent on some system, this raises out of load_library() uncaught, whereas before this change library loading degraded gracefully via the various fallbacks (system installation / wheel installation) elsewhere in this function.
Consider gating this preload to aarch64 (matching the referenced bug) and wrapping it in its own try/except OSError so a missing librt.so.1 doesn't hard-fail library loading.
🛠️ Proposed fix to scope and guard the workaround
- # We manually load librt.so.1 here to workaround a bad dynamic link
- # in nvidia_cufile-1.15.1.6-py3-none-manylinux_2_27_aarch64.whl
- # https://nvbugspro.nvidia.com/bug/6425783
- ctypes.CDLL("librt.so.1", mode=ctypes.RTLD_GLOBAL)
- load_nvidia_dynamic_lib("cufile")
+ if platform.machine() in ("aarch64", "arm64"):
+ # We manually load librt.so.1 here to workaround a bad dynamic link
+ # in nvidia_cufile-1.15.1.6-py3-none-manylinux_2_27_aarch64.whl
+ # https://nvbugspro.nvidia.com/bug/6425783
+ try:
+ ctypes.CDLL("librt.so.1", mode=ctypes.RTLD_GLOBAL)
+ except OSError:
+ pass
+ load_nvidia_dynamic_lib("cufile")(requires import platform at the top of the file)
🤖 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 `@python/libcuopt/libcuopt/load.py` around lines 49 - 53, Scope the librt.so.1
preload workaround in load_library() to aarch64 only, using platform
architecture detection, and wrap ctypes.CDLL("librt.so.1",
mode=ctypes.RTLD_GLOBAL) in a dedicated try/except OSError. Allow loading to
continue through existing fallbacks when the preload is unavailable, while
retaining the subsequent load_nvidia_dynamic_lib("cufile") behavior.
CI Test Summary✅ All 31 test job(s) passed. |
1 participant
Description
Testing a smaller base CI image for wheel tests in #1548 -- that image does not have cuda toolkit installed and we rely on installing all dependencies from wheels.
That test bubbled up a missing
cufiledependency (which was previously satisfied by thelibcufile.sothat was present on the CI image already).I've also added in a workaround for a bug with the
cufilewheels on ARM64, where they fail to load required dynamic libraries.We had to make the same changes in rapidsai/kvikio#1000
xref rapidsai/build-planning#143
CI run before this change: https://github.com/NVIDIA/cuopt/actions/runs/29044331506/job/86218256034?pr=1548
And after this change: https://github.com/NVIDIA/cuopt/actions/runs/29054246782/job/86246625279?pr=1548
Checklist