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

Numba 0.57 compatibility fixes #13271

Merged
merged 3 commits into from
May 4, 2023

Conversation

gmarkall
Copy link
Contributor

@gmarkall gmarkall commented May 2, 2023

Description

For compatibility with Numba 0.57 (required for full CUDA 12.0 support) the following changes are necessary:

  • We can no longer use Record types directly, because Numba 0.57 assumes that it can convert a Record back to a NumPy dtype using its internal mechanism. The internal mechanism doesn't recognize cuDF extension types and has no provision for adding support for new types. To work around this, we directly encode a correspondence between our Row type (directly derived from a Record) and the NumPy object dtype. An object dtype is acceptable for this use because we use our row types for type checking and inference, but not code generation. Because code generation is not needed, we don't need to have an accurate representation of the low-level layout of a Row in the typing.
  • A deprecated address space conversion function was removed from Numba. We switch to using another method that performs the address space conversion itself.
  • The data layout moved from the nvvm module on to an NVVM object. Rather than attempting to get this from Numba, we just define it in cuDF, since there is only one possible data layout in CUDA anyway. This allows the same code path to work with all Numba versions (and removes a previous workaround from it having once moved before).

These changes should be compatible with both Numba 0.56 (the minimum required version at present) and 0.57 (can be used, will become a requirement soon).

Tested locally with Numba 0.57.0rc1 (which doesn't differ from the recently-tagged release in any way material to CUDA).

Checklist

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

For compatibility with Numba 0.57 (required for full CUDA 12.0 support)
the following changes are necessary:

- We can no longer use `Record` types directly, because Numba 0.57 assumes
  that it can convert a `Record` back to a NumPy dtype using its
  internal mechanism. The internal mechanism doesn't recognize cuDF
  extension types and has no provision for adding support for new types.
  To work around this, we directly encode a correspondence between our
  `Row` type (directly derived from a `Record`) and the NumPy `object`
  dtype. An `object` dtype is acceptable for this use because we use our
  row types for type checking and inference, but not code generation.
  Because code generation is not needed, we don't need to have an
  accurate representation of the low-level layout of a `Row` in the
  typing.
- A deprecated address space conversion function was removed from Numba.
  We switch to using another method that performs the address space
  conversion itself.
- The data layout moved from the `nvvm` module on to an `NVVM` object.
  Rather than attempting to get this from Numba, we just define it in
  cuDF, since there is only one possible data layout in CUDA anyway.
  This allows the same code path to work with all Numba versions (and
  removes a previous workaround from it having once moved before).

These changes should be compatible with both Numba 0.56 (the minimum
required version at present) and 0.57 (can be used, will become a
requirement soon).
@gmarkall gmarkall requested a review from a team as a code owner May 2, 2023 12:13
@gmarkall gmarkall requested review from bdice and charlesbluca May 2, 2023 12:13
@github-actions github-actions bot added the Python Affects Python cuDF API. label May 2, 2023
@gmarkall gmarkall added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 2, 2023
@bdice
Copy link
Contributor

bdice commented May 4, 2023

@gmarkall (cc: @vyasr @divyegala for pip wheels): I see this PR is compatible with 0.56 as well as 0.57. Will we need to bump the minimum version to 0.57 for CUDA 12 packages? For now we only want pip packages that support CUDA 12, so the blockage on the conda-forge release of numba 0.57 is not an issue yet.

@gmarkall
Copy link
Contributor Author

gmarkall commented May 4, 2023

Will we need to bump the minimum version to 0.57 for CUDA 12 packages?

Yes - Numba 0.56 can't find the libraries in the locations that the CUDA 12 packages put them.

For now we only want pip packages that support CUDA 12, so the blockage on the conda-forge release of numba 0.57 is not an issue yet.

Might it be an issue for our testing / CI though? Won't we need Numba 0.57 in conda-forge to be able to generally develop and test against 0.57?

@bdice
Copy link
Contributor

bdice commented May 4, 2023

Might it be an issue for our testing / CI though? Won't we need Numba 0.57 in conda-forge to be able to generally develop and test against 0.57?

Yes, you're correct - after merging this, we'll probably apply a patch in #13289 that only uses numba 0.57 for CUDA 12 wheels until conda-forge has numba 0.57, too. @vyasr @divyegala

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

All my questions about the major changes here were addressed in @vyasr's review and in my comments on the PR as a whole. Thanks so much for this, @gmarkall!

@vyasr
Copy link
Contributor

vyasr commented May 4, 2023

/merge

@rapids-bot rapids-bot bot merged commit 427e2af into rapidsai:branch-23.06 May 4, 2023
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants