-
Notifications
You must be signed in to change notification settings - Fork 917
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
cuDF numba cuda 12 updates #13337
Merged
rapids-bot
merged 42 commits into
rapidsai:branch-23.06
from
brandon-b-miller:cudf-numba-cuda12-updates
May 23, 2023
Merged
cuDF numba cuda 12 updates #13337
Changes from 19 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
76109ce
move functions, use config option to enable mvc, do so before importi…
brandon-b-miller de2b678
move more of numbas setup to _numba_setup
brandon-b-miller 442fefc
update comment in __init__
brandon-b-miller f5f915d
add a few docs
brandon-b-miller 19dd82c
add a debug statement for now
brandon-b-miller d360008
only raise in cec mode
brandon-b-miller 9c76c61
try bumping to numba 0.57
brandon-b-miller 950f98f
conditionally import ptxcompiler
brandon-b-miller c8142ea
update comments a bit
brandon-b-miller 8c7bae8
Apply suggestions from code review
brandon-b-miller c4edd0e
Merge branch 'cudf-numba-cuda12-updates' of github.com:brandon-b-mill…
brandon-b-miller b8d290d
_numba_setup -> _setup_numba
brandon-b-miller a50c642
address more reviews
brandon-b-miller 6ba957c
Merge branch 'branch-23.06' into cudf-numba-cuda12-updates
brandon-b-miller 96b6f01
use a context manager to squash occupancy warnings for numba kernels
brandon-b-miller 47d8a2e
revert numba upgrade
brandon-b-miller 66226c6
Merge branch 'branch-23.06' into cudf-numba-cuda12-updates
brandon-b-miller b9634f9
adjust logic, introduce runtime check in apply/groupby udfs
brandon-b-miller 7a594b3
Address reviews
brandon-b-miller cf642d0
partially address reviews
brandon-b-miller e7b49e9
merge latest and resolve conflicts
brandon-b-miller cb5a756
Revert "revert numba upgrade"
brandon-b-miller dcc73e1
_setup_numba.py -> _numba.py, CUDFNumbaConfig -> _CUDFNumbaConfig
brandon-b-miller 053193a
try vendoring some ptxcompiler code
brandon-b-miller c2285fa
add the comment about the MVC config option and numba.cuda imports ba…
brandon-b-miller b72eef0
fix imports
brandon-b-miller bd27a2f
switch error
brandon-b-miller 8c9c070
slightly adjust logic
brandon-b-miller 662b30b
add missing return
brandon-b-miller 93af613
shuffle imports
brandon-b-miller 2ff5c5d
delete explicit runtime check for MVC in cuda 12+ as it's needed more…
brandon-b-miller 5cb0ce6
attempt a simplifying change
brandon-b-miller fc69663
update ptx/ctk version mapping table
brandon-b-miller 0f1079c
merge latest and resolve conflicts
brandon-b-miller 0797cde
fix local imports
brandon-b-miller e799992
remove extraneous testing code
brandon-b-miller 41e92a9
Apply suggestions from code review
brandon-b-miller 8839f8c
cleanup
brandon-b-miller c27a4b1
clarify cuda 12 comments
brandon-b-miller 6925612
version map changes
brandon-b-miller 439a667
remove function from ptxcompiler that is not used
brandon-b-miller 1bfb382
address remaining reviews
brandon-b-miller File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Any magic in the import order is usually good to explicitly describe for readers.
Is it dangerous / invalid if a user imports
numba.cuda
in their own code before importing cudf?numba
import?)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.
My interpretation of @brandon-b-miller's response here was that in fact there is no longer an explicit ordering issue because we are using the numba config API rather than an environment variable. If there is still an ordering issue then I remain confused about my question in the other thread because it seems like we would be breaking the ordering there.
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.
There's only an ordering issue if we use the config option from numba 0.57. I believe the linker patching approach (which is taken in this PR at this point) is not perturbed by when
numba.cuda
is imported.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.
So this is still an item of concern because we've pivoted back to putting numba 0.57 into this PR. Currently the call to
_setup_numba
is placed very early on in cudf's__init__
even beforevalidate_setup()
is called, asvalidate_setup()
callsrmm._gpu.runtimeGetVersion()
which actually importsnumba.cuda
as a side effect.The order of imports does matter here. In addition there's @bdice 's observation that a user importing
numba.cuda
somewhere in their workflow beforecudf
is imported is likely to break cuDF. So certainly this is an issue we need to come to some consensus about before merging.One option I can think of that could work is detecting this case dynamically somehow and issuing a warning to the user that instructs them to set
CUDA_ENABLE_MINOR_VERSION_COMPATIBILITY=1
in their environment. This should set the variable globally before the user's import.