-
Notifications
You must be signed in to change notification settings - Fork 68
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
Adding format_float kernel #1572
Conversation
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Co-authored-by: Mike Wilson <[email protected]>
Co-authored-by: Mike Wilson <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
…k-rapids-jni into thirtiseven-float_to_string
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
…k-rapids-jni into thirtiseven-float_to_string
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
…k-rapids-jni into float_to_string
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
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.
Some const comments and some nitpicks. Looking good overall.
src/main/cpp/src/CastStringJni.cpp
Outdated
@@ -1,4 +1,5 @@ | |||
/* | |||
* Copyright (c) 2022-2023, NVIDIA CORPORATION. |
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.
Remove duplication.
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, done. I didn't find those copyright issues when upmerging.
Co-authored-by: Mike Wilson <[email protected]> Co-authored-by: Nghia Truong <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Co-authored-by: Nghia Truong <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
build |
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
cudf submodule is still not up to date. You should do a "reset" on the submodule. |
Signed-off-by: Haoyang Li <[email protected]>
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.
Some small nits. I'm ok either way on these.
Co-authored-by: Mike Wilson <[email protected]>
20415e7
build |
Depends on #1508, please review it first.
This PR adds format_float kernel to support float case of
format_number
in spark-rapids.format_number(x,d)
is used to format a number, adding a comma every 3 characters for the integer part, and keeping d digits for the decimal part, in half even rounding mode. i.e. 12345678.123456, 5 => 12,345,678.12346More contexts in NVIDIA/spark-rapids#9173
More tests in plugin PR: NVIDIA/spark-rapids#9790