Skip to content
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 Numba Config to turn on Pynvjitlink Features #17628

Open
wants to merge 9 commits into
base: branch-25.02
Choose a base branch
from

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Dec 19, 2024

Description

Numba-cuda 0.0.18+ merged in a new feature and made the old way of patching linker with pynvjitlink's patch_numba_linker no longer usable by downstream libraries. The current state of Numba-cuda requires that downstream libraries to enable pynvjitlink features only via CUDA_ENABLE_PYNVJITLINK environment variable. A recent PR NVIDIA/numba-cuda#91 makes it so that the features can be turned on by a config variable at runtime.

This PR is an integration test with that PR and changing the way how pynvjitlink is enabled in cuDF. It enables cuDF to use Numba-cuda since 0.2.0+ (which contains the config change).

Supercedes #17359

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Copy link

copy-pr-bot bot commented Dec 19, 2024

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions bot added the Python Affects Python cuDF API. label Dec 19, 2024
@isVoid isVoid added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Dec 19, 2024
@isVoid isVoid marked this pull request as ready for review December 20, 2024 14:12
@isVoid isVoid requested review from a team as code owners December 20, 2024 14:12
@@ -688,7 +688,7 @@ dependencies:
- output_types: [conda, requirements, pyproject]
packages:
- cachetools
- &numba-cuda-dep numba-cuda>=0.0.13,<0.0.18
- &numba-cuda-dep numba-cuda>=0.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to keep an upper bound here. Numba-cuda is not stable yet, afaik.

Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

There's a suggestion from Bradley, but otherwise LGTM. Thanks @isVoid .

Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

#17359 also has some changes in aggregation.pyx, do we not need those?

@isVoid
Copy link
Contributor Author

isVoid commented Dec 31, 2024

#17359 also has some changes in aggregation.pyx, do we not need those?

I think those changes are to avoid importing any of cudf.utils.cudautils (which transitively imports numba.cuda) in the cudf startup phase. We don't want that because before NVIDIA/numba-cuda#91, MVC mode is enabled via setting the ENABLE_PYNVJITLINK numba config before importing numba-cuda. With that PR merged, we removed such ordering requirement, aka, we can either import numba-cuda first, then change the config to enable/disable nvjitlinker when needed; Or change the setting, then import numba-cuda.

@isVoid
Copy link
Contributor Author

isVoid commented Dec 31, 2024

The doc-build CI job is failing:

dumping search index in English (code: en)... done
dumping object inventory... done
build finished with problems, 21 warnings (with warnings treated as errors).
make: *** [Makefile:20: dirhtml] Error 1
/__w/cudf/cudf

But I don't see any warnings output in the CI. Are there any warning suppression in the CI?

@vyasr
Copy link
Contributor

vyasr commented Dec 31, 2024

But I don't see any warnings output in the CI. Are there any warning suppression in the CI?

No, you probably just need to scroll up further. There are a set of warnings printed after all the "reading sources" lines in before all of the "writing output" lines.

I see them here, but they look odd and completely unrelated to the changes in this PR so I am skeptical of how meaningful they are. I've kicked off a rerun to see if they show up again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants