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

Add support for copysign to the specification #693

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Sep 21, 2023

This PR

  • resolves Add copysign function to the standard #593 by adding support for composing a floating-point value having the magnitude of x1_i and the sign of x2_i for each element in x1_i.
  • chooses to follow strict IEEE 754 semantics, in which the sign bit of NaN may be manipulated.
  • limits to only real-valued floating-point data types.
  • requires that input arguments participate in type promotion.

Questions

  • Are we comfortable limiting to only floating-point data types? Libraries would be free, of course, to support other dtypes (e.g., NumPy), but this would not be considered portable according to the spec.
  • Are we comfortable with embracing full IEEE 754 NaN semantics, namely signed NaNs?

@kgryte kgryte added the API extension Adds new functions or objects to the API. label Sep 21, 2023
@kgryte kgryte added this to the v2023 milestone Sep 21, 2023
@rgommers
Copy link
Member

Are we comfortable limiting to only floating-point data types?

Yes, because real-valued floating-point dtypes is all that's in C99.

Are we comfortable with embracing full IEEE 754 NaN semantics, namely signed NaNs?

If it's supported by all libraries, then it should be fine I think. I quickly checked NumPy, PyTorch and JAX, all three seem to support it.

@seberg
Copy link
Contributor

seberg commented Sep 26, 2023

I see nothing wrong with "supporting" signed NaNs. In practice, I doubt libraries take care with having meaningful signs. Python float("nan") is always positive, np.nan I suspect is, but I am not 100% right now (I would be happy to ensure it though).

The C NAN macro does not have a defined sign though. If you really want to worry more about signs, you could specify the sign of namespace.nan (I would say its low prio until someone shouts).

EDIt: To be clear, the C macro is not just theoretical, a negative sign is actually typical

EDIT2: But if anyone has a problem with supporting it on some hardware, then I guess can just add a note for that.

@kgryte
Copy link
Contributor Author

kgryte commented Oct 18, 2023

@seberg To clarify, while np.nan is a double whose signbit is 0, NumPy does support signed NaNs:

In [1]: np.signbit(-np.nan)
Out[1]: True

In [2]: np.signbit(np.nan)
Out[2]: False

In [3]: np.signbit(np.copysign(np.nan, -1.0))
Out[3]: True

In [4]: np.signbit(np.copysign(np.nan, 1.0))
Out[4]: False

In [5]: np.signbit(np.copysign(-np.nan, 1.0))
Out[5]: False

In [6]: np.signbit(np.copysign(np.nan, -np.nan))
Out[6]: True

In [7]: np.signbit(np.copysign(np.nan, np.nan))
Out[7]: False

@kgryte
Copy link
Contributor Author

kgryte commented Oct 18, 2023

As this PR has been reviewed and discussed without objections, will go ahead and merge. Any subsequent revisions can be addressed in follow-up PRs...

@kgryte kgryte merged commit 03886d6 into data-apis:main Oct 18, 2023
1 check passed
@kgryte kgryte deleted the copysign branch October 18, 2023 23:44
@asmeurer
Copy link
Member

Python float("nan") is always positive,

I'm curious, do you know if this is guaranteed by the language, or does it just happen to be true for all implementations?

@seberg
Copy link
Contributor

seberg commented Apr 20, 2024

CPython ensures this and I am pretty sure even tests for it (I has once looked at simplifying the code responsible). I.e., IIRC, we can rely on Mark Dickinson to ensure this.
I suppose other Python implementations may not, but I would guess that if you point out CPython does, you can ask them to change it (if it matters).

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 this pull request may close these issues.

Add copysign function to the standard
4 participants