-
Notifications
You must be signed in to change notification settings - Fork 9
Fix cuda_toolkit_check driver version and add cuda-bindings dep #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
aad73a2
512c820
c65de81
5ba4499
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,9 +173,8 @@ def _gather_toolkit_info() -> CudaToolkitInfo: # pragma: no cover | |
| except (DynamicLibNotFoundError, RuntimeError): | ||
| info.missing_libs.append(soname) | ||
|
|
||
| # Get driver version | ||
| try: | ||
| info.driver_major = get_driver_version(kernel_mode=True)[0] | ||
| info.driver_major = get_driver_version()[0] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we purposely grabbing the user-mode driver version, e.g. 13.0.1. Instead of passing |
||
| except Exception: | ||
| info.driver_major = None | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| from rapids_cli.doctor.checks.cuda_toolkit import ( | ||
| CudaToolkitInfo, | ||
| _ctypes_cuda_version, | ||
| _gather_toolkit_info, | ||
| _get_toolkit_cuda_major, | ||
| cuda_toolkit_check, | ||
| ) | ||
|
|
@@ -176,3 +177,16 @@ def test_check_cuda_home_newer_than_driver(set_toolkit_info): | |
| ): | ||
| with pytest.raises(ValueError, match="CUDA_HOME"): | ||
| cuda_toolkit_check() | ||
|
|
||
|
|
||
| def test_gather_toolkit_info_driver_major_is_cuda_major(): | ||
| """Regression: driver_major must be the CUDA Driver API major, not the kernel driver major.""" | ||
| try: | ||
| info = _gather_toolkit_info() | ||
| except Exception as e: | ||
| pytest.skip(f"_gather_toolkit_info unavailable on this platform: {e}") | ||
| if info.driver_major is not None: | ||
| assert info.driver_major < 100, ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm trying to follow what are we testing here. Looks like we are testing that the major version of Are user-mode driver version, e.g. 13 in 13.0.1 is < 100 . Don't we need to grab kernel-mode driver version, e.g. 580.65.06 like we we were doing? until now |
||
| f"driver_major={info.driver_major} looks like a kernel driver " | ||
| f"version, not a CUDA Driver API major" | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to add a comment here and
pyproject.tomlwith the reason behind this.