[VL] Fix High Precision Rounding

[ArnavBalyan]

What changes were proposed in this pull request?

Gluten Result: Round(0.19324999999999998, 4) = 0.1933
Vanilla Spark Result: Round(0.19324999999999998, 4) = 0.1932

• Currently gluten round has std::nextafter to offset the small error coming from multiply result. However, for high precision numbers the nextafter leads to a bigger delta than the expected value.
• This eventually causes wrong results for high precision rounding (starts occurring beyond 15 decimals).
• Currently we use double in the best case for intermediate calculations. Now moved to long double for holding the intermediate results more accurately.
• In the future, we can use boost multi-precision to support arbitrary precision at runtime, which will more closely match the Java BigDecimal and give better performance.

Also fixes: #5366.

How was this patch tested?

• Unit Tests
• Local Spark shell

After the fix:
Gluten Result: Round(0.19324999999999998, 4) = 0.1932
Vanilla Spark Result: Round(0.19324999999999998, 4) = 0.1932

The following query was also tested for the attached issue: (This can be flaky since we still use double which will lose some precision)
select round(avg(cast(col as double)), 4) as topic_2 from VALUES (0.188), (0.194), (0.194), (0.194), (0.194), (0.194), (0.194), (0.194) AS tab(col);

[github-actions]

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

• Other pull requests

[github-actions]

Run Gluten Clickhouse CI

[zhztheplayer]

cc @PHILO-HE @rui-mo

[github-actions]

Run Gluten Clickhouse CI

[github-actions]

Run Gluten Clickhouse CI

[ArnavBalyan]

Hi @zhztheplayer,@PHILO-HE ,@rui-mo could you please help triggering the workflow. Thank you!

[rui-mo]

Thanks. Looks good to me.

[github-actions]

Run Gluten Clickhouse CI

[github-actions]

Run Gluten Clickhouse CI

[FelixYBW]

What changes were proposed in this pull request?

Gluten Result: Round(0.19324999999999998, 4) = 0.1933
Vanilla Spark Result: Round(0.19324999999999998, 4) = 0.1932

• Currently gluten round has std::nextafter to offset the small error coming from multiply result. However, for high precision numbers the nextafter leads to a bigger delta than the expected value.
• This eventually causes wrong results for high precision rounding (starts occurring beyond 15 decimals).
• Currently we use double in the best case for intermediate calculations. Now moved to long double for holding the intermediate results more accurately.
• In the future, we can use boost multi-precision to support arbitrary precision at runtime, which will more closely match the Java BigDecimal and give better performance.

Also fixes: #5366.

How was this patch tested?

• Unit Tests
• Local Spark shell

After the fix: Gluten Result: Round(0.19324999999999998, 4) = 0.1932 Vanilla Spark Result: Round(0.19324999999999998, 4) = 0.1932

The following query was also tested for the attached issue: (This can be flaky since we still use double which will lose some precision) select round(avg(cast(col as double)), 4) as topic_2 from VALUES (0.188), (0.194), (0.194), (0.194), (0.194), (0.194), (0.194), (0.194) AS tab(col);

Excellent description!

[FelixYBW]

@kecookier can you confirm if the PR can fix your issue? Looks good to me

[rui-mo]

Just wondering if there is any reason for removing below case from the test. Is it due to some limitation?

checkEvaluation(Round(0.19324999999999998, 4), 0.1932)

[ArnavBalyan]

I realized next_after can return different results on different machines or compilers. In this case, the intermediate round result is almost on the edge of being rounded to the next decimal (difference was almost negligible), so removed to prevent flakiness. I think we seem to be using double as an alternative to java bigdecimal at multiple places. We will probably eventually have to move to boost/mpfr otherwise might see difference in results compared to vanilla spark. I think it's not a big problem now, but maybe in the future. Wdyt?

[rui-mo]

Thanks for clarifying. I see your point.

I think we seem to be using double as an alternative to java bigdecimal at multiple places.

Just curious. Have you noticed other issues except for the round-related ones?

[ArnavBalyan]

