On-the-fly endianness conversion #125
Conversation
|
I wonder if we should have a solution to fall back onto the scipy functions too, looks like that transition to Sparse had unintended consequences |
|
@raphaeldussin That is indeed a cleaner solution I think. Opened an issue : pydata/sparse#521. |
|
Does someone have access to a machine with an architecture not supported by numba? I think my solution would solve potential issues there, but I don't know for sure. |
|
let's not rush to merge this until we figure out the right course of action |
|
@raphaeldussin I suggest we merge this, as it now includes a fall-back to scipy. |
|
@huard should we figure out the dask graph/performance issue first before we had more commits? |
| @@ -121,6 +122,15 @@ def apply_weights(weights, indata, shape_in, shape_out): | |||
| Extra dimensions are the same as `indata`. | |||
| If input data is C-ordered, output will also be C-ordered. | |||
| """ | |||
| # Limitation from numba : some big-endian dtypes are not supported. | |||
| try: | |||
There was a problem hiding this comment.
how costly is this test? wouldn't it be cheaper to just test with:
if indata.dtype.byteorder == '>'There was a problem hiding this comment.
I don't think it's very costly. My intuition comes from the fact that it happens when numba doesn't support the given dtype, so no computation is done at all. But indeed, it is usually less costly to perform a single check than a try-except call, that's from pure python.
However, I was trying to be as general as I can be here. This change was triggered by numba not supporting a given byte-order, but could there be other things that numba doesn't support, especially on other machines? Evidently, I can't test those with such a computer, but I tried to be ready for other eventualities...
There was a problem hiding this comment.
fair point, I'm ok with sacrificing a bit of performance for compatibility. try/except it is then
| @@ -121,6 +122,15 @@ def apply_weights(weights, indata, shape_in, shape_out): | |||
| Extra dimensions are the same as `indata`. | |||
| If input data is C-ordered, output will also be C-ordered. | |||
| """ | |||
| # Limitation from numba : some big-endian dtypes are not supported. | |||
| try: | |||
There was a problem hiding this comment.
fair point, I'm ok with sacrificing a bit of performance for compatibility. try/except it is then
Fixes #124.
#111 introduced an unforeseen constrain : machine architectures and data dtypes are now restricted by what
numbasupports.numbais used bysparseunder the hood. I have not heard of problems with machine architectures, but issue #124 is due to data coming from a different computer that uses "big-endian" (numpy's ">f8" dtype).This introduces on-the-fly byte-swapping, to convert to little-endian. The performance cost is unmeasured, but should be quite low. I find it a bit ugly to perform that conversion in xESMF instead ofsparsedirectly, but this is the fastest solution I could think of.EDIT: Instead of converting dtypes on-the-fly, I converted the weights from it's
sparseformat to ascipymatrix, which doesn't use numba and thus resolves the problem. It also conserves the dtype, which might be wanted.I used a function from
numbadirectly, should I add it as an explicit dependency?Also, it might not be necessary to raise a warning?