-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-41089: [C++] Clean up remaining tasks related to half float casts #41084
GH-41089: [C++] Clean up remaining tasks related to half float casts #41084
Conversation
Could you open a new issue for this? |
Sure, filed #41089 |
|
|
||
// Slightly adapted from values present here | ||
// https://blogs.mathworks.com/cleve/2017/05/08/half-precision-16-bit-floating-point-arithmetic/ | ||
AssertFormatting(formatter, Float16::FromBits(0x3c00).bits(), "1"); |
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.
This can certainly be simplified to 0x3c00
, no?
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.
Good point, simplified.
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.
Thanks for the update @ClifHouck . This looks good to me.
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 71ec6d4. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
…casts (apache#41084) ### Rationale for this change Address remaining tasks brought up post-merge in this PR: apache#40067 ### What changes are included in this PR? Adds a couple of tests and cleans up cruft on another test. ### Are these changes tested? Yes, as these are test-only related changes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41089 Authored-by: Clif Houck <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…casts (apache#41084) ### Rationale for this change Address remaining tasks brought up post-merge in this PR: apache#40067 ### What changes are included in this PR? Adds a couple of tests and cleans up cruft on another test. ### Are these changes tested? Yes, as these are test-only related changes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41089 Authored-by: Clif Houck <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…casts (apache#41084) ### Rationale for this change Address remaining tasks brought up post-merge in this PR: apache#40067 ### What changes are included in this PR? Adds a couple of tests and cleans up cruft on another test. ### Are these changes tested? Yes, as these are test-only related changes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41089 Authored-by: Clif Houck <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…casts (apache#41084) ### Rationale for this change Address remaining tasks brought up post-merge in this PR: apache#40067 ### What changes are included in this PR? Adds a couple of tests and cleans up cruft on another test. ### Are these changes tested? Yes, as these are test-only related changes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41089 Authored-by: Clif Houck <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…casts (apache#41084) ### Rationale for this change Address remaining tasks brought up post-merge in this PR: apache#40067 ### What changes are included in this PR? Adds a couple of tests and cleans up cruft on another test. ### Are these changes tested? Yes, as these are test-only related changes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41089 Authored-by: Clif Houck <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…casts (apache#41084) ### Rationale for this change Address remaining tasks brought up post-merge in this PR: apache#40067 ### What changes are included in this PR? Adds a couple of tests and cleans up cruft on another test. ### Are these changes tested? Yes, as these are test-only related changes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41089 Authored-by: Clif Houck <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Rationale for this change
Address remaining tasks brought up post-merge in this PR: #40067
What changes are included in this PR?
Adds a couple of tests and cleans up cruft on another test.
Are these changes tested?
Yes, as these are test-only related changes.
Are there any user-facing changes?
No.