-
Notifications
You must be signed in to change notification settings - Fork 53
Do we need from_dlpack when DLPack is supported by asarray? #155
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
Comments
Hmmm the only argument I can think of is |
I think there's a couple of problems with
I think this doesn't follow logically, it's what I would call "design drift". I'd much rather remove buffer protocol support from |
Would it be better, then, to remove both DLPack and buffer protocol supports from |
Forgot to reply:
I'd say whether or not it's inferior to DLPack is arguable. DLPack is good now because we put a lot of thoughts on the stream sync that leads to the |
I'm not sure, maybe. I'm fine removing just DLPack as well, or leaving as is, or removing both. I think idiomatic code would use:
It's not great in a standard to have to deal with optional other protocols that are hard to test for. Saying the buffer protocol must/may be supported is not ideal in both cases. And
I'm not sure what this means, since the buffer protocol does not support GPUs. Do you mean |
No, I meant the buffer protocol. You can create a The only trouble with the buffer protocol that one might run into is it's hard to control who'd be the recipient. If the GPU buffer is intercepted by a CPU library, we obviously would get a segfault. But at least for mpi4py this is a desired behavior (so that we know the MPI library is not built correctly to support CUDA, or that the MPI launch configuration is set to disable CUDA). |
It'd be great to come to a conclusion here.
I dunno, I can't imagine any situation where you'd prefer a segfault over an exception with a clear message? Unless I'm misunderstanding, you could make a mistake as a user here and pass a CUDA buffer like that to a CPU-only library, and you get a segfault. So I'd say that's off-label buffer protocol usage that is used at ones' own risk but should certainly not be recommended in a standard. The buffer protocol could be made properly device-aware, but it takes ~4 years before it's landed in Python versions that can be used as the minimum supported Python version for a library. So I'm personally not that interested in investing time there. We do have to make a decision on the original "remove
|
Per the consortium meeting held on 21 October 2021, the preferred option was 2, keeping |
I guess one possible advantage of
from_dlpack()
is that it's more explicit? If that's desired, then perhaps we should add other explicit constructors likefrom_buffer()
. Otherwise, it seems redundant with the genericasarray()
, and I would lean towards removing it.The text was updated successfully, but these errors were encountered: