Skip to content

Need for signbit in the array API spec #670

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

Closed
oleksandr-pavlyk opened this issue Aug 1, 2023 · 7 comments · Fixed by #705
Closed

Need for signbit in the array API spec #670

oleksandr-pavlyk opened this issue Aug 1, 2023 · 7 comments · Fixed by #705
Labels
API extension Adds new functions or objects to the API.
Milestone

Comments

@oleksandr-pavlyk
Copy link
Contributor

The array specification for atan2 requires implementations to output -0.0 for some special values, e.g.

If x1_i is -0 and x2_i is +0, the result is -0.

which brings up a question on how implementors can distinguish between -0.0 and +0.0 in their test suite.

Equality testing spec mandates that these values be indistinguishable, and for a good reason.

The sign function also clamps these two values into +0.0.

I think this calls for the standard to have the signbit function:

  • numpy.signbit ref
  • cupy.signbit ref
  • torch.signbit ref
  • tf.experimental.numpy.signbit ref
  • dask.array.signbit ref
  • JAX does not seem to have signbit function, although it is implementable (example in one of JAX's issues)
@honno
Copy link
Member

honno commented Aug 1, 2023

Yeah it is a bit clunky right now. In array-api-tests we cast 0d array floats to builtin floats, before using math.copysign(1, your_builtin_float) == 1 to see whether its positively signed or not. Maybe it's not too big of a deal to add a function for it (or possibly modifying sign), don't have a sense for that.

@asmeurer
Copy link
Member

asmeurer commented Aug 1, 2023

There is a proposal for copysign at #593.

@rgommers rgommers added the API extension Adds new functions or objects to the API. label Sep 6, 2023
@rgommers
Copy link
Member

rgommers commented Sep 6, 2023

which brings up a question on how implementors can distinguish between -0.0 and +0.0 in their test suite.

For an implementer to test something, it isn't required to use only functions in the standard. This is checking the value of a bit, which can be done with a private function even.

Not saying that it's not a good idea to have this function - but especially given that JAX doesn't have it, it'd be nice to have a stronger reason.

I checked the SciPy code base. There's a lot of use of the C/C++ versions of signbit in compiled code, but only a couple in Python code:

scipy/special/tests/test_wrightomega.py
55:        assert_(np.signbit(res.imag) == np.bool_(False))

scipy/special/tests/test_trig.py
58:    assert np.signbit(y)
62:    assert not np.signbit(y)
66:    assert not np.signbit(y)

scipy/linalg/_matfuncs_expm.pyx.in
561:            S[:, t:] = np.signbit(Y)

Scikit-learn doesn't have any usage of signbit, and Pandas has four instances:

pandas/tests/indexes/test_numpy_compat.py
105:    "func", [np.isfinite, np.isinf, np.isnan, np.signbit], ids=lambda x: x.__name__
128:        is_complex_dtype(index) and func is np.signbit

pandas/tests/indexes/multi/test_analytics.py
253:    [np.isfinite, np.isinf, np.isnan, np.signbit],

pandas/tests/arithmetic/test_numeric.py
45:    if np.signbit(np.array(zero)).any():
48:        assert np.signbit(np.array(zero)).all()

pandas/core/ops/missing.py
111:        zneg_mask = zmask & np.signbit(y)

@jakevdp
Copy link

jakevdp commented Nov 13, 2023

JAX does have signbit: https://jax.readthedocs.io/en/latest/_autosummary/jax.numpy.signbit.html

@rgommers
Copy link
Member

Okay, that is two votes for adding signbit and all libraries have it, so there are no blockers. The signature is as simple as it gets:

signbit(x: array, /) -> array

Let's add it?

@rgommers rgommers added this to the v2023 milestone Nov 13, 2023
@kgryte
Copy link
Contributor

kgryte commented Nov 13, 2023

I can create a PR later this week.

@rgommers
Copy link
Member

PR is up for review: gh-705

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API extension Adds new functions or objects to the API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants