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

feat: add __dlpack__, from_dlpack support #2649

Merged
merged 10 commits into from
Aug 16, 2023
Merged

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Aug 15, 2023

This PR fixes #2648.

  • Adds Array.__dlpack__(stream=None) and Array.__dlpack_device__()
  • Adds ak.from_dlpack
  • Fixes copying from CUDA in to_nplike
  • Replaces Array.__array__ with Array.__array_interface__ and Array.__cuda_array_interface__ (as these are higher precedence, and symmetric to one-another. It's arbitrary, we could keep __array__, but I think __array_interface__ should be fairly well supported).

@agoose77 agoose77 changed the title feat: add from_dlpack operation to backends feat: add from_dlpack operation to backends Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #2649 (884bed5) into main (a960a56) will decrease coverage by 0.05%.
The diff coverage is 70.24%.

❗ Current head 884bed5 differs from pull request most recent head 976e7d1. Consider uploading reports for the commit 976e7d1 to get more accurate results

Additional details and impacted files
Files Changed Coverage Δ
src/awkward/_connect/numpy.py 92.43% <ø> (+0.25%) ⬆️
src/awkward/_nplikes/typetracer.py 76.17% <44.44%> (-0.36%) ⬇️
src/awkward/contents/recordarray.py 85.01% <50.00%> (-0.14%) ⬇️
src/awkward/operations/ak_from_dlpack.py 55.17% <55.17%> (ø)
src/awkward/highlevel.py 76.41% <62.06%> (-0.57%) ⬇️
src/awkward/_nplikes/numpylike.py 73.74% <63.63%> (-0.34%) ⬇️
src/awkward/_connect/dlpack.py 93.54% <93.54%> (ø)
src/awkward/_nplikes/__init__.py 80.95% <100.00%> (ø)
src/awkward/_nplikes/array_module.py 88.88% <100.00%> (+0.16%) ⬆️
src/awkward/_nplikes/cupy.py 37.20% <100.00%> (+0.73%) ⬆️
... and 3 more

@agoose77 agoose77 temporarily deployed to docs-preview August 15, 2023 18:21 — with GitHub Actions Inactive
Comment on lines +60 to +63
nplike: NumpyLike
if device_type == DLPackDevice.CPU:
nplike = Numpy.instance()
elif device_type == DLPackDevice.CUDA:
nplike = Cupy.instance()
elif device_type == DLPackDevice.CUDA_PINNED:
# TODO: this should support GPU
# nplike = (Numpy if prefer_cpu else Cupy).instance()
nplike = Numpy.instance()
elif device_type == DLPackDevice.ROCM:
nplike = Cupy.instance()
elif device_type == DLPackDevice.ROCM_PINNED:
nplike = (Numpy if prefer_cpu else Cupy).instance()
elif device_type == DLPackDevice.CUDA_MANAGED:
nplike = (Numpy if prefer_cpu else Cupy).instance()
else:
raise AssertionError
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could generalise this to include JAX. However, I think this is a lot simpler to reason about for now.

@agoose77 agoose77 changed the title feat: add from_dlpack operation to backends feat: add __dlpack__, from_dlpack support Aug 15, 2023
@agoose77 agoose77 marked this pull request as ready for review August 15, 2023 20:46
@agoose77 agoose77 temporarily deployed to docs-preview August 15, 2023 20:59 — with GitHub Actions Inactive
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

This converts from DLPack arrays (ak.from_dlpack) and converts single-buffer Awkward Arrays to DLPack.

Just to be clear, that "single-buffer Awkward Arrays" is a proper subset of "Awkward Arrays that won't fail in ak.to_numpy":

>>> array = ak.Array([
...     [{"x": 1, "y": 1.1}, {"x": 2, "y": 2.2}], 
...     [{"x": 3, "y": 3.3}, {"x": 4, "y": 4.4}],
... ])
>>> ak.to_numpy(array)    # also np.asarray(array)
array([[(1, 1.1), (2, 2.2)],
       [(3, 3.3), (4, 4.4)]], dtype=[('x', '<i8'), ('y', '<f8')])

>>> array = ak.Array([1.1, 2.2, None, 4.4, None, None, 7.7])
>>> ak.to_numpy(array)    # NOT np.asarray(array)
masked_array(data=[1.1, 2.2, --, 4.4, --, --, 7.7],
             mask=[False, False,  True, False,  True,  True, False],
       fill_value=1e+20)

In both of the above, array contains multiple buffers that might be on different GPUs in the "cuda" backend, so it's good that they'll fail to be converted to DLPack.


It looks good to me; go ahead and merge it if you don't have any last-minute changes.

Comment on lines +1119 to +1124
if not backend.nplike.supports_structured_dtypes:
raise TypeError(
f"backend {backend.name!r} does not support structured "
f"dtypes required for converting {type(self).__name__} "
f"into a backend array"
)
Copy link
Member

Choose a reason for hiding this comment

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

You noticed this: okay, so maybe I didn't need to tell you.

Copy link
Collaborator Author

@agoose77 agoose77 Aug 15, 2023

Choose a reason for hiding this comment

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

This is a cleanup more than a feature; NumPy does support both DLPack and structured dtypes. However, NumPy doesn't support structured dtypes with DLPack, and will report that the conversion fails, e.g.

>>> import awkward as ak
>>> array = ak.Array([{'x': 1}]).to_numpy()
>>> ptr = array.__dlpack__()
Traceback (most recent call last):
  File "...", line 5, in <module>
    ptr = array.__dlpack__()
          ^^^^^^^^^^^^^^^^^^
BufferError: DLPack only supports signed/unsigned integers, float and complex dtypes.
[('x', '<i8')]

@agoose77 agoose77 enabled auto-merge (squash) August 16, 2023 13:46
@agoose77 agoose77 disabled auto-merge August 16, 2023 20:27
@agoose77 agoose77 merged commit eff0a81 into main Aug 16, 2023
32 checks passed
@agoose77 agoose77 deleted the agoose77/feat-from-dlpack branch August 16, 2023 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for DLPack
2 participants