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

[FEA] CEC for CUDA 12: using nvjitlink (instead of ptxcompiler / cubinlinker) #12822

Closed
sevagh opened this issue Feb 22, 2023 · 10 comments · Fixed by #13650
Closed

[FEA] CEC for CUDA 12: using nvjitlink (instead of ptxcompiler / cubinlinker) #12822

sevagh opened this issue Feb 22, 2023 · 10 comments · Fixed by #13650
Labels
feature request New feature or request

Comments

@sevagh
Copy link
Contributor

sevagh commented Feb 22, 2023

cuDF uses ptxcompiler and cubinlinker for CEC. Will this be changing for CUDA 12? We should discuss.

@bdice @gmarkall @vyasr

@sevagh sevagh added feature request New feature or request Needs Triage Need team to review and classify labels Feb 22, 2023
@bdice bdice removed the Needs Triage Need team to review and classify label Feb 22, 2023
@bdice
Copy link
Contributor

bdice commented Feb 22, 2023

In my understanding, we have (at least) two options:

  1. Create ptxcompiler/cubinlinker packages for CUDA 12 that do "nothing" (or at least less work than the current packages by leveraging nvJitLink?) -- then we don't have to make CUDA-version-dependent lists of dependencies for cuDF.
  2. Alter cuDF in a way that doesn't depend on ptxcompiler/cubinlinker packages when building for CUDA 12 (but this may require some kind of nvJitLink support built into cuDF?). This requires CUDA version knowledge when determining cuDF dependencies.

Happy to be corrected if anything above is inaccurate.

@vyasr
Copy link
Contributor

vyasr commented Feb 23, 2023

My understanding was that this wouldn't be a build-time issue at all, but that numba would "just work" when using CUDA 12 JITted kernels across different minor CUDA versions (probably by using nvJitLink under the hood). I thought that all cudf would need to do is to make the cubinlinker and ptxcompiler requirements conditional on the CUDA version.

@sevagh
Copy link
Contributor Author

sevagh commented Feb 23, 2023

The only issue I encountered when test-driving CUDA 12 pip wheel builds for cuDF (i.e. cudf-cu12, dask-cudf-cu12) was in the test step, where the install of cudf-cu12 couldn't find ptxcompiler-cu12, cubinlinker-cu12 to install from its requirements.

So, if the only change for CUDA 12 is to delete those lines (perhaps using ci/release/apply_wheel_modifications.sh, Vyas' new mechanism for modifying setuptools/python package files), that's fine too.

@vyasr
Copy link
Contributor

vyasr commented Feb 23, 2023

If simply removing the requirements makes everything work then yeah we could use that script (although it wouldn't be enough by itself, we would also need to do something for conda packages.

@bdice
Copy link
Contributor

bdice commented Feb 23, 2023

Conda packages are easier because we can template the meta.yaml on the RAPIDS_CUDA_VERSION environment variable. That will already be necessary to make CUDA 11/12 compatible sets of CUDA dependencies.

@vyasr
Copy link
Contributor

vyasr commented Feb 23, 2023

Yup I agree, it's not hard. Just one more thing to do in addition to what we'd do for wheels.

@jakirkham
Copy link
Member

cc @gmarkall

@jakirkham
Copy link
Member

(bump)

@vyasr
Copy link
Contributor

vyasr commented May 11, 2023

Short term proposal for 23.06: #13339

@jakirkham jakirkham changed the title [FEA] CEC for CUDA 12: future of cubinlinker and ptxcompiler [FEA] CEC for CUDA 12: using nvjitlink (instead of ptxcompiler / cubinlinker) Jun 21, 2023
@jakirkham
Copy link
Member

Have tried to retitle this issue to make it clearer to others. Please feel free to revise if there is a better way to capture the intent here

rapids-bot bot pushed a commit that referenced this issue Nov 20, 2023
Fixes #12822

This PR provides minor version compatibility in the CUDA 12.x range through `nvjitlink` via the preliminary [nvjiitlink python binding](https://github.com/gmarkall/nvjitlink). Thus far this PR merely leverages a local installation of the library and should not be merged until `nvjitlink` is hosted on `conda-forge` and cuDF's dependencies are adjusted accordingly, likely as part of this PR.

Authors:
  - https://github.com/brandon-b-miller
  - Ashwin Srinath (https://github.com/shwina)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Ashwin Srinath (https://github.com/shwina)

URL: #13650
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants