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

Adding format_float kernel #1572

Merged
merged 73 commits into from
Dec 18, 2023
Merged

Conversation

thirtiseven
Copy link
Collaborator

@thirtiseven thirtiseven commented Nov 17, 2023

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.12346

More contexts in NVIDIA/spark-rapids#9173

More tests in plugin PR: NVIDIA/spark-rapids#9790

thirtiseven and others added 30 commits October 13, 2023 14:04
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]>
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]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Copy link
Collaborator

@hyperbolic2346 hyperbolic2346 left a 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.

@@ -1,4 +1,5 @@
/*
* Copyright (c) 2022-2023, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove duplication.

Copy link
Collaborator Author

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.

src/main/cpp/src/cast_string.hpp Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/cpp/src/cast_string.hpp Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/java/com/nvidia/spark/rapids/jni/CastStrings.java Outdated Show resolved Hide resolved
thirtiseven and others added 2 commits December 13, 2023 16:09
Co-authored-by: Mike Wilson <[email protected]>
Co-authored-by: Nghia Truong <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
thirtiseven and others added 2 commits December 15, 2023 09:01
Signed-off-by: Haoyang Li <[email protected]>
ttnghia
ttnghia previously approved these changes Dec 15, 2023
@thirtiseven
Copy link
Collaborator Author

build

Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
@ttnghia
Copy link
Collaborator

ttnghia commented Dec 15, 2023

cudf submodule is still not up to date. You should do a "reset" on the submodule.

Signed-off-by: Haoyang Li <[email protected]>
ttnghia
ttnghia previously approved these changes Dec 15, 2023
hyperbolic2346
hyperbolic2346 previously approved these changes Dec 15, 2023
Copy link
Collaborator

@hyperbolic2346 hyperbolic2346 left a 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.

src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
src/main/cpp/src/format_float.cu Outdated Show resolved Hide resolved
@thirtiseven
Copy link
Collaborator Author

build

@thirtiseven thirtiseven merged commit 48d2736 into NVIDIA:branch-24.02 Dec 18, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants