-
Notifications
You must be signed in to change notification settings - Fork 50
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
docs: clarify that sqrt
must be correctly rounded in accordance with IEEE 754
#882
base: main
Are you sure you want to change the base?
Conversation
Given #826 (comment) says that the default for single-precision |
The default for single-precision divide in CUDA isn't to round with this precision either. |
@@ -23,11 +23,18 @@ including the corresponding element-wise array APIs defined in this standard | |||
- multiply | |||
- divide | |||
|
|||
for floating-point operands must return the nearest representable value according to IEEE 754-2019 and a supported rounding mode. By default, the rounding mode should be ``roundTiesToEven`` (i.e., round to nearest with ties rounded toward the nearest value with an even least significant bit). | |||
for real-valued floating-point operands must return the nearest representable value according to IEEE 754-2019 and a supported rounding mode. By default, the rounding mode should be ``roundTiesToEven`` (i.e., round to nearest with ties rounded toward the nearest value with an even least significant bit). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest changing nearest representable
to correctly rounded
.
If the rounding mode is not roundTiesToEven
the value will be correctly rounded, but in general, it will not be the nearest representable. For example, if the rounding mode is roundTowardPositive
, then the correctly rounded value is rounded up regardless of whether the rounded down value is nearer to the infinitely precise mathematical result.
- reciprocal | ||
- sqrt | ||
|
||
for real-valued floating-point operands must return the nearest representable value according to IEEE 754-2019 and a supported rounding mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest changing nearest representable
to correctly rounded
.
I want to comment that
|
The default is to do correct rounding, as @hpkfft noted. However, it involves to not set array-api/src/array_api_stubs/_draft/elementwise_functions.py Lines 1478 to 1481 in 532db5b
Therefore, I do not think the "correctly rounded" requirement is achievable. Certainly it is not CuPy's default. Ref: https://docs.nvidia.com/cuda/floating-point/index.html#compiler-flags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(see above)
Denormal result support (as opposed to flush to zero ( |
Oh, I think the note you referenced is not in the spec itself? But, I would suggest that your observation ought to be considered a separate bug/PR and not block this PR. |
This PR:
sqrt
should follow IEEE 754 and always return correctly rounded result. This was implied (i.e., conforming implementations should be IEEE 754 compliant), but never made explicit.