Add _get_cuda_version_from_cuda_h() in cuda_core/build_hooks.py#1085
Add _get_cuda_version_from_cuda_h() in cuda_core/build_hooks.py#1085rwgk wants to merge 4 commits intoNVIDIA:mainfrom
_get_cuda_version_from_cuda_h() in cuda_core/build_hooks.py#1085Conversation
|
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. |
|
/ok to test |
|
Local testing with a venv that does not have cuda-bindings: |
|
…, _get_cuda_driver_version_linux()
|
Calling nvidia-smi is better because it does exactly the same thing under the hood (opening up libcuda/nvcuda and getting the driver version), but it is cross-platform and so takes much less lines of code, and it is a good habit to keep the code surface in |
The main reason I was leaning into it is that I think it'd be a useful addition to pathfinder. It's more lines of code only because Assume that the code I added lives in pathfinder, you'd just reuse that and have a more secure, more robust, and faster (which doesn't matter here, but is useful in general) solution with even less lines of code. |
leofang
left a comment
leofang
left a comment
There was a problem hiding this comment.
The main reason I was leaning into it is that I think it'd be a useful addition to pathfinder.
Why would the pathfinder need to expose the driver version check as a public API? Or you only had the driver loading part in mind?
leofang
added
enhancement
It'd be useful in general, not just here. It's just a few lines of Python code that is very similar to other functionality in pathfinder, and does not introduce any new dependencies. Having the function there, would make what's needed here a no-brainer. |
| if CUDA_PATH is None: | ||
| return None |
There was a problem hiding this comment.
Note: I changed this to return None if no env var is set, but I think it is a mistake because we still need paths to the headers when creating Extension instances below. I think it is time to evaluate using pathfinder. It is already a build-time dependency anyway (cuda-bindings brought it in).
FWIW it seems #692 suddenly becomes within reach!
| cuda_version = _get_cuda_driver_version() | ||
| if cuda_version: | ||
| return str(cuda_version // 1000) |
There was a problem hiding this comment.
Since we must need CUDA headers at build time, this is no longer needed in terms of deciding cuda-bindings version, but it can be a useful check in case there is a local driver installation that cannot support the package being built. We can raise a nice warning in such cases via this check.
|
I put this in the freezer for now, but the branch is still around: freezer/cuda_core_build_hooks_get_cuda_version_macro |
Removed preview folders for the following PRs: - PR #1085
2 participants
WIP — Will work on it more asap.