Handle byte order markers at the beginning of format strings #179
Conversation
|
@anjakefala Please take a look. |
dalcinl
force-pushed
the
feature/byteorder
branch
from
8c84b2f
to
af1693d
Compare
|
Note: I think this patch fixes the buffer protocol and the CAI protocol, but not the DLPack protocol because apparently DLPack forgot about endianness. Raised an issue to track it (dmlc/dlpack#97). |
Sorry, I was wrong. It seems strictly speaking CAI could also have an issue, since we currently ignore the endianness there ( mpi4py/src/mpi4py/MPI/ascaibuf.pxi Lines 92 to 93 in 84925d9 Frankly this is an oversight not just on us but also on those who contributed to CAI (well, me included...) -- CUDA is little endian so it does not make sense to accept/exchange big endianness. What should we do here? No one complained yet so just ignore it (since in practice it doesn't matter)? |
|
@leofang It would be rather trivial to add a check for native byte order to CAI, although I'm not sure whether it is worth it, DLPack takes precedence over CAI. So far I has been considering CAI as deprecated/legacy stuff. However, errors should never pass silently. |
dalcinl
force-pushed
the
feature/byteorder
branch
from
af1693d
to
13abee8
Compare
|
@leofang I implemented a few minor refactors. Afterwards, I added proper checks in CAI. Please take a new look. |
dalcinl
changed the title
|
@dalcinl This is great! Thank you so much! Once there is a tar, I happily offer to test it with my data. |
| f"__cuda_array_interface__: " | ||
| f"typestr {typestr!r} " | ||
| f"with non-native byte order") | ||
| elif byteorder == c'>': # little-endian |
That's what I thought too, until after seeing some Python only libraries that can't use dlpack. Numba is one representative example 😄 |
So DLPack cannot deal with complex quadruple precision numbers, no support for readonly, and no support for endianness. On top of that the standardized Python protocol cannot be implemented with Python-only code. Looks like issues are piling up... |
I do not have an ETA for a new release tarball. However, installing mpi4py from git is trivial, just do: python -m pip install https://github.com/mpi4py/mpi4py/tarball/masterI'm going to merge this PR right now. The tests I've added are all passing in our CI. I have not tested on an actual big endian machine yet, but if we ever do, the added tests will catch any issues. |
Complex numbers can be handled, but yeah the rest is being discussed. |
Sorry, I made I mistake in my comment, I edited it. DLPack cannot handle quadruple precision complex numbers. |
Fixes #177.