Thanks, so far only seen with round. I'll continue to explore if there are any other cases like this

[rui-mo]

@kecookier If there is any other relevant issue please feel free to comment.

[kecookier]

Thanks, I'll test it.

[jiangjiangtian]

@rui-mo I found an issue, but I can't tell whether the issue is from round.

SELECT round(3/32*CAST(5.92 AS DECIMAL(20,2)),2);

gluten returns 0.56, but vanilla returns 0.55.
The SQLs below have the same results between gluten and vanilla, which are 0.5549999999999999 and 0.55:

SELECT 3/32*CAST(5.92 AS DECIMAL(20,2));
SELECT round(0.5549999999999999, 2);

[jiangjiangtian]

@rui-mo I found an issue, but I can't tell whether the issue is from round.

SELECT round(3/32*CAST(5.92 AS DECIMAL(20,2)),2);

gluten returns 0.56, but vanilla returns 0.55. The SQLs below have the same results between gluten and vanilla, which are 0.5549999999999999 and 0.55:

SELECT 3/32*CAST(5.92 AS DECIMAL(20,2));
SELECT round(0.5549999999999999, 2);

I find that the argument passed to round is 0.555. I will check it.

[ArnavBalyan]

@rui-mo I found an issue, but I can't tell whether the issue is from round.

SELECT round(3/32*CAST(5.92 AS DECIMAL(20,2)),2);

gluten returns 0.56, but vanilla returns 0.55. The SQLs below have the same results between gluten and vanilla, which are 0.5549999999999999 and 0.55:

SELECT 3/32*CAST(5.92 AS DECIMAL(20,2));
SELECT round(0.5549999999999999, 2);

I find that the argument passed to round is 0.555. I will check it.

I think there will always be an issue with decimals because of how c++ represents floating point, there is a negligible loss when representing high precision using double. Java's bigdecimal does not pose this and can scale upto whatever the memory will support with dynamic precision. As long as we continue to use double in c++ as an alternative to bigdecimal, we will see issues with arithmetic. One of the long term fixes could be using mpfr/boost throughout the code but it might require significant effort. Thanks!

[jiangjiangtian]

@rui-mo I found an issue, but I can't tell whether the issue is from round.

SELECT round(3/32*CAST(5.92 AS DECIMAL(20,2)),2);

gluten returns 0.56, but vanilla returns 0.55. The SQLs below have the same results between gluten and vanilla, which are 0.5549999999999999 and 0.55:

SELECT 3/32*CAST(5.92 AS DECIMAL(20,2));
SELECT round(0.5549999999999999, 2);

I find that the argument passed to round is 0.555. I will check it.

I think there will always be an issue with decimals because of how c++ represents floating point, there is a negligible loss when representing high precision using double. Java's bigdecimal does not pose this and can scale upto whatever the memory will support with dynamic precision. As long as we continue to use double in c++ as an alternative to bigdecimal, we will see issues with arithmetic. One of the long term fixes could be using mpfr/boost throughout the code but it might require significant effort. Thanks!

Thanks for your reply!
I have a minimal example that doesn't produce the right answer:

SELECT round(cast(0.5549999999999999 as double), 2);

gluten returns 0.56, vanilla returns 0.55.
Besides, I run the following SQL and I get 0.1933:

SELECT round(cast(0.19324999999999998 AS DOUBLE), 4);

[rui-mo]

@jiangjiangtian Would you like to open an issue in Gluten with a repro? Perhaps also add to the issue tracker #4652.

[ArnavBalyan]

@jiangjiangtian could you please also send your hardware/os specs, I'll double check the change and try to fix it, thanks for testing this!

[jiangjiangtian]

@jiangjiangtian could you please also send your hardware/os specs, I'll double check the change and try to fix it, thanks for testing this!

OS is RHEL 8.1(4.18.0-147).
CPU is Intel(R) Xeon(R) Gold 5218 CPU @ 2.30GHz.
gcc is g++ (GCC) 10.2.1 20210130 (Red Hat 10.2.1-11).
Thanks.