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

Support float case of format_number with format_float kernel #9790

Merged
merged 11 commits into from
Jan 9, 2024

Conversation

thirtiseven
Copy link
Collaborator

@thirtiseven thirtiseven commented Nov 20, 2023

Closes #9173

This PR adds better support for the float case of format_number, which made it good enough to be enabled by default.

It will have known compatibility issues from ryu, as the same way as float to string.

Depends on: NVIDIA/spark-rapids-jni#1572

performance test results

10000000 random number generated by BigDataGen:

val dataTable = DBGen().addTable("data", "a float, b double", 10000000)
dataTable.toDF(spark).write.mode("overwrite").parquet("format_float_data")

test code:

spark.time(df.selectExpr("COUNT(format_number(a, -1)) as a", "COUNT(format_number(a, 0)) as b", "COUNT(format_number(a, 5)) as c", "COUNT(format_number(a, 50)) as d").show())
Data Type GPU Time (ms) CPU Time (ms) Speed up
double 324 12,160 37.53x
float 222 6,101 27.48x

I plan to move special case checking to the kernel next, but personally I think it is a nit for this pr. done and it runs much faster than before the change.

results before move nan/inf replacement to kernel
Data Type GPU Time (ms) CPU Time (ms) Speed up
double 1,326.8 9,841.6 7.42x
float 991.4 4,663.2 4.70x

Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
@thirtiseven thirtiseven marked this pull request as ready for review November 21, 2023 10:46
@thirtiseven thirtiseven changed the base branch from branch-23.12 to branch-24.02 November 22, 2023 13:40
@sameerz sameerz added the feature request New feature or request label Nov 28, 2023
@thirtiseven
Copy link
Collaborator Author

@revans2 please take a look on this too if you have time, thanks!

@thirtiseven
Copy link
Collaborator Author

build

Signed-off-by: Haoyang Li <[email protected]>
This configuration is enabled by default. To disable this operation on the GPU set
[`spark.rapids.sql.castFloatToString.enabled`](additional-functionality/advanced_configs.md#sql.castFloatToString.enabled) to `false`.

The `format_number` function also use ryu as the solution when formatting floating-point data types to
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: also uses ryu

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

'format_number(a, 5)').collect(), conf = float_format_number_conf)
gpu = with_gpu_session(lambda spark: unary_op_df(spark, gen).selectExpr('*',
'format_number(a, 5)').collect(), conf = float_format_number_conf)
mismatched = sum(x[0] != x[1] for x in zip(cpu, gpu))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I preferred the version that checked that when we parsed them back to a float the numbers were within the error bounds instead of saying that we cannot be wrong more than some set percentage.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

@thirtiseven
Copy link
Collaborator Author

The test code is really not strong enough, I made a fix from the kernel side: NVIDIA/spark-rapids-jni#1676

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

build

@thirtiseven thirtiseven merged commit af91522 into NVIDIA:branch-24.02 Jan 9, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Support format_number
3 participants