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

Order of signed zeros when computing the minimum and maximum #707

Closed
kgryte opened this issue Nov 16, 2023 · 5 comments · Fixed by #751
Closed

Order of signed zeros when computing the minimum and maximum #707

kgryte opened this issue Nov 16, 2023 · 5 comments · Fixed by #751
Milestone

Comments

@kgryte
Copy link
Contributor

kgryte commented Nov 16, 2023

Currently, the specifications for computing the min and max do not state what should happen in the event of signed zeros. E.g.,

> x = xp.asarray([0.0, -0.0])
> xp.min(x)
???

NumPy returns -0.0 for the above, which is likely the desired behavior.

In [1]: np.min([0.0, -0.0])
Out[1]: -0.0

In [2]: np.min([-0.0, 0.0] )
Out[2]: -0.0

The ordering of signed zeros does not appear to have been discussed in #10 or in #17 which were opened during initial development of the Array API Standard.

This issue has relevance for #667, as, if signed zeros are ordered when computing the minimum and maximum, xp.where is not a simple replacement for element-wise minimum and maximum APIs.

@rgommers
Copy link
Member

Implementations don't sort signed zeros:

>>> import numpy as np
>>> x = np.asarray([-0.0, 0.0])
>>> x
array([-0.,  0.])
>>> np.min(x)
0.0
>>> np.signbit(x)
array([ True, False])
>>> np.signbit(np.min(x))
False
>>> np.min([-0.0, 0.0])
0.0
>>> np.min([0.0, -0.0])
-0.0

>>> import cupy as cp
>>> cp.signbit(cp.min(cp.asarray([-0.0, 0.0])))
array(False)
>>> cp.signbit(cp.min(cp.asarray([0.0, -0.0])))
array(True)

>>> import torch
>>> torch.signbit(torch.min(torch.asarray([0.0, -0.0])))
tensor(True)
>>> torch.signbit(torch.min(torch.asarray([-0.0, 0.0])))
tensor(False)

>>> np.sort([0.0, -0.0])
array([ 0., -0.])
>>> np.sort([-0.0, 0.0])
array([-0.,  0.])

So -1 for requiring that. I'm not sure it's useful to even mention that, since sorting properties are used in multiple APIs and the semantics are already given by how == behaves.

@leofang
Copy link
Contributor

leofang commented Nov 16, 2023

-1 on this too. IEEE-754 says +0 and -0 compares equal (https://en.wikipedia.org/wiki/IEEE_754#Comparison_predicates), and often when computing max/min we just call std::less or other comparison operators in the reduction algorithm, so this is likely hard to implement.

@kgryte
Copy link
Contributor Author

kgryte commented Nov 16, 2023

@rgommers Odd. I am not able to recreate your results for NumPy. For me, NumPy consistently returns -0. Looking at my NumPy version, I have v1.23.5. Did this behavior change in more recent NumPy versions?

@rgommers
Copy link
Member

Perhaps, not sure. May be version and platform-dependent, because min/max have SIMD implementations.

@kgryte
Copy link
Contributor Author

kgryte commented Nov 30, 2023

During the previous workgroup meeting (2023-11-16), this issue was discussed, and consensus was that the order of signed zeros when computing the minimum and maximum should not be specified. Hence, the return value of xp.min(xp.array([0.0, -0.0])) should be implementation-defined.

As a follow-up, we should explicitly add a note to min and max indicating that users should not expect consistent behavior across array libraries for this specific edge case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants