From 3bd03a3a2cd4b2651e93d48a4491c6174229f3d3 Mon Sep 17 00:00:00 2001 From: arnavb Date: Sun, 4 Aug 2024 19:15:25 +0000 Subject: [PATCH 1/5] Support Round for High Precision --- cpp/velox/operators/functions/Arithmetic.h | 18 ++++++++++++------ .../GlutenMathExpressionsSuite.scala | 4 ++++ .../GlutenMathExpressionsSuite.scala | 5 +++++ .../GlutenMathExpressionsSuite.scala | 4 ++++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/cpp/velox/operators/functions/Arithmetic.h b/cpp/velox/operators/functions/Arithmetic.h index 0474e1554981..ebf1ba455b54 100644 --- a/cpp/velox/operators/functions/Arithmetic.h +++ b/cpp/velox/operators/functions/Arithmetic.h @@ -7,7 +7,6 @@ * the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 - * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -18,6 +17,7 @@ #include #include #include +#include namespace gluten { template @@ -38,12 +38,18 @@ struct RoundFunction { return number; } - double factor = std::pow(10, decimals); - static const TNum kInf = std::numeric_limits::infinity(); - if (number < 0) { - return (std::round(std::nextafter(number, -kInf) * factor * -1) / factor) * -1; + /* + * Using long double for high precision during intermediate calculations. + * TODO: Make this more efficient with Boost to support high arbitrary precision at runtime. + */ + long double num = static_cast(number); + long double factor = std::pow(10.0L, static_cast(decimals)); + static const long double kInf = std::numeric_limits::infinity(); + + if (num < 0) { + return static_cast((std::round(std::nextafter(num, -kInf) * factor * -1) / factor) * -1); } - return std::round(std::nextafter(number, kInf) * factor) / factor; + return static_cast(std::round(std::nextafter(num, kInf) * factor) / factor); } template diff --git a/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala b/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala index 54583547d057..e23d2480dbc0 100644 --- a/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala +++ b/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala @@ -121,6 +121,10 @@ class GlutenMathExpressionsSuite extends MathExpressionsSuite with GlutenTestsTr checkEvaluation(Round(-3.5, 0), -4.0) checkEvaluation(Round(-0.35, 1), -0.4) checkEvaluation(Round(-35, -1), -40) + checkEvaluation(Round(0.19324999999999998, 4), 0.1932) + checkEvaluation(Round(1.12345678901234567, 8), 1.12345679) + checkEvaluation(Round(-0.98765432109876543, 5), -0.98765) + checkEvaluation(Round(12345.67890123456789, 6), 12345.678901) checkEvaluation(BRound(2.5, 0), 2.0) checkEvaluation(BRound(3.5, 0), 4.0) checkEvaluation(BRound(-2.5, 0), -2.0) diff --git a/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala b/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala index a60f0dce644b..c05eb21c88ea 100644 --- a/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala +++ b/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala @@ -249,6 +249,11 @@ class GlutenMathExpressionsSuite extends MathExpressionsSuite with GlutenTestsTr checkEvaluation(Round(-3.5, 0), -4.0) checkEvaluation(Round(-0.35, 1), -0.4) checkEvaluation(Round(-35, -1), -40) + checkEvaluation(Round(0.19324999999999998, 4), 0.1932) + checkEvaluation(Round(1.12345678901234567, 8), 1.12345679) + checkEvaluation(Round(-0.98765432109876543, 5), -0.98765) + checkEvaluation(Round(12345.67890123456789, 6), 12345.678901) + checkEvaluation(Round(-35, -1), -40) checkEvaluation(Round(BigDecimal("45.00"), -1), BigDecimal(50)) checkEvaluation(BRound(2.5, 0), 2.0) checkEvaluation(BRound(3.5, 0), 4.0) diff --git a/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala b/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala index e220924880c7..92a96803f520 100644 --- a/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala +++ b/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala @@ -248,6 +248,10 @@ class GlutenMathExpressionsSuite extends MathExpressionsSuite with GlutenTestsTr checkEvaluation(BRound(-3.5, 0), -4.0) checkEvaluation(BRound(-0.35, 1), -0.4) checkEvaluation(BRound(-35, -1), -40) + checkEvaluation(Round(0.19324999999999998, 4), 0.1932) + checkEvaluation(Round(1.12345678901234567, 8), 1.12345679) + checkEvaluation(Round(-0.98765432109876543, 5), -0.98765) + checkEvaluation(Round(12345.67890123456789, 6), 12345.678901) checkEvaluation(BRound(BigDecimal("45.00"), -1), BigDecimal(40)) checkEvaluation(checkDataTypeAndCast(RoundFloor(Literal(2.5), Literal(0))), Decimal(2)) checkEvaluation(checkDataTypeAndCast(RoundFloor(Literal(3.5), Literal(0))), Decimal(3)) From f5ab0ed688b30a3aa8df96e40c60dbadaf271afa Mon Sep 17 00:00:00 2001 From: arnavb Date: Mon, 5 Aug 2024 03:46:41 +0000 Subject: [PATCH 2/5] lint --- cpp/velox/operators/functions/Arithmetic.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/velox/operators/functions/Arithmetic.h b/cpp/velox/operators/functions/Arithmetic.h index ebf1ba455b54..65b7888420ba 100644 --- a/cpp/velox/operators/functions/Arithmetic.h +++ b/cpp/velox/operators/functions/Arithmetic.h @@ -16,8 +16,8 @@ #include #include #include -#include #include +#include namespace gluten { template @@ -39,9 +39,9 @@ struct RoundFunction { } /* - * Using long double for high precision during intermediate calculations. - * TODO: Make this more efficient with Boost to support high arbitrary precision at runtime. - */ + * Using long double for high precision during intermediate calculations. + * TODO: Make this more efficient with Boost to support high arbitrary precision at runtime. + */ long double num = static_cast(number); long double factor = std::pow(10.0L, static_cast(decimals)); static const long double kInf = std::numeric_limits::infinity(); From a093c25af448ecde6ceaf647b70a24f67b31af4d Mon Sep 17 00:00:00 2001 From: arnavb Date: Mon, 5 Aug 2024 03:47:43 +0000 Subject: [PATCH 3/5] format --- cpp/velox/operators/functions/Arithmetic.h | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/velox/operators/functions/Arithmetic.h b/cpp/velox/operators/functions/Arithmetic.h index 65b7888420ba..f45013d99094 100644 --- a/cpp/velox/operators/functions/Arithmetic.h +++ b/cpp/velox/operators/functions/Arithmetic.h @@ -7,6 +7,7 @@ * the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. From 668f49736cff048c51c62dc541a689a976615814 Mon Sep 17 00:00:00 2001 From: arnavb Date: Tue, 6 Aug 2024 05:19:04 +0000 Subject: [PATCH 4/5] fix test --- cpp/velox/operators/functions/Arithmetic.h | 10 ++++------ .../expressions/GlutenMathExpressionsSuite.scala | 1 - .../expressions/GlutenMathExpressionsSuite.scala | 1 - .../expressions/GlutenMathExpressionsSuite.scala | 1 - 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/cpp/velox/operators/functions/Arithmetic.h b/cpp/velox/operators/functions/Arithmetic.h index f45013d99094..cacd3fb7ef31 100644 --- a/cpp/velox/operators/functions/Arithmetic.h +++ b/cpp/velox/operators/functions/Arithmetic.h @@ -43,16 +43,14 @@ struct RoundFunction { * Using long double for high precision during intermediate calculations. * TODO: Make this more efficient with Boost to support high arbitrary precision at runtime. */ - long double num = static_cast(number); long double factor = std::pow(10.0L, static_cast(decimals)); - static const long double kInf = std::numeric_limits::infinity(); + static const TNum kInf = std::numeric_limits::infinity(); - if (num < 0) { - return static_cast((std::round(std::nextafter(num, -kInf) * factor * -1) / factor) * -1); + if (number < 0) { + return static_cast((std::round(std::nextafter(number, -kInf) * factor * -1) / factor) * -1); } - return static_cast(std::round(std::nextafter(num, kInf) * factor) / factor); + return static_cast(std::round(std::nextafter(number, kInf) * factor) / factor); } - template FOLLY_ALWAYS_INLINE void call(TInput& result, const TInput& a, const int32_t b = 0) { result = round(a, b); diff --git a/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala b/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala index e23d2480dbc0..765a64f91baf 100644 --- a/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala +++ b/gluten-ut/spark32/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala @@ -121,7 +121,6 @@ class GlutenMathExpressionsSuite extends MathExpressionsSuite with GlutenTestsTr checkEvaluation(Round(-3.5, 0), -4.0) checkEvaluation(Round(-0.35, 1), -0.4) checkEvaluation(Round(-35, -1), -40) - checkEvaluation(Round(0.19324999999999998, 4), 0.1932) checkEvaluation(Round(1.12345678901234567, 8), 1.12345679) checkEvaluation(Round(-0.98765432109876543, 5), -0.98765) checkEvaluation(Round(12345.67890123456789, 6), 12345.678901) diff --git a/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala b/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala index c05eb21c88ea..122f8dc066af 100644 --- a/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala +++ b/gluten-ut/spark33/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala @@ -249,7 +249,6 @@ class GlutenMathExpressionsSuite extends MathExpressionsSuite with GlutenTestsTr checkEvaluation(Round(-3.5, 0), -4.0) checkEvaluation(Round(-0.35, 1), -0.4) checkEvaluation(Round(-35, -1), -40) - checkEvaluation(Round(0.19324999999999998, 4), 0.1932) checkEvaluation(Round(1.12345678901234567, 8), 1.12345679) checkEvaluation(Round(-0.98765432109876543, 5), -0.98765) checkEvaluation(Round(12345.67890123456789, 6), 12345.678901) diff --git a/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala b/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala index 92a96803f520..7308352e40c6 100644 --- a/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala +++ b/gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala @@ -248,7 +248,6 @@ class GlutenMathExpressionsSuite extends MathExpressionsSuite with GlutenTestsTr checkEvaluation(BRound(-3.5, 0), -4.0) checkEvaluation(BRound(-0.35, 1), -0.4) checkEvaluation(BRound(-35, -1), -40) - checkEvaluation(Round(0.19324999999999998, 4), 0.1932) checkEvaluation(Round(1.12345678901234567, 8), 1.12345679) checkEvaluation(Round(-0.98765432109876543, 5), -0.98765) checkEvaluation(Round(12345.67890123456789, 6), 12345.678901) From 9d5705e91a858d00074f768e3eede0aa3559caf0 Mon Sep 17 00:00:00 2001 From: arnavb Date: Tue, 6 Aug 2024 05:31:51 +0000 Subject: [PATCH 5/5] address comments --- cpp/velox/operators/functions/Arithmetic.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/velox/operators/functions/Arithmetic.h b/cpp/velox/operators/functions/Arithmetic.h index cacd3fb7ef31..7b4c9ae9db7c 100644 --- a/cpp/velox/operators/functions/Arithmetic.h +++ b/cpp/velox/operators/functions/Arithmetic.h @@ -39,10 +39,8 @@ struct RoundFunction { return number; } - /* - * Using long double for high precision during intermediate calculations. - * TODO: Make this more efficient with Boost to support high arbitrary precision at runtime. - */ + // Using long double for high precision during intermediate calculations. + // TODO: Make this more efficient with Boost to support high arbitrary precision at runtime. long double factor = std::pow(10.0L, static_cast(decimals)); static const TNum kInf = std::numeric_limits::infinity();