Skip to content

Conversation

@tqchen
Copy link
Member

@tqchen tqchen commented Dec 5, 2025

This PR updates the exchange API to be capsule following the new convention. Note that the exchange API python recommendation is not yet updated on the array-api side and so far there hasn't yet been downstream implementation, so this change should be safe in generally.

@tqchen
Copy link
Member Author

tqchen commented Dec 5, 2025

cc @Kathryn-cat

@tqchen
Copy link
Member Author

tqchen commented Dec 5, 2025

RFC thread #179

This PR updates the exchange API to be capsule following the new convention.
Note that the exchange API python recommendation is not yet updated on the array-api
side and so far there hasn't yet been downstream implementation, so this change
should be safe in generally.
@tqchen
Copy link
Member Author

tqchen commented Dec 12, 2025

cc @leofang @rgommers also updated python_spec.rst here

@tqchen
Copy link
Member Author

tqchen commented Dec 12, 2025

Array api spec PR data-apis/array-api#984

@leofang leofang self-requested a review December 23, 2025 06:28
@leofang
Copy link
Collaborator

leofang commented Dec 23, 2025

Sorry for delay, will try to finish my review in a day or two!

Copy link
Collaborator

@leofang leofang left a comment

Choose a reason for hiding this comment

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

Thanks, @tqchen! LGTM overall, left some questions that we should address before merging.


.. code-block:: C
PyObject *api_obj = type(tensor_obj).__dlpack_c_exchange_api__; // as C code.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am confused. Is this valid C code? Shouldn't we use PyObject_HasAttrString or alike to query if the Python attribute exists with this type?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not valid C code, but as a pesudo cython like code to query the types, happy to change to C code here

A reference implementations of the C Exchange API in frameworks:


* PyTorch: `C++ <https://github.com/pytorch/pytorch/blob/main/torch/csrc/Module.cpp#L692>`__
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's link to a particular commit (copy permalink) in case the upstream drifts away.



* PyTorch: `C++ <https://github.com/pytorch/pytorch/blob/main/torch/csrc/Module.cpp#L692>`__
* Paddle: `C++ <https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/pybind/pybind.cc#L856>`__
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto

direct interaction between the array framework PyObject* and DLPack,
as a result it is harder to implement the C Exchange API through ctypes (because
ctypes releases GIL by default and sometimes in non-free-threading environment,
GIL is needed to interact with the Python C API).
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dalcinl or @seberg to comment. I am aware that this is to capture the meeting discussion on why ctypes does not work for our purposes, but I am a bit confused by the mention of GIL.

*
* PyObject *api_obj = type(tensor_obj).__c_dlpack_exchange_api__; // as C-code
* MyDLPackExchangeAPI *api = PyLong_AsVoidPtr(api_obj);
* PyObject *api_obj = type(tensor_obj).__dlpack_c_exchange_api__; // as C-code
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto, now that I take a closer look, this seems odd to me

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.

4 participants