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 hypot specification for computing the square root of the sum of squares #703

Merged
merged 4 commits into from Feb 21, 2024

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Nov 6, 2023

This PR

  • resolves Add support for computing the hypotenuse via hypot #544 by adding a specification for computing the square root of the sum of squares.
  • follows IEEE 754 guidance that, if one of the operands is +-infinity, the result is infinity, regardless of whether the other operand is NaN. Intentionally, this deviates from a naive sqrt(x**2 + y**2) implementation.
  • only allows underflow when both arguments are subnormal and the correct result is also subnormal. This follows C99 and the POSIX standard and effectively prohibits a naive implementation. The entire purpose of this API is to avoid underflow/overflow during intermediate steps of computation and allowing a naive implementation defeats that purpose.
  • follows sqrt in requiring floating-point data types. The guidance uses "should", not "must", to allow implementations to support non-floating-point data types. The reason for the floating-point restriction is that the output data type must be a real-valued floating-point data type and promoting from, e.g., an integral to a floating-point data type is implementation-defined. For defined promotion semantics, a consumer should provide input arrays having floating-point data types.

@kgryte kgryte added API extension Adds new functions or objects to the API. topic: Complex Data Types Complex number data types. labels Nov 6, 2023
@kgryte kgryte added this to the v2023 milestone Nov 6, 2023
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, thanks @kgryte.

I plan to merge this soon unless there are more review comments.

@leofang
Copy link
Contributor

leofang commented Nov 16, 2023

I am a bit concerned about mentioning subnormal here. In CuPy we always set the compiler flag -ftz=true which would flush all denormals to zero. This helps significantly with floating point operations on GPU. Would this PR imply that we can no longer do this?

@leofang
Copy link
Contributor

leofang commented Feb 20, 2024

Q: I forgot what conclusion we had on this API, @kgryte do we plan to target v2023?

@kgryte
Copy link
Contributor Author

kgryte commented Feb 21, 2024

@leofang I've added a note concerning subnormal behavior and the fact that actual hardware support may vary. Does this allay your concerns?

Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@rgommers
Copy link
Member

Great, looks like everyone is happy now - let's hit the green button. Thanks @kgryte & reviewers!

@rgommers rgommers merged commit bc48a1a into data-apis:main Feb 21, 2024
3 checks passed
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. topic: Complex Data Types Complex number data types.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for computing the hypotenuse via hypot
4 participants