search

NVIDIA / cuda-python

CUDA Python Low-level Bindings
https://nvidia.github.io/cuda-python/
Other
811 stars 63 forks source link

cuda.cudart.cudaRuntimeGetVersion() hard-codes the runtime version, rather than querying the runtime #16

Closed bdice closed 8 months ago

bdice commented 2 years ago

The current implementation of cuda.cudart.cudaRuntimeGetVersion() hard-codes the runtime version, rather than querying the runtime for its version. This results in incorrect runtime versions if the runtime version is different from the version of cuda-python.

https://github.com/NVIDIA/cuda-python/blob/746b773c91e1ede708fe9a584b8cdb1c0f32b51d/cuda/_lib/ccudart/ccudart.pyx#L79-L82

https://github.com/NVIDIA/cuda-python/blob/746b773c91e1ede708fe9a584b8cdb1c0f32b51d/cuda/_lib/ccudart/utils.pyx#L37

Additional context

A workaround used in https://github.com/rapidsai/rmm/pull/946 is to use numba's API for this instead:

import numba.cuda

def cudaRuntimeGetVersion():
    major, minor = numba.cuda.runtime.get_version()
    return major * 1000 + minor * 10
jakirkham commented 2 years ago

cc @vzhurba01

kmaehashi commented 2 years ago

This is a good finding, @bdice! This issue also impacts CuPy, as it heavily relies on cudaRuntimeGetVersion to test the availability of features. https://github.com/cupy/cupy/blob/514457a58b34baa386ace365b997e49698bf36d3/cupy_backends/cuda/libs/nvrtc.pyx#L83-L84

vzhurba01 commented 2 years ago

Thanks @bdice! Sorry for the late response, for now this is a known limitation of CUDA Python which first requires a fix in the CUDA Runtime library. Let me explain...

Problem: The CUDA Runtime library has a race-condition on Windows with regards to the DLL unload order resulting in a crash. This was reported to happen often enough to be a very frustrating experience for users, so CUDA Python needed to find an alternative.

Solution: What CUDA Python does today is it actually re-writes the CUDA Runtime library as seen in cuda/_lib/ccudart. While it does contain some simplification, it is functionally equivalent to the latest CUDA Runtime release. This avoids loading the CUDA Runtime library, avoiding the race-condition.

Next Step: Once this race-condition is resolved in the CUDA library, we'll switch to loading and calling into the library (just like CUDA Driver and NVRTC) and remove the rewritten CUDA Runtime. Since the rewritten Runtime is functionally equivalent, there should be no noticeable change once this transition is complete. I know that the work for this race-condition has been scheduled, but I don't know when it's expected to be resolved.

How this relates to the cudaRuntimeGetVersion, it's value will always be the latest toolkit because the rewritten Runtime will always be up-to-date. In the meantime I'm really sorry for the awkwardness this has caused.

bdice commented 2 years ago

Thanks @vzhurba01! Do you know if this rewriting behavior would cause any other unexpected behavior besides checking the runtime version? We may need workarounds for other functions if so. I am not familiar enough with the API surface area to think of any off the top of my head, but it seems possible.

vzhurba01 commented 2 years ago

I've just scanned through the APIs and nothing else stood out to me. I think this is a one-off case because it uses a global value as defined by the library.

But I should also mention this: The usage of cuda.cudart.CUDART_VERSION is not the same as C users and might cause confusion.

cuda.cudart.CUDART_VERSION is for CUDA Python cuda.cudart.cudaRuntimeGetVersion() is for CUDA Runtime library

All other APIs appear to simply format parameters between Runtime and Driver, then call into driver. While the logic of APIs would be different between older Runtime vs. latest Runtime, this shouldn't be a concern because of backwards comparability.

leofang commented 2 years ago

It seems querying the nvrtc version as the cudart version is a valid workaround? https://github.com/cupy/cupy/pull/6736

bdice commented 1 year ago

@vzhurba01 Following up on this. Is it still impossible to get the true runtime version with cuda-python? We face this issue repeatedly in RMM, Numba, cuDF, cupy, and other libraries. It's a serious need to address for pretty much all Python packages that are using CUDA.

For example:

I know there's been some changes since mid-2022 in how cuda-python works (e.g. ccudart.pxd was changed to ccudart.pxd.in and there's some logic that reads the actual runtime headers, as I understand it). I wondered if any of those changes might have improved the situation here.

leofang commented 8 months ago

Revisiting this issue a little more than a year later, I come with a different viewpoint from I had before.

First of all, thanks to @vzhurba01 this issue is worked around for Linux users in CUDA Python 12.3. A new API cudart.getLocalRuntimeVersion() is added to probe the local libcudart.so version. (btw @vzhurba01, this API is currently undocumented -- it does not appear in the API reference.)

Second, based on Vlad's description on the (old) status quo, one can think of cudart as being statically linked to CUDA Python (which allows users to interact with the statically linked copy from Python). Then, it is perfectly valid that an API like cudaRuntimeGetVersion() returns the version used at build time, since for CUDA Python users the local libcudart.so version is irrelevant.

I think the "new" status quo (having both cudaRuntimeGetVersion() for build-time version and getLocalRuntimeVersion() for "run-time"[^1] version) is a perfect balance. Other CUDA libraries also start offering the former.

[^1]: Quotation added because whether or not this version is useful/relevant is context dependent. For Numba etc, it is. But not so much for pure CUDA Python users.

vzhurba01 commented 8 months ago

With the CUDA Python 12.3.0 release, as @leofang mentioned we now have getLocalRuntimeVersion() to help probe the local libcudart.so. Missing the API reference is now fixed, though description remains similar to cudaRuntimeGetVersion()).

With these two APIs available, looks like this issue is ready to be closed!