Skip to content

[MNT] Drop NUMBA_CUDA_USE_NVIDIA_BINDING; always use cuda.core and cuda.bindings as fallback #479

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 46 commits into from
Oct 14, 2025
Merged

[MNT] Drop NUMBA_CUDA_USE_NVIDIA_BINDING; always use cuda.core and cuda.bindings as fallback #479

merged 46 commits into from
Oct 14, 2025

Conversation

@rparolin
Copy link
Contributor

@rparolin rparolin commented Sep 19, 2025 •
edited
Loading

Summary

Historically, numba-cuda supported two binding layers (ctypes and cuda-python). This change simplifies maintenance and aligns with the long-term direction to use cuda.core while leveraging cuda.bindings to fill gaps.

What changed?

  • Remove the NUMBA_CUDA_USE_NVIDIA_BINDING environment variable and all ctypes-based CUDA paths.
  • Require CUDA Python package at import.
  • Always prefer cuda.core APIs; use cuda.bindings where required
  • Tests updated to remove conditional paths

Breaking changes

  • The NUMBA_CUDA_USE_NVIDIA_BINDING env var no longer exists.
  • Legacy ctypes-based NVRTC/driver paths are removed.
  • cuda/cuda.bindings must be installed for Numba-CUDA to import.

Fixes: #154

@copy-pr-bot
Copy link

copy-pr-bot bot commented Sep 19, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@rparolin rparolin marked this pull request as ready for review September 19, 2025 00:52
@rparolin

This comment was marked as spam.

Sign in to view
@rparolin

This comment was marked as outdated.

Sign in to view
@rparolin

This comment was marked as outdated.

Sign in to view
@NVIDIA NVIDIA deleted a comment from copy-pr-bot bot Sep 19, 2025
@rparolin

This comment was marked as outdated.

Sign in to view
@rparolin

This comment was marked as outdated.

Sign in to view
brandon-b-miller
brandon-b-miller reviewed Sep 19, 2025
View reviewed changes
@rparolin

This comment was marked as outdated.

Sign in to view
@rparolin

This comment was marked as outdated.

Sign in to view
@rparolin
Copy link
Contributor Author

rparolin commented Sep 19, 2025

/ok to test 474a1a2

@rparolin
Copy link
Contributor Author

rparolin commented Sep 20, 2025

/ok to test c1e11ce

@rparolin rparolin marked this pull request as draft September 20, 2025 00:24
@rparolin
Copy link
Contributor Author

rparolin commented Sep 22, 2025

/ok to test 3f0ceb3

@rparolin
Copy link
Contributor Author

rparolin commented Sep 22, 2025

/ok to test 35b172b

@gmarkall gmarkall disabled auto-merge October 10, 2025 11:50
@rparolin
Copy link
Contributor Author

rparolin commented Oct 10, 2025

/ok to test f8b4d8c

cpcloud
cpcloud reviewed Oct 10, 2025
View reviewed changes
numba_cuda/numba/cuda/__init__.py
# Require NVIDIA CUDA bindings at import time
if not (
importlib.util.find_spec("cuda")
and importlib.util.find_spec("cuda.bindings")
Copy link
Contributor

@cpcloud cpcloud Oct 10, 2025

Choose a reason for hiding this comment

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

This should be an impossible code path without actively trying to create a broken installation, so we can remove these checks.

Copy link
Contributor

@cpcloud cpcloud Oct 13, 2025

Choose a reason for hiding this comment

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

I will change this in a follow-up.

@gmarkall gmarkall mentioned this pull request Oct 13, 2025
Open
@gmarkall
Copy link
Contributor

gmarkall commented Oct 13, 2025

I'm intermittently seeing #517 with this PR as well. I'm still looking to understand if that's an existing bug or a side-effect of this PR.

gmarkall
gmarkall reviewed Oct 13, 2025
View reviewed changes
numba_cuda/numba/cuda/cudadrv/driver.py
)
# For backwards compatibility: indicate that the NVIDIA CUDA Python bindings are
# in use. Older code checks this flag to branch on binding-specific behavior.
USE_NV_BINDING = True
Copy link
Contributor

@gmarkall gmarkall Oct 13, 2025

Choose a reason for hiding this comment

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

I'm not aware of code that checks driver.USE_NV_BINDING. The code I was thinking of that checks whether the NVIDIA bindings are in use checks config.CUDA_USE_NVIDIA_BINDING. E.g.: https://github.com/rapidsai/rmm/blob/branch-25.12/python/rmm/rmm/allocators/numba.py#L76

gmarkall
gmarkall requested changes Oct 13, 2025
View reviewed changes
Copy link
Contributor

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

I think this needs

diff --git a/numba_cuda/numba/cuda/cudadrv/nvrtc.py b/numba_cuda/numba/cuda/cudadrv/nvrtc.py
index 868397b3..0f50be8f 100644
--- a/numba_cuda/numba/cuda/cudadrv/nvrtc.py
+++ b/numba_cuda/numba/cuda/cudadrv/nvrtc.py
@@ -115,6 +115,8 @@ def compile(src, name, cc, ltoir=False, lineinfo=False, debug=False):
         relocatable_device_code=True,
         link_time_optimization=ltoir,
         name=name,
+        debug=debug,
+        lineinfo=lineinfo
     )
 
     class Logger:

to address #479 (comment), and that this is why tests are still failing.

@rparolin
Copy link
Contributor Author

rparolin commented Oct 13, 2025

/ok to test 7f98a05

@rparolin rparolin requested review from cpcloud and gmarkall October 13, 2025 19:52
@rparolin
Copy link
Contributor Author

rparolin commented Oct 13, 2025

/ok to test ee2c048

@cpcloud cpcloud enabled auto-merge (squash) October 13, 2025 20:37
@rparolin
Copy link
Contributor Author

rparolin commented Oct 13, 2025

/ok to test 6d886d6

@gmarkall gmarkall disabled auto-merge October 14, 2025 08:12
@gmarkall
Copy link
Contributor

gmarkall commented Oct 14, 2025

/ok to test

@copy-pr-bot
Copy link

copy-pr-bot bot commented Oct 14, 2025

/ok to test

@gmarkall, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

@gmarkall
Copy link
Contributor

gmarkall commented Oct 14, 2025

/ok to test 92b9be2

@gmarkall gmarkall enabled auto-merge (squash) October 14, 2025 08:21
@gmarkall gmarkall merged commit 2df2c59 into main Oct 14, 2025
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpcloud cpcloud cpcloud approved these changes

@gmarkall gmarkall gmarkall approved these changes

@leofang leofang Awaiting requested review from leofang

@brandon-b-miller brandon-b-miller Awaiting requested review from brandon-b-miller

@isVoid isVoid Awaiting requested review from isVoid

@kkraus14 kkraus14 Awaiting requested review from kkraus14

Assignees

@gmarkall gmarkall

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[MNT] Drop CUDA_USE_NVIDIA_BINDING and always use cuda.core (and fallback to cuda.bindings)

7 participants

@rparolin @cpcloud @gmarkall @kkraus14 @isVoid @brandon-b-miller