-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
ENH: Extend the functionlty of C++ type np::Half
#23298
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
Conversation
e96f394
to
943b2ef
Compare
/// If there are no hardware optimization available, rounding will always | ||
/// be set to ties to even. |
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.
Doesn't this result in a double round-to-even?
Considering a reduced case of floats with 8, 4, and 2 bit of mantissa;
01011111
->01
(direct rounding from 8 to 2 bits)01011111
->0110
->10
(rounding via the intermediate 4 bits)
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.
Agreed, I reverted the change and reinstated the previous behavior, which relies on explicit conversion instead of using single-precision as an argument when hardware-native conversion is available
943b2ef
to
c872b25
Compare
c872b25
to
1ca3358
Compare
- optimize float/double conversions on x86, requires for now raising up the baseline features to `f16c` at least during the build. - optimize float/double conversions on ppc64le, requires for now raising up the baseline features to `VSX3` at least during the build. - Brings `np::Half` to npymath
1ca3358
to
ea15a57
Compare
close/reopen CI errors seems unrelated |
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.
Don't want to nitpick grammar here and it is mostly a reorganization (thanks for spotting that subtle rounding issue Eric! I am very sure we have tests to spot such an error luckily.).
Thanks Sayed, lets get this in and unblock the ufunc work!
EDIT: In case anyone wonders, the exported symbols stay identical (except that the private namespace is added), so that is fine.
|
||
/// @name Comparison with no guarantee of NaN behavior | ||
/// @{ | ||
constexpr bool Less(Half r) const |
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.
Seems fair, I wonder whether Less
and LessEqual
is a typical nomenclature for no-nan versions.
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 think its fine since comparison operators are defined to handles NaNs. using methods Less
or LessEqual
instead of comparison operators and also instead of std::less
or std::less_equal
suppose to infuriates any C++ coder for the reason.
TBD
add support for Arm native half-precision operationsrevert this support due to massive fp exceptions triggered by testing unite
test_half.py
,optimize float/double conversions on x86, which requires for now raising the baseline features to
f16c
at least during the build.optimize float/double conversions on ppc64le, which requires for now raising the baseline features to
VSX3
at least during the build.Enable
np::Half
for npymath