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

Precedence of selecting from multiple exchange protocols? #142

Closed
leofang opened this issue Mar 11, 2021 · 8 comments
Closed

Precedence of selecting from multiple exchange protocols? #142

leofang opened this issue Mar 11, 2021 · 8 comments
Labels

Comments

@leofang
Copy link
Contributor

leofang commented Mar 11, 2021

I conceive that a few libraries may end up in a situation where more than one protocols are supported, for example,

  • __array_interface__
  • __cuda_array_interface__
  • Python buffer protocol
  • DLPack

Does it matter which protocol an array implementation should try first? Is it up to the library implementors?

@rgommers
Copy link
Member

rgommers commented Mar 11, 2021

I think it's up to the library implementors, however there are a lot of subtleties so it may be useful to give some pointers.

Ideally people would use from_dlpack rather than asarray, which doesn't have this problem nor the problematic history of asarray.

In asarray, implementors should probably check __dlpack_device__ first. They may then still decide to use __cuda_array_interface__ first instead of __dlpack__ if the device is CUDA for example. It shouldn't matter, if implemented correctly.

For libraries that support multiple devices, implementing all these protocols is a little tricky. For example, you then have to implement __dir__ such that hasattr(x, 'some_protocol') says False if that protocol does not support the device that x is on. The buffer protocol is particularly problematic, since you cannot hide it but it may still fail. I suspect that's the reason that deep learning frameworks don't implement the buffer protocol.

@leofang
Copy link
Contributor Author

leofang commented Mar 11, 2021

Ideally people would use from_dlpack rather than asarray, which doesn't have this problem nor the problematic history of asarray.

Right, but AFAIK we allow asarray to look up the DLPack support, so this complexity cannot be avoided. In particular, the __dlpack__ method is going to be implemented in the "parent" objects such as numpy.ndarray and cupy.ndarray, so {numpy, cupy}.asarray may be impacted too.

The buffer protocol is particularly problematic, since you cannot hide it but it may still fail.

This is part of the reason I am asking about this. For both NumPy and mpi4py, the buffer protocol is already supported and DLPack will be too, and for mpi4py I am considering to check the protocols in the following order:

if (buffer protocol is supported):
    use buffer protocol
elif (__cuda_array_interface__ exists):
    use __cuda_array_interface__
elif (__dlpack__ exists):  # new
    use __dlpack__         # new  
else:
    raise

The pro on this order is that by the time __dlpack__ is checked, CPU buffers are excluded and we only need to deal with CUDA/ROCm (unless there are some exotic, non-NumPy objects that are allocated on CPU but do not support buffer protocol, which I am unaware of so far, but even if it exists it's probably still fine). I expect the same logic should work for cupy.asarray as well.

@oleksandr-pavlyk
Copy link
Contributor

Just a nit:

unless there are some exotic, non-NumPy objects that are allocated on CPU but do not support buffer protocol

Python arbitrary precision integer comes to mind, but I do not think it affects your issue.

@rgommers
Copy link
Member

This is part of the reason I am asking about this. For both NumPy and mpi4py, the buffer protocol is already supported and DLPack will be too, and for mpi4py I am considering to check the protocols in the following order:

That sounds reasonable. Although you may consider checking for __dlpack_device__ first, if it exists you can skip the non-device-relevant protocols.

The pro on this order is that by the time __dlpack__ is checked, CPU buffers are excluded

Not quite. Take a PyTorch tensor on CPU for example - it does not support the buffer protocol.

which I am unaware of so far, but even if it exists it's probably still fine). I expect the same logic should work for cupy.asarray as well.

I agree, it will still be fine.

@leofang
Copy link
Contributor Author

leofang commented Mar 27, 2021

@rgommers Do you know off top of your head which is tried first in NumPy, buffer protocol or __array_interface__?

@rgommers
Copy link
Member

The order is:

  1. buffer protocol
  2. __array_interface__
  3. __array__

@leofang
Copy link
Contributor Author

leofang commented Mar 29, 2021

@leofang
Copy link
Contributor Author

leofang commented Jul 26, 2021

Given this doesn't seem to be an issue anymore, I'm closing it. Just FYI: In mpi4py we try in this order: buffer protocol -> DLPack -> CAI.

@leofang leofang closed this as completed Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants