From c24d3907825aa5932ca5f9f1c28555fbc12064e4 Mon Sep 17 00:00:00 2001 From: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> Date: Thu, 10 Oct 2024 14:39:14 -0700 Subject: [PATCH 1/7] Add to_dlpack/from_dlpack APIs to pylibcudf --- python/cudf/cudf/_lib/interop.pyx | 67 ++------------- python/pylibcudf/pylibcudf/__init__.pxd | 2 + python/pylibcudf/pylibcudf/interop.pxd | 8 ++ python/pylibcudf/pylibcudf/interop.pyx | 83 ++++++++++++++++++- .../pylibcudf/pylibcudf/libcudf/interop.pxd | 10 ++- 5 files changed, 107 insertions(+), 63 deletions(-) create mode 100644 python/pylibcudf/pylibcudf/interop.pxd diff --git a/python/cudf/cudf/_lib/interop.pyx b/python/cudf/cudf/_lib/interop.pyx index 1dc586bb257..ad5ac2e2a01 100644 --- a/python/cudf/cudf/_lib/interop.pyx +++ b/python/cudf/cudf/_lib/interop.pyx @@ -1,24 +1,8 @@ # Copyright (c) 2020-2024, NVIDIA CORPORATION. -from cpython cimport pycapsule -from libcpp.memory cimport unique_ptr -from libcpp.utility cimport move - import pylibcudf -from pylibcudf.libcudf.interop cimport ( - DLManagedTensor, - from_dlpack as cpp_from_dlpack, - to_dlpack as cpp_to_dlpack, -) -from pylibcudf.libcudf.table.table cimport table -from pylibcudf.libcudf.table.table_view cimport table_view - -from cudf._lib.utils cimport ( - columns_from_pylibcudf_table, - columns_from_unique_ptr, - table_view_from_columns, -) +from cudf._lib.utils cimport columns_from_pylibcudf_table from cudf.core.buffer import acquire_spill_lock from cudf.core.dtypes import ListDtype, StructDtype @@ -30,20 +14,9 @@ def from_dlpack(dlpack_capsule): DLPack Tensor PyCapsule is expected to have the name "dltensor". """ - cdef DLManagedTensor* dlpack_tensor = pycapsule.\ - PyCapsule_GetPointer(dlpack_capsule, 'dltensor') - pycapsule.PyCapsule_SetName(dlpack_capsule, 'used_dltensor') - - cdef unique_ptr[table] c_result - - with nogil: - c_result = move( - cpp_from_dlpack(dlpack_tensor) - ) - - res = columns_from_unique_ptr(move(c_result)) - dlpack_tensor.deleter(dlpack_tensor) - return res + return columns_from_pylibcudf_table( + pylibcudf.interop.from_dlpack(dlpack_capsule) + ) def to_dlpack(list source_columns): @@ -54,35 +27,13 @@ def to_dlpack(list source_columns): """ if any(column.null_count for column in source_columns): raise ValueError( - "Cannot create a DLPack tensor with null values. \ - Input is required to have null count as zero." - ) - - cdef DLManagedTensor *dlpack_tensor - cdef table_view source_table_view = table_view_from_columns(source_columns) - - with nogil: - dlpack_tensor = cpp_to_dlpack( - source_table_view + "Cannot create a DLPack tensor with null values. " + "Input is required to have null count as zero." ) - - return pycapsule.PyCapsule_New( - dlpack_tensor, - 'dltensor', - dlmanaged_tensor_pycapsule_deleter + plc_table = pylibcudf.Table( + [col.to_pylibcudf(mode="read") for col in source_columns] ) - - -cdef void dlmanaged_tensor_pycapsule_deleter(object pycap_obj) noexcept: - cdef DLManagedTensor* dlpack_tensor = 0 - try: - dlpack_tensor = pycapsule.PyCapsule_GetPointer( - pycap_obj, 'used_dltensor') - return # we do not call a used capsule's deleter - except Exception: - dlpack_tensor = pycapsule.PyCapsule_GetPointer( - pycap_obj, 'dltensor') - dlpack_tensor.deleter(dlpack_tensor) + return pylibcudf.interop.to_dlpack(plc_table) def gather_metadata(object cols_dtypes): diff --git a/python/pylibcudf/pylibcudf/__init__.pxd b/python/pylibcudf/pylibcudf/__init__.pxd index b98b37fe0fd..516516c1a30 100644 --- a/python/pylibcudf/pylibcudf/__init__.pxd +++ b/python/pylibcudf/pylibcudf/__init__.pxd @@ -12,6 +12,7 @@ from . cimport ( expressions, filling, groupby, + interop, join, labeling, lists, @@ -59,6 +60,7 @@ __all__ = [ "filling", "gpumemoryview", "groupby", + "interop", "join", "lists", "merge", diff --git a/python/pylibcudf/pylibcudf/interop.pxd b/python/pylibcudf/pylibcudf/interop.pxd new file mode 100644 index 00000000000..8d2872b82f0 --- /dev/null +++ b/python/pylibcudf/pylibcudf/interop.pxd @@ -0,0 +1,8 @@ +# Copyright (c) 2024, NVIDIA CORPORATION. + +from pylibcudf.table cimport Table + + +cpdef Table from_dlpack(managed_tensor) + +cpdef to_dlpack(Table input) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 1a03fa5b45b..daaeaac48ff 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -1,6 +1,10 @@ # Copyright (c) 2023-2024, NVIDIA CORPORATION. -from cpython.pycapsule cimport PyCapsule_GetPointer, PyCapsule_New +from cpython.pycapsule cimport ( + PyCapsule_GetPointer, + PyCapsule_New, + PyCapsule_SetName, +) from libc.stdlib cimport free from libcpp.memory cimport unique_ptr from libcpp.utility cimport move @@ -16,11 +20,14 @@ from pylibcudf.libcudf.interop cimport ( ArrowArray, ArrowArrayStream, ArrowSchema, + DLManagedTensor, column_metadata, from_arrow_column as cpp_from_arrow_column, from_arrow_stream as cpp_from_arrow_stream, + from_dlpack as cpp_from_dlpack, to_arrow_host_raw, to_arrow_schema_raw, + to_dlpack as cpp_to_dlpack, ) from pylibcudf.libcudf.table.table cimport table @@ -315,3 +322,77 @@ def _to_arrow_scalar(cudf_object, metadata=None): # Note that metadata for scalars is primarily important for preserving # information on nested types since names are otherwise irrelevant. return to_arrow(Column.from_scalar(cudf_object, 1), metadata=metadata)[0] + + +cpdef Table from_dlpack(managed_tensor): + """ + Convert a DLPack DLTensor into a cudf table. + + For details, see :cpp:func:`cudf::from_dlpack` + + Parameters + ---------- + managed_tensor : PyCapsule + A 1D or 2D column-major (Fortran order) tensor. + + Returns + ------- + Table + Table with a copy of the tensor data. + """ + cdef unique_ptr[table] c_result + cdef DLManagedTensor* dlpack_tensor = PyCapsule_GetPointer( + managed_tensor, "dltensor" + ) + PyCapsule_SetName(managed_tensor, "used_dltensor") + + with nogil: + c_result = move( + cpp_from_dlpack(dlpack_tensor) + ) + + result = Table.from_libcudf(move(c_result)) + dlpack_tensor.deleter(dlpack_tensor) + return result + + +cpdef to_dlpack(Table input): + """ + Convert a cudf table into a DLPack DLTensor. + + For details, see :cpp:func:`cudf::to_dlpack` + + Parameters + ---------- + input : Table + A 1D or 2D column-major (Fortran order) tensor. + + Returns + ------- + PyCapsule + 1D or 2D DLPack tensor with a copy of the table data, or nullptr. + """ + cdef DLManagedTensor *dlpack_tensor + + with nogil: + dlpack_tensor = cpp_to_dlpack( + input.view() + ) + + return PyCapsule_New( + dlpack_tensor, + "dltensor", + dlmanaged_tensor_pycapsule_deleter + ) + + +cdef void dlmanaged_tensor_pycapsule_deleter(object pycap_obj) noexcept: + cdef DLManagedTensor* dlpack_tensor = 0 + try: + dlpack_tensor = PyCapsule_GetPointer( + pycap_obj, "used_dltensor" + ) + # we do not call a used capsule's deleter + except Exception: + dlpack_tensor = PyCapsule_GetPointer(pycap_obj, "dltensor") + dlpack_tensor.deleter(dlpack_tensor) diff --git a/python/pylibcudf/pylibcudf/libcudf/interop.pxd b/python/pylibcudf/pylibcudf/libcudf/interop.pxd index 30b97fdec34..72b5eb7d618 100644 --- a/python/pylibcudf/pylibcudf/libcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/libcudf/interop.pxd @@ -32,11 +32,13 @@ cdef extern from "cudf/interop.hpp" nogil: cdef extern from "cudf/interop.hpp" namespace "cudf" \ nogil: - cdef unique_ptr[table] from_dlpack(const DLManagedTensor* tensor - ) except + + cdef unique_ptr[table] from_dlpack( + const DLManagedTensor* managed_tensor + ) except + - DLManagedTensor* to_dlpack(table_view input_table - ) except + + DLManagedTensor* to_dlpack( + table_view input + ) except + cdef cppclass column_metadata: column_metadata() except + From 41d9c53df26ab6136a0b613d220735460043f30b Mon Sep 17 00:00:00 2001 From: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com> Date: Thu, 10 Oct 2024 15:24:04 -0700 Subject: [PATCH 2/7] Add test --- python/pylibcudf/pylibcudf/tests/test_interop.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/python/pylibcudf/pylibcudf/tests/test_interop.py b/python/pylibcudf/pylibcudf/tests/test_interop.py index 01c998f16d4..934d9b4ab8b 100644 --- a/python/pylibcudf/pylibcudf/tests/test_interop.py +++ b/python/pylibcudf/pylibcudf/tests/test_interop.py @@ -3,6 +3,7 @@ import pyarrow as pa import pylibcudf as plc import pytest +from utils import assert_table_eq def test_list_dtype_roundtrip(): @@ -66,3 +67,10 @@ def test_decimal_other(data_type): arrow_type = plc.interop.to_arrow(data_type, precision=precision) assert arrow_type == pa.decimal128(precision, 0) + + +def test_dlpack(): + expected = pa.table({"a": [1, 2, 3], "b": [5, 6, 7]}) + plc_table = plc.interop.from_arrow(expected) + result = plc.interop.from_dlpack(plc.interop.to_dlpack(plc_table)) + assert_table_eq(expected, result) From 139067cb960ca74d7f2169fd1ebbe1710653216e Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Tue, 22 Oct 2024 14:34:03 -0700 Subject: [PATCH 3/7] address review --- .../all_cuda-118_arch-x86_64.yaml | 1 + .../all_cuda-125_arch-x86_64.yaml | 1 + dependencies.yaml | 1 + python/cudf/cudf/_lib/interop.pyx | 12 ++---- python/pylibcudf/pylibcudf/interop.pxd | 4 +- python/pylibcudf/pylibcudf/interop.pyx | 41 +++++++++++-------- .../pylibcudf/pylibcudf/tests/test_interop.py | 25 ++++++++++- python/pylibcudf/pyproject.toml | 1 + 8 files changed, 58 insertions(+), 28 deletions(-) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index bd5e6c3d569..d8e9fa6e30c 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -23,6 +23,7 @@ dependencies: - cuda-sanitizer-api=11.8.86 - cuda-version=11.8 - cudatoolkit +- cupy - cupy>=12.0.0 - cxx-compiler - cython>=3.0.3 diff --git a/conda/environments/all_cuda-125_arch-x86_64.yaml b/conda/environments/all_cuda-125_arch-x86_64.yaml index 565a3ebfa3c..a9d92d7a8a8 100644 --- a/conda/environments/all_cuda-125_arch-x86_64.yaml +++ b/conda/environments/all_cuda-125_arch-x86_64.yaml @@ -24,6 +24,7 @@ dependencies: - cuda-python>=12.0,<13.0a0 - cuda-sanitizer-api - cuda-version=12.5 +- cupy - cupy>=12.0.0 - cxx-compiler - cython>=3.0.3 diff --git a/dependencies.yaml b/dependencies.yaml index ff97b67f0ce..9f48bb219df 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -818,6 +818,7 @@ dependencies: - hypothesis - *numpy - pandas + - cupy test_python_cudf: common: - output_types: [conda, requirements, pyproject] diff --git a/python/cudf/cudf/_lib/interop.pyx b/python/cudf/cudf/_lib/interop.pyx index ad5ac2e2a01..1c9d3a01b80 100644 --- a/python/cudf/cudf/_lib/interop.pyx +++ b/python/cudf/cudf/_lib/interop.pyx @@ -8,7 +8,7 @@ from cudf.core.buffer import acquire_spill_lock from cudf.core.dtypes import ListDtype, StructDtype -def from_dlpack(dlpack_capsule): +def from_dlpack(object dlpack_capsule): """ Converts a DLPack Tensor PyCapsule into a list of columns. @@ -25,15 +25,11 @@ def to_dlpack(list source_columns): DLPack Tensor PyCapsule will have the name "dltensor". """ - if any(column.null_count for column in source_columns): - raise ValueError( - "Cannot create a DLPack tensor with null values. " - "Input is required to have null count as zero." + return pylibcudf.interop.to_dlpack( + pylibcudf.Table( + [col.to_pylibcudf(mode="read") for col in source_columns] ) - plc_table = pylibcudf.Table( - [col.to_pylibcudf(mode="read") for col in source_columns] ) - return pylibcudf.interop.to_dlpack(plc_table) def gather_metadata(object cols_dtypes): diff --git a/python/pylibcudf/pylibcudf/interop.pxd b/python/pylibcudf/pylibcudf/interop.pxd index 8d2872b82f0..2a0a8c15fdd 100644 --- a/python/pylibcudf/pylibcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/interop.pxd @@ -3,6 +3,6 @@ from pylibcudf.table cimport Table -cpdef Table from_dlpack(managed_tensor) +cpdef Table from_dlpack(object managed_tensor) -cpdef to_dlpack(Table input) +cpdef object to_dlpack(Table input) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index 4cf27b5c90e..ba50010bc3a 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -2,6 +2,7 @@ from cpython.pycapsule cimport ( PyCapsule_GetPointer, + PyCapsule_IsValid, PyCapsule_New, PyCapsule_SetName, ) @@ -324,7 +325,7 @@ def _to_arrow_scalar(cudf_object, metadata=None): return to_arrow(Column.from_scalar(cudf_object, 1), metadata=metadata)[0] -cpdef Table from_dlpack(managed_tensor): +cpdef Table from_dlpack(object managed_tensor): """ Convert a DLPack DLTensor into a cudf table. @@ -340,23 +341,28 @@ cpdef Table from_dlpack(managed_tensor): Table Table with a copy of the tensor data. """ + if not PyCapsule_IsValid(managed_tensor, "dltensor"): + raise ValueError("Invalid capsule object") cdef unique_ptr[table] c_result cdef DLManagedTensor* dlpack_tensor = PyCapsule_GetPointer( managed_tensor, "dltensor" ) PyCapsule_SetName(managed_tensor, "used_dltensor") + # Note: A copy is always performed when converting the dlpack + # data to a libcudf table. We also delete the dlpack_tensor pointer + # as the poionter is not deleted by libcudf's from_dlpack function. + # TODO: https://github.com/rapidsai/cudf/issues/10874 + # TODO: https://github.com/rapidsai/cudf/issues/10849 with nogil: - c_result = move( - cpp_from_dlpack(dlpack_tensor) - ) + c_result = cpp_from_dlpack(dlpack_tensor) - result = Table.from_libcudf(move(c_result)) + cdef Table result = Table.from_libcudf(move(c_result)) dlpack_tensor.deleter(dlpack_tensor) return result -cpdef to_dlpack(Table input): +cpdef object to_dlpack(Table input): """ Convert a cudf table into a DLPack DLTensor. @@ -372,12 +378,16 @@ cpdef to_dlpack(Table input): PyCapsule 1D or 2D DLPack tensor with a copy of the table data, or nullptr. """ + for col in input._columns: + if col.null_count(): + raise ValueError( + "Cannot create a DLPack tensor with null values. " + "Input is required to have null count as zero." + ) cdef DLManagedTensor *dlpack_tensor with nogil: - dlpack_tensor = cpp_to_dlpack( - input.view() - ) + dlpack_tensor = cpp_to_dlpack(input.view()) return PyCapsule_New( dlpack_tensor, @@ -387,12 +397,9 @@ cpdef to_dlpack(Table input): cdef void dlmanaged_tensor_pycapsule_deleter(object pycap_obj) noexcept: - cdef DLManagedTensor* dlpack_tensor = 0 - try: - dlpack_tensor = PyCapsule_GetPointer( - pycap_obj, "used_dltensor" - ) + if PyCapsule_IsValid(pycap_obj, "used_dltensor"): # we do not call a used capsule's deleter - except Exception: - dlpack_tensor = PyCapsule_GetPointer(pycap_obj, "dltensor") - dlpack_tensor.deleter(dlpack_tensor) + return + cdef DLManagedTensor* dlpack_tensor + dlpack_tensor = PyCapsule_GetPointer(pycap_obj, "dltensor") + dlpack_tensor.deleter(dlpack_tensor) diff --git a/python/pylibcudf/pylibcudf/tests/test_interop.py b/python/pylibcudf/pylibcudf/tests/test_interop.py index 934d9b4ab8b..1ed744b5437 100644 --- a/python/pylibcudf/pylibcudf/tests/test_interop.py +++ b/python/pylibcudf/pylibcudf/tests/test_interop.py @@ -1,5 +1,7 @@ # Copyright (c) 2024, NVIDIA CORPORATION. +import cupy as cp +import numpy as np import pyarrow as pa import pylibcudf as plc import pytest @@ -69,8 +71,29 @@ def test_decimal_other(data_type): assert arrow_type == pa.decimal128(precision, 0) -def test_dlpack(): +def test_dlpack_plc_able(): expected = pa.table({"a": [1, 2, 3], "b": [5, 6, 7]}) plc_table = plc.interop.from_arrow(expected) result = plc.interop.from_dlpack(plc.interop.to_dlpack(plc_table)) assert_table_eq(expected, result) + + +def test_dlpack_cupy_array(): + arr = cp.arange(3) + result = plc.interop.from_dlpack(arr.toDlpack()) + expected = pa.table({"a": [0, 1, 2]}) + assert_table_eq(expected, result) + + +def test_dlpack_numpy_array(): + arr = np.arange(3) + result = plc.interop.from_dlpack(arr.__dlpack__()) + expected = pa.table({"a": [0, 1, 2]}) + assert_table_eq(expected, result) + + +def test_to_dlpack_error(): + expected = pa.table({"a": [1, None, 3], "b": [5, 6, 7]}) + plc_table = plc.interop.from_arrow(expected) + with pytest.raises(ValueError): + plc.interop.from_dlpack(plc.interop.to_dlpack(plc_table)) diff --git a/python/pylibcudf/pyproject.toml b/python/pylibcudf/pyproject.toml index ea5b3065896..a6d2cb43d6b 100644 --- a/python/pylibcudf/pyproject.toml +++ b/python/pylibcudf/pyproject.toml @@ -40,6 +40,7 @@ classifiers = [ [project.optional-dependencies] test = [ + "cupy", "fastavro>=0.22.9", "hypothesis", "numpy>=1.23,<3.0a0", From 5dcd95ac3fa500c21090a173bd5f76110bf4019e Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Thu, 24 Oct 2024 05:41:31 -0700 Subject: [PATCH 4/7] remove cupy --- conda/environments/all_cuda-118_arch-x86_64.yaml | 1 - conda/environments/all_cuda-125_arch-x86_64.yaml | 1 - dependencies.yaml | 1 - python/pylibcudf/pyproject.toml | 1 - 4 files changed, 4 deletions(-) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index d8e9fa6e30c..bd5e6c3d569 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -23,7 +23,6 @@ dependencies: - cuda-sanitizer-api=11.8.86 - cuda-version=11.8 - cudatoolkit -- cupy - cupy>=12.0.0 - cxx-compiler - cython>=3.0.3 diff --git a/conda/environments/all_cuda-125_arch-x86_64.yaml b/conda/environments/all_cuda-125_arch-x86_64.yaml index a9d92d7a8a8..565a3ebfa3c 100644 --- a/conda/environments/all_cuda-125_arch-x86_64.yaml +++ b/conda/environments/all_cuda-125_arch-x86_64.yaml @@ -24,7 +24,6 @@ dependencies: - cuda-python>=12.0,<13.0a0 - cuda-sanitizer-api - cuda-version=12.5 -- cupy - cupy>=12.0.0 - cxx-compiler - cython>=3.0.3 diff --git a/dependencies.yaml b/dependencies.yaml index 9f48bb219df..ff97b67f0ce 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -818,7 +818,6 @@ dependencies: - hypothesis - *numpy - pandas - - cupy test_python_cudf: common: - output_types: [conda, requirements, pyproject] diff --git a/python/pylibcudf/pyproject.toml b/python/pylibcudf/pyproject.toml index a6d2cb43d6b..ea5b3065896 100644 --- a/python/pylibcudf/pyproject.toml +++ b/python/pylibcudf/pyproject.toml @@ -40,7 +40,6 @@ classifiers = [ [project.optional-dependencies] test = [ - "cupy", "fastavro>=0.22.9", "hypothesis", "numpy>=1.23,<3.0a0", From be356395001a65e11f360ebf16c28f26c70ab914 Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Thu, 24 Oct 2024 13:51:37 -0700 Subject: [PATCH 5/7] add test for from_dlpack error --- .../pylibcudf/pylibcudf/tests/test_interop.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/python/pylibcudf/pylibcudf/tests/test_interop.py b/python/pylibcudf/pylibcudf/tests/test_interop.py index 1ed744b5437..41d1b24f740 100644 --- a/python/pylibcudf/pylibcudf/tests/test_interop.py +++ b/python/pylibcudf/pylibcudf/tests/test_interop.py @@ -71,29 +71,29 @@ def test_decimal_other(data_type): assert arrow_type == pa.decimal128(precision, 0) -def test_dlpack_plc_able(): +def test_round_trip_dlpack_plc_table(): expected = pa.table({"a": [1, 2, 3], "b": [5, 6, 7]}) plc_table = plc.interop.from_arrow(expected) result = plc.interop.from_dlpack(plc.interop.to_dlpack(plc_table)) assert_table_eq(expected, result) -def test_dlpack_cupy_array(): - arr = cp.arange(3) - result = plc.interop.from_dlpack(arr.toDlpack()) - expected = pa.table({"a": [0, 1, 2]}) - assert_table_eq(expected, result) - - -def test_dlpack_numpy_array(): - arr = np.arange(3) +@pytest.mark.parametrize("array", [np.array, cp.array]) +def test_round_trip_dlpack_array(array): + arr = array([1, 2, 3]) result = plc.interop.from_dlpack(arr.__dlpack__()) - expected = pa.table({"a": [0, 1, 2]}) + expected = pa.table({"a": [1, 2, 3]}) assert_table_eq(expected, result) def test_to_dlpack_error(): - expected = pa.table({"a": [1, None, 3], "b": [5, 6, 7]}) - plc_table = plc.interop.from_arrow(expected) - with pytest.raises(ValueError): + plc_table = plc.interop.from_arrow( + pa.table({"a": [1, None, 3], "b": [5, 6, 7]}) + ) + with pytest.raises(ValueError, match="Cannot create a DLPack tensor"): plc.interop.from_dlpack(plc.interop.to_dlpack(plc_table)) + + +def test_from_dlpack_error(): + with pytest.raises(ValueError, match="Invalid capsule object"): + plc.interop.from_dlpack(1) From f6e8d4d4e63decccbf7b6de1b745182d29a01e93 Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Thu, 24 Oct 2024 13:54:43 -0700 Subject: [PATCH 6/7] address review --- python/pylibcudf/pylibcudf/libcudf/interop.pxd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/pylibcudf/pylibcudf/libcudf/interop.pxd b/python/pylibcudf/pylibcudf/libcudf/interop.pxd index 72b5eb7d618..b75e9ca7001 100644 --- a/python/pylibcudf/pylibcudf/libcudf/interop.pxd +++ b/python/pylibcudf/pylibcudf/libcudf/interop.pxd @@ -37,7 +37,7 @@ cdef extern from "cudf/interop.hpp" namespace "cudf" \ ) except + DLManagedTensor* to_dlpack( - table_view input + const table_view& input ) except + cdef cppclass column_metadata: From ba909d6f16293e944fa0134700ab110b94425a4f Mon Sep 17 00:00:00 2001 From: Matthew Murray Date: Fri, 25 Oct 2024 11:15:28 -0700 Subject: [PATCH 7/7] address review --- python/pylibcudf/pylibcudf/interop.pyx | 14 +++++++++----- python/pylibcudf/pylibcudf/tests/test_interop.py | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/python/pylibcudf/pylibcudf/interop.pyx b/python/pylibcudf/pylibcudf/interop.pyx index ba50010bc3a..61e812353b7 100644 --- a/python/pylibcudf/pylibcudf/interop.pyx +++ b/python/pylibcudf/pylibcudf/interop.pyx @@ -342,16 +342,18 @@ cpdef Table from_dlpack(object managed_tensor): Table with a copy of the tensor data. """ if not PyCapsule_IsValid(managed_tensor, "dltensor"): - raise ValueError("Invalid capsule object") + raise ValueError("Invalid PyCapsule object") cdef unique_ptr[table] c_result cdef DLManagedTensor* dlpack_tensor = PyCapsule_GetPointer( managed_tensor, "dltensor" ) + if dlpack_tensor is NULL: + raise ValueError("PyCapsule object contained a NULL pointer") PyCapsule_SetName(managed_tensor, "used_dltensor") # Note: A copy is always performed when converting the dlpack # data to a libcudf table. We also delete the dlpack_tensor pointer - # as the poionter is not deleted by libcudf's from_dlpack function. + # as the pointer is not deleted by libcudf's from_dlpack function. # TODO: https://github.com/rapidsai/cudf/issues/10874 # TODO: https://github.com/rapidsai/cudf/issues/10849 with nogil: @@ -400,6 +402,8 @@ cdef void dlmanaged_tensor_pycapsule_deleter(object pycap_obj) noexcept: if PyCapsule_IsValid(pycap_obj, "used_dltensor"): # we do not call a used capsule's deleter return - cdef DLManagedTensor* dlpack_tensor - dlpack_tensor = PyCapsule_GetPointer(pycap_obj, "dltensor") - dlpack_tensor.deleter(dlpack_tensor) + cdef DLManagedTensor* dlpack_tensor = PyCapsule_GetPointer( + pycap_obj, "dltensor" + ) + if dlpack_tensor is not NULL: + dlpack_tensor.deleter(dlpack_tensor) diff --git a/python/pylibcudf/pylibcudf/tests/test_interop.py b/python/pylibcudf/pylibcudf/tests/test_interop.py index 41d1b24f740..654f0ee4d32 100644 --- a/python/pylibcudf/pylibcudf/tests/test_interop.py +++ b/python/pylibcudf/pylibcudf/tests/test_interop.py @@ -95,5 +95,5 @@ def test_to_dlpack_error(): def test_from_dlpack_error(): - with pytest.raises(ValueError, match="Invalid capsule object"): + with pytest.raises(ValueError, match="Invalid PyCapsule object"): plc.interop.from_dlpack(1)