GH-40384: [Python] Expand the C Device Interface bindings to support import on CUDA device #40385

jorisvandenbossche · 2024-03-06T15:08:37Z

Rationale for this change

Follow-up on #39979 which added _export_to_c_device/_import_from_c_device methods, but for now only for CPU devices.

What changes are included in this PR?

Ensure pyarrow.cuda is imported before importing data through the C Interface, to ensure the CUDA device is registered
Add tests for exporting/importing with the device interface on CUDA

Are these changes tested?

Yes, added tests for CUDA.

GitHub Issue: [Python] Expand the the C Device Interface bindings to support CUDA device #40384

github-actions · 2024-03-06T15:09:21Z

⚠️ GitHub issue #40384 has been automatically assigned in GitHub to PR creator.

…barrow_cuda for now)

python/pyarrow/includes/libarrow_memory.pxd

jorisvandenbossche · 2024-03-19T13:33:05Z

@github-actions crossbow submit test-cuda-python

github-actions · 2024-03-19T13:35:27Z

Revision: 05dd7a5

Submitted crossbow builds: ursacomputing/crossbow @ actions-b585ced356

Task	Status
test-cuda-python

jorisvandenbossche · 2024-03-19T15:25:58Z

@github-actions crossbow submit test-cuda-python

github-actions · 2024-03-19T15:28:13Z

Revision: 83b0d58

Submitted crossbow builds: ursacomputing/crossbow @ actions-17a7524191

Task	Status
test-cuda-python

pitrou · 2024-03-19T16:26:16Z

python/pyarrow/_cuda.pyx

