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

[BUG] rounding float/double in some cases for large scales produces different results. #9350

Open
revans2 opened this issue Sep 29, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@revans2
Copy link
Collaborator

revans2 commented Sep 29, 2023

Describe the bug
In our compatibility docs we explicitly say that round and bround can be off in many cases. But I think we can detect and fix these cases for the most part, or at least in a lot of cases.

https://github.com/NVIDIA/spark-rapids/blob/branch-23.10/docs/compatibility.md#floating-point

Spark when they do rounding will convert the float/double to a Decimal number with unlimited precision and then round it to the given scale and convert the answer back. This is fine and all, but it is also really slow, and can need up to 128 bytes to hold the number. In almost all cases it looks like the round becomes a noop on the CPU. The output is exactly the same as the input. For us we end up doing a bunch of floating point math and get a result that is very close, but slightly off from that Spark CPU gets.

It appears to happen when the exponent of the floating point number - the scale we are rounding to is larger than what the fraction could hold. For floats that is about 24-bit (8 digits) and for doubles it is about 53-bits (16 digits)

I don't think this would fix everything in all cases, but it should get us a lot closer, and feels like something we could actually do.

Steps/Code to reproduce bug

I applied this patch and found at least one row that was off for one floating point test that was larger than what approximate float thought was good. I ma sure there are a lot more.

diff --git a/integration_tests/src/main/python/arithmetic_ops_test.py b/integration_tests/src/main/python/arithmetic_ops_test.py
index 9749208d5..5201365f3 100644
--- a/integration_tests/src/main/python/arithmetic_ops_test.py
+++ b/integration_tests/src/main/python/arithmetic_ops_test.py
@@ -626,6 +626,8 @@ _arith_data_gens_for_round = numeric_gens + _arith_decimal_gens_no_neg_scale_38_
     decimal_gen_32bit_neg_scale,
     DecimalGen(precision=15, scale=-8),
     DecimalGen(precision=30, scale=-5),
+    DecimalGen(precision=38, scale=37),
+    DecimalGen(precision=37, scale=36),
     pytest.param(_decimal_gen_36_neg5, marks=pytest.mark.skipif(
         is_spark_330_or_later(), reason='This case overflows in Spark 3.3.0+')),
     pytest.param(_decimal_gen_38_neg10, marks=pytest.mark.skipif(
@@ -635,15 +637,12 @@ _arith_data_gens_for_round = numeric_gens + _arith_decimal_gens_no_neg_scale_38_
 @incompat
 @approximate_float
 @pytest.mark.parametrize('data_gen', _arith_data_gens_for_round, ids=idfn)
-def test_decimal_bround(data_gen):
+@pytest.mark.parametrize('scale', [-1, 0, 1, 2, 10, 11, 13, 15, 16, 17], ids=idfn)
+def test_decimal_bround(data_gen, scale):
     assert_gpu_and_cpu_are_equal_collect(
-            lambda spark: unary_op_df(spark, data_gen).selectExpr(
-                'bround(a)',
-                'bround(1.234, 2)',
-                'bround(a, -1)',
-                'bround(a, 1)',
-                'bround(a, 2)',
-                'bround(a, 10)'))
+            lambda spark: unary_op_df(spark, data_gen, length=4096).selectExpr(
+                'a',
+                'bround(a, {}) as b'.format(scale)))
@revans2 revans2 added bug Something isn't working ? - Needs Triage Need team to review and classify labels Sep 29, 2023
@mattahrens mattahrens removed the ? - Needs Triage Need team to review and classify label Oct 3, 2023
@jlowe
Copy link
Member

jlowe commented Nov 29, 2023

For the single precision floating point case, rapidsai/cudf#14528 is related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants