Repository navigation
sign special case implementations #136
Description
According to v2022.12 (and v2023.12) of the array API standard, the special cases of sign include:
For real-valued operands... If x_i is NaN, the result is NaN.
However,
from array_api_compat import numpy as np, cupy as cp, torch
np.sign(np.asarray(np.nan)) # nan
cp.sign(cp.asarray(cp.nan)) # array( 0.000e+00)
torch.sign(torch.asarray(torch.nan)) # tensor(0.)There may be other special cases that are not yet implemented. I haven't done a complete review, but I noticed that torch gives an error when the input is complex.
torch.sign(torch.asarray(1+1j))
# RuntimeError: Unlike NumPy, torch.sign is not intended to support complex numbers. Please use torch.sgn instead.
Activity
[-]`array_api_compat` `sign` special case implementations[/-][+]`sign` special case implementations[/+]asmeurer commentedon May 10, 2024
I haven't tried to do any special-cases workarounds here yet. I guess if this is causing issues for you we can add a workaround.
asmeurer commentedon May 10, 2024
We definitely should add complex support to torch.sign. It sounds like that will just be an easy wrapper of torch.sgn.
mdhaber commentedon May 10, 2024
Yeah I actually did run into this special case while converting code from NumPy to array API. The purpose of the code is to produce a different status flag that depends on whether an element is positive, negative, zero, or NaN, so no wonder I ran into it. So it would be helpful to add the special case, but support for 2023.12 would be higher priority to me.
asmeurer commentedon May 10, 2024
We can add it. The main concern with adding special-case handling is it means adding a mask to the function, so it could be a minor hit to performance. I would at least make sure there are upstream issues about this to the libraries that don't implement it correctly.
rgommers commentedon May 10, 2024
I think that won't be a minor hit. For element-wise functions, adding
isnanchecks and masking may slow things down by 2x or more.In general, these special cases have not been well validated yet, so I'd be quite reluctant to assume they are all correctly specified or support them in the compat layer.
In this case, it may be possible to fix in NumPy. There's a bunch of special-casing to make
sign(nan)returnnan, which looks like old code and disagrees with C99'ssignbiton purpose:https://github.com/numpy/numpy/blob/2a9b9134270371b43223fc848b753fceab96b4a5/numpy/_core/src/umath/loops.c.src#L1341-L1350
https://github.com/numpy/numpy/blob/2a9b9134270371b43223fc848b753fceab96b4a5/numpy/_core/src/umath/loops.c.src#L2262-L2271
Changing it would be a minor backwards-compat break, but probably an improvement. Much larger changes were made to
np.signrecently in gh-numpy/numpy#25441, however thesign(nan) = nanwas not discussed there. It does seem likesigndeviates fromsignbitfor no good reason; it was probably an ad-hoc pre-C99 decision.asmeurer commentedon May 10, 2024
NumPy is the one that agrees with what the array API says. If we think sign(nan) should not return NaN, then the array API should change.
rgommers commentedon May 10, 2024
I'm honestly not sure. These special cases are a pain, it requires a lot of time investment to figure out what was discussed before, and why different libraries end up with different return values. I suspect the following:
signbit(+nan) = false,signbit(-nan) = truenanbecause someone way back when decided this was needed, and hence there is some custom logicsignwas added very early on (Add additional element-wise mathematical functions array-api#36) with some special cases but without a nan special rulesignbitfunction was added (Addsignbitspecification for determining whether a sign bit is set array-api#705)signandsignbit, but it's hard to piece together.seberg commentedon May 10, 2024
I think one normal rule of NaNs is that they are propagated unless there is a clear reason why not.
signis very different fromsignbitafter all as that is defined to return a bool and additionally maps to an implementation detail of IEEE float representation.Further, returning NaN preserves the full "partial order" (e.g. in C++20) of
value <=> 0(less, equal, greater, and unordered).So IMO, NumPy does the right thing unless there is a good argument why typical use-cases would expect a
0return forNaNand it sounds like the report here is a use-case where you want the full partial order to be preserved!TBH, I would lean towards torch should fix this, but if there is a good reason why they don't want to (what is it?), then it has to stay undefined which seems unfortunate for the actual use-case above.
asmeurer commentedon May 10, 2024
I found this old discussion https://mail.python.org/archives/list/[email protected]/thread/A2JFHOZZOF634CNZ7E27THQEBU4EZFTS/#F3KS7QWVPIXSYB7CSSY37OXYM4JVZTZQ
signandsignbitare completely different things for complex numbers, as I pointed out at https://mail.python.org/archives/list/[email protected]/message/VBYOVSTN2GTBPEJ3OPDS2S5DLPQJFFX3/ It's probably not a coincidence that torch also doesn't define complexsign.It does seem valuable to figure this out, since there's at least one real-world use-case. Maybe we should move this discussion to the array-api repo.
Fix sign() for torch and cupy
asmeurer commentedon May 10, 2024
I have a fix at #137, which we can at least use to check the performance implications.
asmeurer commentedon May 10, 2024
By the way, you can see the other special cases that are not being followed in the xfails files (we do not yet attempt to fix any of them, and most are on operators which can't be fixed anyways):
torch
array-api-compat/torch-xfails.txt
Lines 89 to 179 in 376038e
cupy
array-api-compat/cupy-xfails.txt
Lines 61 to 169 in 376038e
numpy
array-api-compat/numpy-xfails.txt
Lines 18 to 41 in 376038e
I will say that even though there are quite a few of these, the
signspecial case seems to stand out. Almost all the rest seem to have to do with handling -0 correctly.kgryte commentedon May 11, 2024
@rgommers The addition of the special case for
NaNhandling insigncomes from data-apis/array-api#556.The specification is correct on this, as returning
NaNfollows naturally from the definition of the signum function whereand where
NaN/NaNfollows the special cases for division, thus ensuring arithmetic consistency.This was not a pre-C99 oversight and was intentional. We shouldn't expect the
signumandsignbitfunctions to return equivalent results, and NumPy is correct here. It would perhaps have been better if NumPy had chosensignumas the name rather thansignto make this delineation in behavior more clear.seberg commentedon May 18, 2024
Since its maybe interesting. Turns out getting NaNs may make this faster on the GPU: cupy/cupy#8327
mdhaber commentedon Aug 25, 2024
Just thought I'd mention that I ran into this again in a different context.