-
Notifications
You must be signed in to change notification settings - Fork 916
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
Adapt cudf numba config for numba 0.61 removal #17705
base: branch-25.02
Are you sure you want to change the base?
Adapt cudf numba config for numba 0.61 removal #17705
Conversation
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 thanks.
I looked briefly to see if we could avoid this kind of issue in the future by erroring if we got a deprecation warning from numba, but it looks like that warning was only emitted when the value was set to old_style
: https://github.com/numba/numba/blob/53e976f1b0c6683933fa0a93738362914bffc1cd/numba/core/config.py#L93-L96.
python/cudf/cudf/utils/_numba.py
Outdated
numba_config.CUDA_LOW_OCCUPANCY_WARNINGS | ||
) | ||
numba_config.CUDA_LOW_OCCUPANCY_WARNINGS = 0 | ||
@contextlib.contextmanager |
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.
Sometimes we choose to manually implement context managers because the overhead of contextlib.contextmanager
is pretty high. Is this such a case? The history / blame might tell us.
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 appears this was added in #13337 without comment about performance, but locally I do see that the decorator version is almost 4x slower than the class version so I'll change this back to the class version.
Description
closes #17703
Also refactored the old-style class context manager with the new-style
@contextlib.contextmanager
decorator.Checklist