@@ -965,6 +965,56 @@ def read_record_batch(object buffer, object schema, *,
    return pyarrow_wrap_batch(batch)


+def _import_device_array_cuda(in_ptr, type):


Didn't we decide at some point that we would have a registration system on the C++ side, so that per-device type memory mappers could be added at library initialization time?

Ah, good point, I forgot about that given this conditional import works in python .. Will look into the registry!

Registry was added in #40699

github-actions · 2024-03-28T07:42:16Z

Revision: 73bfca1

Submitted crossbow builds: ursacomputing/crossbow @ actions-f86b23b51b

Task	Status
test-cuda-python

jorisvandenbossche · 2024-04-08T08:58:22Z

@github-actions crossbow submit test-cuda-python

github-actions · 2024-04-08T09:00:39Z

Revision: 78a7fb5

Submitted crossbow builds: ursacomputing/crossbow @ actions-bba6bd0230

Task	Status
test-cuda-python

pitrou · 2024-04-08T15:32:43Z

python/pyarrow/lib.pyx

+        try:
+            import pyarrow.cuda  # no-cython-lint
+        except ImportError:
+            pass


Do we really want to silence the error? The error message could be more informative than the error the user would later get when a device array fails importing...

Good point. Pushed a change to capture the error message, and to actually raise a more informative error message here about pyarrow not being built with CUDA support (embedding the original import error message), instead of using the message from the C++ registry.

pitrou · 2024-04-08T15:35:12Z

python/pyarrow/array.pxi

            void* c_type_ptr
            shared_ptr[CArray] c_array

+        if c_device_array.device_type == 2:


We could perhaps instead add kDLCUDA to

arrow/python/pyarrow/includes/libarrow.pxd

Lines 1322 to 1323 in f8784ac

ctypedef enum DLDeviceType:

kDLCPU = 1

That's for DLPack, and also those definitions are the same, I would prefer to not mix them in our code. But added a similar line to libarrow.pxd to expose ARROW_DEVICE_CUDA, which also makes the above easier to read

pitrou · 2024-04-08T15:36:17Z

Are the AppVeyor failures unrelated?

jorisvandenbossche · 2024-04-09T07:34:26Z

@github-actions crossbow submit test-cuda-python

github-actions · 2024-04-09T07:36:44Z

Revision: 3e9f913

Submitted crossbow builds: ursacomputing/crossbow @ actions-80c1cb7576

Task	Status
test-cuda-python

jorisvandenbossche · 2024-06-18T16:11:37Z

@github-actions crossbow submit test-cuda-python

github-actions · 2024-06-18T16:14:18Z

Revision: eca6869

Submitted crossbow builds: ursacomputing/crossbow @ actions-246d24263e

Task	Status
test-cuda-python

conbench-apache-arrow · 2024-06-21T14:10:19Z

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 89d6354.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

…yArrow (#40717) ### Rationale for this change PyArrow implementation for the specification additions being proposed in #40708 ### What changes are included in this PR? New `__arrow_c_device_array__` method to `pyarrow.Array` and `pyarrow.RecordBatch`, and support in the `pyarrow.array(..)`, `pyarrow.record_batch(..)` and `pyarrow.table(..)` functions to consume objects that have those methods. ### Are these changes tested? Yes (for CPU only for now, #40385 is a prerequisite to test this for CUDA) * GitHub Issue: #38325

…a in PyArrow (apache#40717) ### Rationale for this change PyArrow implementation for the specification additions being proposed in apache#40708 ### What changes are included in this PR? New `__arrow_c_device_array__` method to `pyarrow.Array` and `pyarrow.RecordBatch`, and support in the `pyarrow.array(..)`, `pyarrow.record_batch(..)` and `pyarrow.table(..)` functions to consume objects that have those methods. ### Are these changes tested? Yes (for CPU only for now, apache#40385 is a prerequisite to test this for CUDA) * GitHub Issue: apache#38325

github-actions bot added Component: Python awaiting committer review Awaiting committer review labels Mar 6, 2024

jorisvandenbossche added 3 commits March 19, 2024 14:00

support + test cuda device interface (with hardcoded dependency on li…

ef8be15

…barrow_cuda for now)

test on cpu

143f511

try with #define

42e883c

jorisvandenbossche force-pushed the device-interface-cuda branch from e91517d to 42e883c Compare March 19, 2024 13:08

jorisvandenbossche commented Mar 19, 2024

View reviewed changes

python/pyarrow/includes/libarrow_memory.pxd Outdated Show resolved Hide resolved

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Mar 19, 2024

try with helper function duplicated in _cuda.pyx

05dd7a5

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 19, 2024

fixup import from pyarrow._cuda

83b0d58

fix linting issues in test_cuda.py

fd08ac1

jorisvandenbossche marked this pull request as ready for review March 19, 2024 15:32

jorisvandenbossche changed the title ~~GH-40384: [Python] Expand the the C Device Interface bindings to support CUDA device~~ GH-40384: [Python] Expand the the C Device Interface bindings to support import on CUDA device Mar 19, 2024

jorisvandenbossche requested a review from pitrou March 19, 2024 15:34

pitrou reviewed Mar 19, 2024

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Mar 20, 2024

jorisvandenbossche mentioned this pull request Mar 21, 2024

GH-38325: [Python] Implement PyCapsule interface for Device data in PyArrow #40717

Merged

jorisvandenbossche added 2 commits March 27, 2024 12:46

Merge remote-tracking branch 'upstream/main' into device-interface-cuda

3667071

undo non-test changes + try import pyarrow.cuda instead to register

1bfc05c

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 27, 2024

jorisvandenbossche changed the title ~~GH-40384: [Python] Expand the the C Device Interface bindings to support import on CUDA device~~ GH-40384: [Python] Expand the C Device Interface bindings to support import on CUDA device Apr 1, 2024

Merge remote-tracking branch 'upstream/main' into device-interface-cuda

78a7fb5

github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Apr 8, 2024

pitrou reviewed Apr 8, 2024

View reviewed changes

jorisvandenbossche added 2 commits April 9, 2024 08:46

Merge remote-tracking branch 'upstream/main' into device-interface-cuda

820bb47

address feedback: raise informative error message

3e9f913

github-actions bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Apr 9, 2024

pitrou approved these changes Apr 9, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/main' into device-interface-cuda

eca6869

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 18, 2024

jorisvandenbossche merged commit 89d6354 into apache:main Jun 19, 2024
11 of 12 checks passed

jorisvandenbossche removed the awaiting change review Awaiting change review label Jun 19, 2024

jorisvandenbossche mentioned this pull request Jun 19, 2024

[Python] Expand the the C Device Interface bindings to support CUDA device #40384

Closed

jorisvandenbossche deleted the device-interface-cuda branch June 19, 2024 09:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-40384: [Python] Expand the C Device Interface bindings to support import on CUDA device #40385

GH-40384: [Python] Expand the C Device Interface bindings to support import on CUDA device #40385

jorisvandenbossche commented Mar 6, 2024 •

edited

Loading

github-actions bot commented Mar 6, 2024

jorisvandenbossche commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

jorisvandenbossche commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

pitrou Mar 19, 2024

jorisvandenbossche Mar 20, 2024

jorisvandenbossche Mar 27, 2024

github-actions bot commented Mar 28, 2024

jorisvandenbossche commented Apr 8, 2024

github-actions bot commented Apr 8, 2024

pitrou Apr 8, 2024

jorisvandenbossche Apr 9, 2024

pitrou Apr 8, 2024

jorisvandenbossche Apr 9, 2024

pitrou commented Apr 8, 2024

jorisvandenbossche commented Apr 9, 2024

github-actions bot commented Apr 9, 2024

jorisvandenbossche commented Jun 18, 2024

github-actions bot commented Jun 18, 2024

conbench-apache-arrow bot commented Jun 21, 2024

		@@ -965,6 +965,56 @@ def read_record_batch(object buffer, object schema, *,
		return pyarrow_wrap_batch(batch)


		def _import_device_array_cuda(in_ptr, type):

GH-40384: [Python] Expand the C Device Interface bindings to support import on CUDA device #40385

GH-40384: [Python] Expand the C Device Interface bindings to support import on CUDA device #40385

Conversation

jorisvandenbossche commented Mar 6, 2024 • edited Loading

Rationale for this change

What changes are included in this PR?

Are these changes tested?

github-actions bot commented Mar 6, 2024

jorisvandenbossche commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

jorisvandenbossche commented Mar 19, 2024

github-actions bot commented Mar 19, 2024

pitrou Mar 19, 2024

Choose a reason for hiding this comment

jorisvandenbossche Mar 20, 2024

Choose a reason for hiding this comment

jorisvandenbossche Mar 27, 2024

Choose a reason for hiding this comment

github-actions bot commented Mar 28, 2024

jorisvandenbossche commented Apr 8, 2024

github-actions bot commented Apr 8, 2024

pitrou Apr 8, 2024

Choose a reason for hiding this comment

jorisvandenbossche Apr 9, 2024

Choose a reason for hiding this comment

pitrou Apr 8, 2024

Choose a reason for hiding this comment

jorisvandenbossche Apr 9, 2024

Choose a reason for hiding this comment

pitrou commented Apr 8, 2024

jorisvandenbossche commented Apr 9, 2024

github-actions bot commented Apr 9, 2024

jorisvandenbossche commented Jun 18, 2024

github-actions bot commented Jun 18, 2024

conbench-apache-arrow bot commented Jun 21, 2024

jorisvandenbossche commented Mar 6, 2024 •

edited

Loading