From ee3abf0f87e163231a5595d8cfcdb182341e935f Mon Sep 17 00:00:00 2001 From: Yuan Chao Chou Date: Tue, 14 Sep 2021 12:55:12 -0700 Subject: [PATCH] Address comments for D30917653 Summary: Title. Address comments by funrollloops Differential Revision: D30941572 fbshipit-source-id: 73a1f7722ca12e012c70af7f715633f4a337cc84 --- .../prestosql/tests/ArithmeticTest.cpp | 58 ++++++------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/velox/functions/prestosql/tests/ArithmeticTest.cpp b/velox/functions/prestosql/tests/ArithmeticTest.cpp index dc652a8d20290..387f280efcf8b 100644 --- a/velox/functions/prestosql/tests/ArithmeticTest.cpp +++ b/velox/functions/prestosql/tests/ArithmeticTest.cpp @@ -56,6 +56,11 @@ class ArithmeticTest : public functions::test::FunctionBaseTest { std::string(e.what()).find(errorMessage) != std::string::npos); } } + + const double kInf = std::numeric_limits::infinity(); + const double kNan = std::numeric_limits::quiet_NaN(); + const float kInfF = std::numeric_limits::infinity(); + const float kNanF = std::numeric_limits::quiet_NaN(); }; TEST_F(ArithmeticTest, divide) @@ -71,46 +76,31 @@ __attribute__((__no_sanitize__("float-divide-by-zero"))) assertError("c0 / c1", {10}, {0}, "division by zero"); assertError("c0 / c1", {0}, {0}, "division by zero"); - float nan = std::nanf(""); - float inf = std::numeric_limits::infinity(); assertExpression( - "c0 / c1", {10.5, 9.2, 0.0}, {2, 0, 0}, {5.25, inf, nan}); + "c0 / c1", {10.5, 9.2, 0.0}, {2, 0, 0}, {5.25, kInfF, kNanF}); assertExpression( - "c0 / c1", {10.5, 9.2, 0.0}, {2, 0, 0}, {5.25, inf, nan}); + "c0 / c1", {10.5, 9.2, 0.0}, {2, 0, 0}, {5.25, kInfF, kNanF}); } TEST_F(ArithmeticTest, modulus) { - // Doubles as input. std::vector numerDouble = {0, 6, 0, -7, -1, -9, 9, 10.1}; std::vector denomDouble = {1, 2, -1, 3, -1, -3, -3, -99.9}; - std::vector expectedDouble; - - expectedDouble.reserve(numerDouble.size()); - for (size_t i = 0; i < numerDouble.size(); i++) { - expectedDouble.emplace_back(std::fmod(numerDouble[i], denomDouble[i])); - } - - double nan = std::nan(""); - double inf = std::numeric_limits::infinity(); + std::vector expectedDouble = {0, 0, 0, -1, 0, 0, 0, 10.1}; // Check using function name and alias. assertExpression( "modulus(c0, c1)", numerDouble, denomDouble, expectedDouble); assertExpression( "modulus(c0, c1)", - {5.1, nan, 5.1, inf, 5.1}, - {0.0, 5.1, nan, 5.1, inf}, - {nan, nan, nan, nan, 5.1}); + {5.1, kNan, 5.1, kInf, 5.1}, + {0.0, 5.1, kNan, 5.1, kInf}, + {kNan, kNan, kNan, kNan, 5.1}); +} - // Integers as input. +TEST_F(ArithmeticTest, modulus_integer) { std::vector numerInt = {9, 10, 0, -9, -10, -11}; std::vector denomInt = {3, -3, 11, -1, 199999, 77}; - std::vector expectedInt; - expectedInt.reserve(numerInt.size()); - - for (size_t i = 0; i < numerInt.size(); i++) { - expectedInt.emplace_back(numerInt[i] % denomInt[i]); - } + std::vector expectedInt = {0, 1, 0, 0, -10, -11}; assertExpression( "modulus(c0, c1)", numerInt, denomInt, expectedInt); @@ -118,13 +108,10 @@ TEST_F(ArithmeticTest, modulus) { } TEST_F(ArithmeticTest, power) { - float inf = std::numeric_limits::infinity(); - - // Doubles as input. std::vector baseDouble = { - 0, 0, 0, -1, -1, -1, -9, 9.1, 10.1, 11.1, -11.1, 0, inf, inf}; + 0, 0, 0, -1, -1, -1, -9, 9.1, 10.1, 11.1, -11.1, 0, kInf, kInf}; std::vector exponentDouble = { - 0, 1, -1, 0, 1, -1, -3.3, 123456.432, -99.9, 0, 100000, inf, 0, inf}; + 0, 1, -1, 0, 1, -1, -3.3, 123456.432, -99.9, 0, 100000, kInf, 0, kInf}; std::vector expectedDouble; expectedDouble.reserve(baseDouble.size()); @@ -137,12 +124,13 @@ TEST_F(ArithmeticTest, power) { "power(c0, c1)", baseDouble, exponentDouble, expectedDouble); assertExpression( "pow(c0, c1)", baseDouble, exponentDouble, expectedDouble); +} - // Integers as input. +TEST_F(ArithmeticTest, power_integer) { std::vector baseInt = {9, 10, 11, -9, -10, -11, 0}; std::vector exponentInt = {3, -3, 0, -1, 199999, 77, 0}; std::vector expectedInt; - expectedInt.reserve(baseDouble.size()); + expectedInt.reserve(baseInt.size()); for (size_t i = 0; i < baseInt.size(); i++) { expectedInt.emplace_back(pow(baseInt[i], exponentInt[i])); @@ -155,8 +143,6 @@ TEST_F(ArithmeticTest, power) { } TEST_F(ArithmeticTest, exp) { - constexpr double kInf = std::numeric_limits::infinity(); - constexpr double kNan = std::numeric_limits::quiet_NaN(); const double kE = std::exp(1); const auto exp = [&](std::optional a) { @@ -173,8 +159,6 @@ TEST_F(ArithmeticTest, exp) { } TEST_F(ArithmeticTest, ln) { - constexpr double kInf = std::numeric_limits::infinity(); - constexpr double kNan = std::numeric_limits::quiet_NaN(); const double kE = std::exp(1); const auto ln = [&](std::optional a) { @@ -192,7 +176,6 @@ TEST_F(ArithmeticTest, ln) { TEST_F(ArithmeticTest, sqrt) { constexpr double kDoubleMax = std::numeric_limits::max(); - const double kNan = std::numeric_limits::quiet_NaN(); const auto sqrt = [&](std::optional a) { return evaluateOnce("sqrt(c0)", a); @@ -211,7 +194,6 @@ TEST_F(ArithmeticTest, sqrt) { TEST_F(ArithmeticTest, cbrt) { constexpr double kDoubleMax = std::numeric_limits::max(); - const double kNan = std::numeric_limits::quiet_NaN(); const auto cbrt = [&](std::optional a) { return evaluateOnce("cbrt(c0)", a); @@ -229,8 +211,6 @@ TEST_F(ArithmeticTest, cbrt) { } TEST_F(ArithmeticTest, widthBucket) { - constexpr double kInf = std::numeric_limits::infinity(); - constexpr double kNan = std::numeric_limits::quiet_NaN(); constexpr int64_t kMaxInt64 = std::numeric_limits::max(); const auto widthBucket = [&](std::optional operand,