Skip to content

Use ptxcompiler to patch Numba at runtime to support CUDA enhanced compatibility. - #9687

Merged
ajschmidt8 merged 26 commits into
NVIDIA:branch-21.12from
shwina:cec-ptxcompiler
Nov 29, 2021
Merged

Use ptxcompiler to patch Numba at runtime to support CUDA enhanced compatibility.#9687
ajschmidt8 merged 26 commits into
NVIDIA:branch-21.12from
shwina:cec-ptxcompiler

Conversation

@shwina

@shwina shwina commented Nov 15, 2021

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions github-actions Bot added conda Python Affects Python cuDF API. labels Nov 15, 2021
@gmarkall

Copy link
Copy Markdown
Contributor

This looks good so far on inspection.

Comment thread conda/environments/cudf_dev_cuda11.0.yml Outdated
Comment thread python/cudf/cudf/__init__.py Outdated
@shwina shwina changed the title Use ptxcompiler to patch Numba at runtime to support CUDA-enhanced compatibility. Use ptxcompiler to patch Numba at runtime to support CUDA enhanced compatibility. Nov 16, 2021
Comment thread conda/recipes/cudf/meta.yaml Outdated
Comment thread python/cudf/cudf/tests/test_extension_compilation.py Outdated
Comment thread python/cudf/cudf/tests/test_extension_compilation.py

@gmarkall gmarkall left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread python/cudf/cudf/tests/test_extension_compilation.py
Comment thread python/cudf/cudf/tests/test_extension_compilation.py Outdated
Co-authored-by: Graham Markall <535640+gmarkall@users.noreply.github.com>
Comment thread conda/environments/cudf_dev_cuda11.2.yml Outdated
Comment thread conda/environments/cudf_dev_cuda11.5.yml Outdated
Comment thread conda/recipes/cudf/meta.yaml Outdated

@jakirkham jakirkham left a comment

Copy link
Copy Markdown
Contributor

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 ImportErrors. Maybe the changes below would be sufficient?

Comment thread python/cudf/cudf/__init__.py Outdated
Comment thread python/cudf/cudf/__init__.py Outdated
Comment on lines +109 to +114
# 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed as added above

Suggested change
# 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

@shwina
shwina marked this pull request as ready for review November 18, 2021 14:04
@shwina
shwina requested review from a team as code owners November 18, 2021 14:04
@shwina
shwina requested review from ramakrishnap-nv and skirui-source and removed request for a team November 18, 2021 14:04
@raydouglass raydouglass added feature request New feature or request non-breaking Non-breaking change labels Nov 18, 2021
@@ -51,6 +51,7 @@ requirements:
- nvtx >=0.2.1

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@gmarkall gmarkall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@galipremsagar

Copy link
Copy Markdown
Contributor

rerun tests

@jakirkham

Copy link
Copy Markdown
Contributor

rerun tests

@kkraus14

Copy link
Copy Markdown
Contributor

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...

@gmarkall

Copy link
Copy Markdown
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants