-
Notifications
You must be signed in to change notification settings - Fork 903
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
Use ptxcompiler to patch Numba at runtime to support CUDA enhanced compatibility. #9687
Conversation
This looks good so far on inspection. |
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.
It sounds like we're not going to require ptxcompiler
for ARM (and not even build the package for it) and just require CUDA 11.5 for it - in that case, I think we'll also need to depend on ptxcompiler
conditionally, and guard its use with try ... except ImportError
.
Co-authored-by: Graham Markall <[email protected]>
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.
Per Graham's question about handling ImportError
s. Maybe the changes below would be sufficient?
python/cudf/cudf/__init__.py
Outdated
# Patch Numba to support CUDA enhanced compatibility. | ||
# See https://github.com/rapidsai/ptxcompiler for | ||
# details. | ||
patch_numba_codegen_if_needed() | ||
del patch_numba_codegen_if_needed | ||
|
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.
Removed as added above
# Patch Numba to support CUDA enhanced compatibility. | |
# See https://github.com/rapidsai/ptxcompiler for | |
# details. | |
patch_numba_codegen_if_needed() | |
del patch_numba_codegen_if_needed |
@@ -51,6 +51,7 @@ requirements: | |||
- nvtx >=0.2.1 |
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.
Don't you need to increase the numba pin in this file as well?
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.
Fixed!
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.
Looks good to me!
rerun tests |
rerun tests |
This will make cudf change the behavior of Numba on import now, correct? It's a bit of a shame that we're adding additional side effects to importing cudf that are arguably more impactful than the memory allocator side effects we currently have... |
It patches Numba only in an Enhanced Compatibility scenario - when the toolkit version is newer than the driver version. When the driver version is sufficient, no patching occurs. In other words, the only scenario in which Numba is patched is when it wouldn't have worked at all. In the long term I expect Numba will use ptxcompiler by default, but it also needs a cubin linker than can be linked statically too, which is not available at present. This will give Numba general support for Enhanced Compatibility, so no patch would be needed in any case. |
No description provided.