From c533c055ce1c89e958dd7f60692e0be81eb144e4 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 17:13:22 +0000 Subject: [PATCH 01/18] first pass hyperbolic trig functions --- cpp/src/arrow/compute/api_scalar.cc | 6 + .../compute/kernels/scalar_arithmetic.cc | 239 +++++++++++++++++- 2 files changed, 241 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/compute/api_scalar.cc b/cpp/src/arrow/compute/api_scalar.cc index f00fad29d76..4329831e8bc 100644 --- a/cpp/src/arrow/compute/api_scalar.cc +++ b/cpp/src/arrow/compute/api_scalar.cc @@ -732,8 +732,12 @@ void RegisterScalarOptions(FunctionRegistry* registry) { SCALAR_ARITHMETIC_UNARY(AbsoluteValue, "abs", "abs_checked") SCALAR_ARITHMETIC_UNARY(Acos, "acos", "acos_checked") +SCALAR_ARITHMETIC_UNARY(Acosh, "acosh", "acosh_checked") SCALAR_ARITHMETIC_UNARY(Asin, "asin", "asin_checked") +SCALAR_ARITHMETIC_UNARY(Asinh, "asinh", "asinh_checked") +SCALAR_ARITHMETIC_UNARY(Atanh, "atanh", "atanh_checked") SCALAR_ARITHMETIC_UNARY(Cos, "cos", "cos_checked") +SCALAR_ARITHMETIC_UNARY(Cosh, "cosh", "cosh_checked") SCALAR_ARITHMETIC_UNARY(Ln, "ln", "ln_checked") SCALAR_ARITHMETIC_UNARY(Log10, "log10", "log10_checked") SCALAR_ARITHMETIC_UNARY(Log1p, "log1p", "log1p_checked") @@ -741,7 +745,9 @@ SCALAR_ARITHMETIC_UNARY(Log2, "log2", "log2_checked") SCALAR_ARITHMETIC_UNARY(Sqrt, "sqrt", "sqrt_checked") SCALAR_ARITHMETIC_UNARY(Negate, "negate", "negate_checked") SCALAR_ARITHMETIC_UNARY(Sin, "sin", "sin_checked") +SCALAR_ARITHMETIC_UNARY(Sinh, "sinh", "sinh_checked") SCALAR_ARITHMETIC_UNARY(Tan, "tan", "tan_checked") +SCALAR_ARITHMETIC_UNARY(Tanh, "tanh", "tanh_checked") SCALAR_EAGER_UNARY(Atan, "atan") SCALAR_EAGER_UNARY(Exp, "exp") SCALAR_EAGER_UNARY(Expm1, "expm1") diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index f11449ad574..2ae4f3c8d8c 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -178,6 +178,26 @@ struct SinChecked { } }; +struct Sinh { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static_assert(std::is_same::value, ""); + return std::sinh(val); + } +}; + +struct SinhChecked { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { + static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE(std::isinf(val))) { + *st = Status::Invalid("domain error"); + return val; + } + return std::sinh(val); + } +}; + struct Cos { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -198,6 +218,26 @@ struct CosChecked { } }; +struct Cosh { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static_assert(std::is_same::value, ""); + return std::cosh(val); + } +}; + +struct CoshChecked { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { + static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE(std::isinf(val))) { + *st = Status::Invalid("domain error"); + return val; + } + return std::cosh(val); + } +}; + struct Tan { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -219,6 +259,27 @@ struct TanChecked { } }; +struct Tanh { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static_assert(std::is_same::value, ""); + return std::tanh(val); + } +}; + +struct TanhChecked { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { + static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE(std::isinf(val))) { + *st = Status::Invalid("domain error"); + return val; + } + // Cannot raise range errors (overflow) since PI/2 is not exactly representable + return std::tanh(val); + } +}; + struct Asin { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -242,6 +303,26 @@ struct AsinChecked { } }; +struct Asinh { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static_assert(std::is_same::value, ""); + return std::asinh(val); + } +}; + +struct AsinhChecked { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { + static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE(std::isinf(val))) { + *st = Status::Invalid("domain error"); + return val; + } + return std::asin(val); + } +}; + struct Acos { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -265,14 +346,60 @@ struct AcosChecked { } }; +struct Acosh { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE(val < 1.0)) { + return std::numeric_limits::quiet_NaN(); + } + return std::acosh(val); + } +}; + +struct AcoshChecked { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { + static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE(val < 1.0)) { + *st = Status::Invalid("domain error"); + return val; + } + return std::acosh(val); + } +}; + struct Atan { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { + return std::numeric_limits::quiet_NaN(); + } return std::atan(val); } }; +struct Atanh { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { + *st = Status::Invalid("domain error"); + return val; + } + return std::atanh(val); + } +}; + +struct AtanhChecked { + template + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static_assert(std::is_same::value, ""); + return std::atanh(val); + } +}; + struct Atan2 { template static enable_if_floating_value Call(KernelContext*, Arg0 y, Arg1 x, Status*) { @@ -1173,11 +1300,22 @@ const FunctionDoc sin_doc{"Compute the sine", "to raise an error instead, see \"sin_checked\"."), {"x"}}; + const FunctionDoc sin_checked_doc{"Compute the sine", ("Invalid input values raise an error;\n" "to return NaN instead, see \"sin\"."), {"x"}}; +const FunctionDoc sinh_doc{"Compute the hyperblic sine", + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"sinh_checked\"."), + {"x"}}; + +const FunctionDoc sinh_checked_doc{"Compute the hyperbolic sine", + ("Invalid input values raise an error;\n" + "to return NaN instead, see \"sinh\"."), + {"x"}}; + const FunctionDoc cos_doc{"Compute the cosine", ("NaN is returned for invalid input values;\n" "to raise an error instead, see \"cos_checked\"."), @@ -1188,14 +1326,34 @@ const FunctionDoc cos_checked_doc{"Compute the cosine", "to return NaN instead, see \"cos\"."), {"x"}}; +const FunctionDoc cosh_doc{"Compute the hyperbolic cosine", + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"cosh_checked\"."), + {"x"}}; + +const FunctionDoc cosh_checked_doc{"Compute the hyperbolic cosine", + ("Infinite values raise an error;\n" + "to return NaN instead, see \"cosh\"."), + {"x"}}; + const FunctionDoc tan_doc{"Compute the tangent", ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"tan_checked\"."), + "to raise an error instead, see \"tanh_checked\"."), {"x"}}; const FunctionDoc tan_checked_doc{"Compute the tangent", ("Infinite values raise an error;\n" - "to return NaN instead, see \"tan\"."), + "to return NaN instead, see \"tanh\"."), + {"x"}}; + +const FunctionDoc tanh_doc{"Compute the hyperbolic tangent", + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"tanh_checked\"."), + {"x"}}; + +const FunctionDoc tanh_checked_doc{"Compute the hyperbolic tangent", + ("Infinite values raise an error;\n" + "to return NaN instead, see \"tanh\"."), {"x"}}; const FunctionDoc asin_doc{"Compute the inverse sine", @@ -1208,6 +1366,17 @@ const FunctionDoc asin_checked_doc{"Compute the inverse sine", "to return NaN instead, see \"asin\"."), {"x"}}; + +const FunctionDoc asinh_doc{"Compute the inverse hyperbolic sine", + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"asinh_checked\"."), + {"x"}}; + +const FunctionDoc asinh_checked_doc{"Compute the inverse hyperbolic sine", + ("Invalid input values raise an error;\n" + "to return NaN instead, see \"asinh\"."), + {"x"}}; + const FunctionDoc acos_doc{"Compute the inverse cosine", ("NaN is returned for invalid input values;\n" "to raise an error instead, see \"acos_checked\"."), @@ -1218,15 +1387,35 @@ const FunctionDoc acos_checked_doc{"Compute the inverse cosine", "to return NaN instead, see \"acos\"."), {"x"}}; +const FunctionDoc acosh_doc{"Compute the inverse hyperbolic cosine", + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"acosh_checked\"."), + {"x"}}; + +const FunctionDoc acosh_checked_doc{"Compute the inverse hyperbolic cosine", + ("Invalid input values raise an error;\n" + "to return NaN instead, see \"acosh\"."), + {"x"}}; + const FunctionDoc atan_doc{"Compute the inverse tangent of x", - ("The return value is in the range [-pi/2, pi/2];\n" - "for a full return range [-pi, pi], see \"atan2\"."), + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"atanh_checked\"."), {"x"}}; const FunctionDoc atan2_doc{"Compute the inverse tangent of y/x", ("The return value is in the range [-pi, pi]."), {"y", "x"}}; +const FunctionDoc atanh_doc{"Compute the inverse hyperbolic tangent of x", + ("The return value is in the range [-pi/2, pi/2];\n" + "for a full return range [-pi, pi], see \"atan2\"."), + {"x"}}; + +const FunctionDoc atanh_checked_doc{"Compute the inverse hyperbolic cosine", + ("Invalid input values raise an error;\n" + "to return NaN instead, see \"atanh\"."), + {"x"}}; + const FunctionDoc ln_doc{ "Compute natural logarithm", ("Non-positive values return -inf or NaN. Null values return null.\n" @@ -1691,6 +1880,13 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { "sin_checked", sin_checked_doc); DCHECK_OK(registry->AddFunction(std::move(sin_checked))); + auto sinh = MakeUnaryArithmeticFunctionFloatingPoint("sinh", sinh_doc); + DCHECK_OK(registry->AddFunction(std::move(sinh))); + + auto sinh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( + "sinh_checked", sinh_checked_doc); + DCHECK_OK(registry->AddFunction(std::move(sinh_checked))); + auto cos = MakeUnaryArithmeticFunctionFloatingPoint("cos", cos_doc); DCHECK_OK(registry->AddFunction(std::move(cos))); @@ -1698,6 +1894,13 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { "cos_checked", cos_checked_doc); DCHECK_OK(registry->AddFunction(std::move(cos_checked))); + auto cosh = MakeUnaryArithmeticFunctionFloatingPoint("cosh", cosh_doc); + DCHECK_OK(registry->AddFunction(std::move(cosh))); + + auto cosh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( + "cosh_checked", cosh_checked_doc); + DCHECK_OK(registry->AddFunction(std::move(cosh_checked))); + auto tan = MakeUnaryArithmeticFunctionFloatingPoint("tan", tan_doc); DCHECK_OK(registry->AddFunction(std::move(tan))); @@ -1705,6 +1908,13 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { "tan_checked", tan_checked_doc); DCHECK_OK(registry->AddFunction(std::move(tan_checked))); + auto tanh = MakeUnaryArithmeticFunctionFloatingPoint("tanh", tanh_doc); + DCHECK_OK(registry->AddFunction(std::move(tanh))); + + auto tanh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( + "tanh_checked", tanh_checked_doc); + DCHECK_OK(registry->AddFunction(std::move(tanh_checked))); + auto asin = MakeUnaryArithmeticFunctionFloatingPoint("asin", asin_doc); DCHECK_OK(registry->AddFunction(std::move(asin))); @@ -1712,6 +1922,13 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { "asin_checked", asin_checked_doc); DCHECK_OK(registry->AddFunction(std::move(asin_checked))); + auto asinh = MakeUnaryArithmeticFunctionFloatingPoint("asinh", asinh_doc); + DCHECK_OK(registry->AddFunction(std::move(asinh))); + + auto asinh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( + "asinh_checked", asinh_checked_doc); + DCHECK_OK(registry->AddFunction(std::move(asinh_checked))); + auto acos = MakeUnaryArithmeticFunctionFloatingPoint("acos", acos_doc); DCHECK_OK(registry->AddFunction(std::move(acos))); @@ -1719,12 +1936,26 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { "acos_checked", acos_checked_doc); DCHECK_OK(registry->AddFunction(std::move(acos_checked))); + auto acosh = MakeUnaryArithmeticFunctionFloatingPoint("acosh", acosh_doc); + DCHECK_OK(registry->AddFunction(std::move(acosh))); + + auto acosh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( + "acosh_checked", acosh_checked_doc); + DCHECK_OK(registry->AddFunction(std::move(acosh_checked))); + auto atan = MakeUnaryArithmeticFunctionFloatingPoint("atan", atan_doc); DCHECK_OK(registry->AddFunction(std::move(atan))); auto atan2 = MakeArithmeticFunctionFloatingPoint("atan2", atan2_doc); DCHECK_OK(registry->AddFunction(std::move(atan2))); + auto atanh = MakeUnaryArithmeticFunctionFloatingPoint("atanh", atanh_doc); + DCHECK_OK(registry->AddFunction(std::move(atanh))); + + auto atanh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( + "atanh_checked", atanh_checked_doc); + DCHECK_OK(registry->AddFunction(std::move(atanh_checked))); + // ---------------------------------------------------------------------- // Logarithms auto ln = MakeUnaryArithmeticFunctionFloatingPoint("ln", ln_doc); From 3bf4570cbb4a74ff4b848f02a6cba7ca26d6f996 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 17:33:39 +0000 Subject: [PATCH 02/18] some simple tests --- .../compute/kernels/scalar_arithmetic.cc | 20 ++++++++++--------- .../compute/kernels/scalar_arithmetic_test.cc | 4 ++-- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 2ae4f3c8d8c..5401fdab4bc 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -275,7 +275,6 @@ struct TanhChecked { *st = Status::Invalid("domain error"); return val; } - // Cannot raise range errors (overflow) since PI/2 is not exactly representable return std::tanh(val); } }; @@ -385,8 +384,7 @@ struct Atanh { static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { static_assert(std::is_same::value, ""); if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { - *st = Status::Invalid("domain error"); - return val; + return std::numeric_limits::quiet_NaN(); } return std::atanh(val); } @@ -396,6 +394,10 @@ struct AtanhChecked { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { static_assert(std::is_same::value, ""); + if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { + *st = Status::Invalid("domain error"); + return val; + } return std::atanh(val); } }; @@ -1398,20 +1400,20 @@ const FunctionDoc acosh_checked_doc{"Compute the inverse hyperbolic cosine", {"x"}}; const FunctionDoc atan_doc{"Compute the inverse tangent of x", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"atanh_checked\"."), + ("The return value is in the range [-pi/2, pi/2];\n" + "for a full return range [-pi, pi], see \"atan2\"."), {"x"}}; const FunctionDoc atan2_doc{"Compute the inverse tangent of y/x", ("The return value is in the range [-pi, pi]."), {"y", "x"}}; -const FunctionDoc atanh_doc{"Compute the inverse hyperbolic tangent of x", - ("The return value is in the range [-pi/2, pi/2];\n" - "for a full return range [-pi, pi], see \"atan2\"."), +const FunctionDoc acosh_doc{"Compute the inverse hyperbolic tangent", + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"atanh_checked\"."), {"x"}}; -const FunctionDoc atanh_checked_doc{"Compute the inverse hyperbolic cosine", +const FunctionDoc atanh_checked_doc{"Compute the inverse hyperbolic tangent", ("Invalid input values raise an error;\n" "to return NaN instead, see \"atanh\"."), {"x"}}; diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc index 9cabebc3f4e..a1ed3b0a8bf 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc @@ -1229,8 +1229,8 @@ TEST(TestUnaryArithmetic, DispatchBest) { } TEST(TestUnaryArithmetic, Null) { - for (std::string name : {"abs", "acos", "asin", "cos", "ln", "log10", "log1p", "log2", - "negate", "sin", "tan"}) { + for (std::string name : {"abs", "acos", "acosh", "asin", "asinh", "atanh", "cos", "cosh", "ln", "log10", "log1p", "log2", + "negate", "sin", "sinh", "tan", "tanh"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; AssertNullToNull(name); From 028bead5f803821ba1b376b652534fcb03dad284 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 17:39:07 +0000 Subject: [PATCH 03/18] some simple tests --- cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc index a1ed3b0a8bf..19dd43c1f1d 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc @@ -1187,7 +1187,7 @@ TEST(TestUnaryArithmetic, DispatchBest) { // Float types (with _checked variant) for (std::string name : - {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos"}) { + {"ln", "log2", "log10", "log1p", "sin", "sinh", "cos", "cosh", "tan", "tanh", "asin", "asinh", "acos", "acosh", "atanh"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; for (const auto& ty : {float32(), float64()}) { @@ -1207,7 +1207,7 @@ TEST(TestUnaryArithmetic, DispatchBest) { // Integer -> Float64 (with _checked variant) for (std::string name : - {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos"}) { + {"ln", "log2", "log10", "log1p", "sin", "sinh", "cos", "cosh", "tan", "tanh", "asin", "asinh", "acos", "acosh", "atanh"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; for (const auto& ty : From 02183b64094721e29d15c532f06616611f9c4db1 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 18:22:29 +0000 Subject: [PATCH 04/18] fix --- cpp/src/arrow/compute/kernels/scalar_arithmetic.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 5401fdab4bc..a05214ba9fa 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -392,7 +392,7 @@ struct Atanh { struct AtanhChecked { template - static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { + static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { static_assert(std::is_same::value, ""); if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { *st = Status::Invalid("domain error"); @@ -1408,7 +1408,7 @@ const FunctionDoc atan2_doc{"Compute the inverse tangent of y/x", ("The return value is in the range [-pi, pi]."), {"y", "x"}}; -const FunctionDoc acosh_doc{"Compute the inverse hyperbolic tangent", +const FunctionDoc atanh_doc{"Compute the inverse hyperbolic tangent", ("NaN is returned for invalid input values;\n" "to raise an error instead, see \"atanh_checked\"."), {"x"}}; From 5b4d0154bd1378266789fdce280f9543cbc5a9bc Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 19:06:53 +0000 Subject: [PATCH 05/18] remove addition --- cpp/src/arrow/compute/kernels/scalar_arithmetic.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index a05214ba9fa..ca65032e193 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -372,9 +372,6 @@ struct Atan { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { - return std::numeric_limits::quiet_NaN(); - } return std::atan(val); } }; From 0b06bfc4aa1d9b04ffd358dfa13df5b3a1bf3e69 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 20:48:32 +0000 Subject: [PATCH 06/18] fixes --- cpp/src/arrow/compute/api_scalar.cc | 8 +- cpp/src/arrow/compute/api_scalar.h | 46 ++++++++++ .../compute/kernels/scalar_arithmetic.cc | 84 ++---------------- .../compute/kernels/scalar_arithmetic_test.cc | 88 +++++++++++++++++-- 4 files changed, 136 insertions(+), 90 deletions(-) diff --git a/cpp/src/arrow/compute/api_scalar.cc b/cpp/src/arrow/compute/api_scalar.cc index 4329831e8bc..b0a8c94ada1 100644 --- a/cpp/src/arrow/compute/api_scalar.cc +++ b/cpp/src/arrow/compute/api_scalar.cc @@ -734,10 +734,8 @@ SCALAR_ARITHMETIC_UNARY(AbsoluteValue, "abs", "abs_checked") SCALAR_ARITHMETIC_UNARY(Acos, "acos", "acos_checked") SCALAR_ARITHMETIC_UNARY(Acosh, "acosh", "acosh_checked") SCALAR_ARITHMETIC_UNARY(Asin, "asin", "asin_checked") -SCALAR_ARITHMETIC_UNARY(Asinh, "asinh", "asinh_checked") SCALAR_ARITHMETIC_UNARY(Atanh, "atanh", "atanh_checked") SCALAR_ARITHMETIC_UNARY(Cos, "cos", "cos_checked") -SCALAR_ARITHMETIC_UNARY(Cosh, "cosh", "cosh_checked") SCALAR_ARITHMETIC_UNARY(Ln, "ln", "ln_checked") SCALAR_ARITHMETIC_UNARY(Log10, "log10", "log10_checked") SCALAR_ARITHMETIC_UNARY(Log1p, "log1p", "log1p_checked") @@ -745,9 +743,11 @@ SCALAR_ARITHMETIC_UNARY(Log2, "log2", "log2_checked") SCALAR_ARITHMETIC_UNARY(Sqrt, "sqrt", "sqrt_checked") SCALAR_ARITHMETIC_UNARY(Negate, "negate", "negate_checked") SCALAR_ARITHMETIC_UNARY(Sin, "sin", "sin_checked") -SCALAR_ARITHMETIC_UNARY(Sinh, "sinh", "sinh_checked") SCALAR_ARITHMETIC_UNARY(Tan, "tan", "tan_checked") -SCALAR_ARITHMETIC_UNARY(Tanh, "tanh", "tanh_checked") +SCALAR_EAGER_UNARY(Asinh, "asinh") +SCALAR_EAGER_UNARY(Cosh, "cosh") +SCALAR_EAGER_UNARY(Sinh, "sinh") +SCALAR_EAGER_UNARY(Tanh, "tanh") SCALAR_EAGER_UNARY(Atan, "atan") SCALAR_EAGER_UNARY(Exp, "exp") SCALAR_EAGER_UNARY(Expm1, "expm1") diff --git a/cpp/src/arrow/compute/api_scalar.h b/cpp/src/arrow/compute/api_scalar.h index 21daf936fd2..0e5a388b107 100644 --- a/cpp/src/arrow/compute/api_scalar.h +++ b/cpp/src/arrow/compute/api_scalar.h @@ -784,6 +784,52 @@ Result Atan(const Datum& arg, ExecContext* ctx = NULLPTR); ARROW_EXPORT Result Atan2(const Datum& y, const Datum& x, ExecContext* ctx = NULLPTR); +/// \brief Compute the hyperbolic sine of the array values. +/// \param[in] arg The values to compute the hyperbolic sine for. +/// \param[in] ctx the function execution context, optional +/// \return the elementwise hyperbolic sine of the values +ARROW_EXPORT +Result Sinh(const Datum& arg, ExecContext* ctx = NULLPTR); + +/// \brief Compute the hyperbolic cosine of the array values. +/// \param[in] arg The values to compute the hyperbolic cosine for. +/// \param[in] ctx the function execution context, optional +/// \return the elementwise hyperbolic cosine of the values +ARROW_EXPORT +Result Cosh(const Datum& arg, ExecContext* ctx = NULLPTR); + +/// \brief Compute the hyperbolic tangent of the array values. +/// \param[in] arg The values to compute the hyperbolic tangent for. +/// \param[in] ctx the function execution context, optional +/// \return the elementwise hyperbolic tangent of the values +ARROW_EXPORT +Result Tanh(const Datum& arg, ExecContext* ctx = NULLPTR); + +/// \brief Compute the inverse hyperbolic sine of the array values. +/// \param[in] arg The values to compute the inverse hyperbolic sine for. +/// \param[in] ctx the function execution context, optional +/// \return the elementwise inverse hyperbolic sine of the values +ARROW_EXPORT +Result Asinh(const Datum& arg, ExecContext* ctx = NULLPTR); + +/// \brief Compute the inverse hyperbolic cosine of the array values. +/// \param[in] arg The values to compute the inverse hyperbolic cosine for. +/// \param[in] options arithmetic options (enable/disable overflow checking), optional +/// \param[in] ctx the function execution context, optional +/// \return the elementwise inverse hyperbolic cosine of the values +ARROW_EXPORT +Result Acosh(const Datum& arg, ArithmeticOptions options = ArithmeticOptions(), + ExecContext* ctx = NULLPTR); + +/// \brief Compute the inverse hyperbolic tangent of the array values. +/// \param[in] arg The values to compute the inverse hyperbolic tangent for. +/// \param[in] options arithmetic options (enable/disable overflow checking), optional +/// \param[in] ctx the function execution context, optional +/// \return the elementwise inverse hyperbolic tangent of the values +ARROW_EXPORT +Result Atanh(const Datum& arg, ArithmeticOptions options = ArithmeticOptions(), + ExecContext* ctx = NULLPTR); + /// \brief Get the natural log of a value. /// /// If argument is null the result will be null. diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index ca65032e193..88bf536f7c5 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -186,18 +186,6 @@ struct Sinh { } }; -struct SinhChecked { - template - static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { - static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE(std::isinf(val))) { - *st = Status::Invalid("domain error"); - return val; - } - return std::sinh(val); - } -}; - struct Cos { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -226,18 +214,6 @@ struct Cosh { } }; -struct CoshChecked { - template - static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { - static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE(std::isinf(val))) { - *st = Status::Invalid("domain error"); - return val; - } - return std::cosh(val); - } -}; - struct Tan { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -267,18 +243,6 @@ struct Tanh { } }; -struct TanhChecked { - template - static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { - static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE(std::isinf(val))) { - *st = Status::Invalid("domain error"); - return val; - } - return std::tanh(val); - } -}; - struct Asin { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -380,7 +344,7 @@ struct Atanh { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { + if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0))) { return std::numeric_limits::quiet_NaN(); } return std::atanh(val); @@ -391,7 +355,7 @@ struct AtanhChecked { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { + if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0))) { *st = Status::Invalid("domain error"); return val; } @@ -1306,14 +1270,9 @@ const FunctionDoc sin_checked_doc{"Compute the sine", {"x"}}; const FunctionDoc sinh_doc{"Compute the hyperblic sine", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"sinh_checked\"."), +(""), {"x"}}; -const FunctionDoc sinh_checked_doc{"Compute the hyperbolic sine", - ("Invalid input values raise an error;\n" - "to return NaN instead, see \"sinh\"."), - {"x"}}; const FunctionDoc cos_doc{"Compute the cosine", ("NaN is returned for invalid input values;\n" @@ -1326,15 +1285,9 @@ const FunctionDoc cos_checked_doc{"Compute the cosine", {"x"}}; const FunctionDoc cosh_doc{"Compute the hyperbolic cosine", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"cosh_checked\"."), +(""), {"x"}}; -const FunctionDoc cosh_checked_doc{"Compute the hyperbolic cosine", - ("Infinite values raise an error;\n" - "to return NaN instead, see \"cosh\"."), - {"x"}}; - const FunctionDoc tan_doc{"Compute the tangent", ("NaN is returned for invalid input values;\n" "to raise an error instead, see \"tanh_checked\"."), @@ -1346,15 +1299,9 @@ const FunctionDoc tan_checked_doc{"Compute the tangent", {"x"}}; const FunctionDoc tanh_doc{"Compute the hyperbolic tangent", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"tanh_checked\"."), + (""), {"x"}}; -const FunctionDoc tanh_checked_doc{"Compute the hyperbolic tangent", - ("Infinite values raise an error;\n" - "to return NaN instead, see \"tanh\"."), - {"x"}}; - const FunctionDoc asin_doc{"Compute the inverse sine", ("NaN is returned for invalid input values;\n" "to raise an error instead, see \"asin_checked\"."), @@ -1371,11 +1318,6 @@ const FunctionDoc asinh_doc{"Compute the inverse hyperbolic sine", "to raise an error instead, see \"asinh_checked\"."), {"x"}}; -const FunctionDoc asinh_checked_doc{"Compute the inverse hyperbolic sine", - ("Invalid input values raise an error;\n" - "to return NaN instead, see \"asinh\"."), - {"x"}}; - const FunctionDoc acos_doc{"Compute the inverse cosine", ("NaN is returned for invalid input values;\n" "to raise an error instead, see \"acos_checked\"."), @@ -1882,10 +1824,6 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { auto sinh = MakeUnaryArithmeticFunctionFloatingPoint("sinh", sinh_doc); DCHECK_OK(registry->AddFunction(std::move(sinh))); - auto sinh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( - "sinh_checked", sinh_checked_doc); - DCHECK_OK(registry->AddFunction(std::move(sinh_checked))); - auto cos = MakeUnaryArithmeticFunctionFloatingPoint("cos", cos_doc); DCHECK_OK(registry->AddFunction(std::move(cos))); @@ -1896,10 +1834,6 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { auto cosh = MakeUnaryArithmeticFunctionFloatingPoint("cosh", cosh_doc); DCHECK_OK(registry->AddFunction(std::move(cosh))); - auto cosh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( - "cosh_checked", cosh_checked_doc); - DCHECK_OK(registry->AddFunction(std::move(cosh_checked))); - auto tan = MakeUnaryArithmeticFunctionFloatingPoint("tan", tan_doc); DCHECK_OK(registry->AddFunction(std::move(tan))); @@ -1910,10 +1844,6 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { auto tanh = MakeUnaryArithmeticFunctionFloatingPoint("tanh", tanh_doc); DCHECK_OK(registry->AddFunction(std::move(tanh))); - auto tanh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( - "tanh_checked", tanh_checked_doc); - DCHECK_OK(registry->AddFunction(std::move(tanh_checked))); - auto asin = MakeUnaryArithmeticFunctionFloatingPoint("asin", asin_doc); DCHECK_OK(registry->AddFunction(std::move(asin))); @@ -1924,10 +1854,6 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) { auto asinh = MakeUnaryArithmeticFunctionFloatingPoint("asinh", asinh_doc); DCHECK_OK(registry->AddFunction(std::move(asinh))); - auto asinh_checked = MakeUnaryArithmeticFunctionFloatingPointNotNull( - "asinh_checked", asinh_checked_doc); - DCHECK_OK(registry->AddFunction(std::move(asinh_checked))); - auto acos = MakeUnaryArithmeticFunctionFloatingPoint("acos", acos_doc); DCHECK_OK(registry->AddFunction(std::move(acos))); diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc index 19dd43c1f1d..e2ebb741cfb 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc @@ -1187,7 +1187,7 @@ TEST(TestUnaryArithmetic, DispatchBest) { // Float types (with _checked variant) for (std::string name : - {"ln", "log2", "log10", "log1p", "sin", "sinh", "cos", "cosh", "tan", "tanh", "asin", "asinh", "acos", "acosh", "atanh"}) { + {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos", "acosh", "atanh"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; for (const auto& ty : {float32(), float64()}) { @@ -1198,7 +1198,7 @@ TEST(TestUnaryArithmetic, DispatchBest) { } // Float types - for (std::string name : {"atan", "sign", "exp"}) { + for (std::string name : {"sinh", "cosh", "tanh", "asinh", "atan", "sign", "exp"}) { for (const auto& ty : {float32(), float64()}) { CheckDispatchBest(name, {ty}, {ty}); CheckDispatchBest(name, {dictionary(int8(), ty)}, {ty}); @@ -1207,7 +1207,7 @@ TEST(TestUnaryArithmetic, DispatchBest) { // Integer -> Float64 (with _checked variant) for (std::string name : - {"ln", "log2", "log10", "log1p", "sin", "sinh", "cos", "cosh", "tan", "tanh", "asin", "asinh", "acos", "acosh", "atanh"}) { + {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos", "acosh", "atanh"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; for (const auto& ty : @@ -1219,7 +1219,7 @@ TEST(TestUnaryArithmetic, DispatchBest) { } // Integer -> Float64 - for (std::string name : {"atan"}) { + for (std::string name : {"sinh", "cosh", "tanh", "asinh", "atan"}) { for (const auto& ty : {int8(), int16(), int32(), int64(), uint8(), uint16(), uint32(), uint64()}) { CheckDispatchBest(name, {ty}, {float64()}); @@ -1229,15 +1229,15 @@ TEST(TestUnaryArithmetic, DispatchBest) { } TEST(TestUnaryArithmetic, Null) { - for (std::string name : {"abs", "acos", "acosh", "asin", "asinh", "atanh", "cos", "cosh", "ln", "log10", "log1p", "log2", - "negate", "sin", "sinh", "tan", "tanh"}) { + for (std::string name : {"abs", "acos", "acosh", "asin", "atanh", "cos", "ln", "log10", "log1p", "log2", + "negate", "sin", "tan"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; AssertNullToNull(name); } } - for (std::string name : {"atan", "bit_wise_not", "sign"}) { + for (std::string name : {"sinh", "cosh", "tanh", "asinh", "atan", "bit_wise_not", "sign"}) { AssertNullToNull(name); } } @@ -2497,6 +2497,19 @@ TYPED_TEST(TestUnaryArithmeticFloating, TrigSin) { this->AssertUnaryOpRaises(Sin, "[Inf, -Inf]", "domain error"); } +TYPED_TEST(TestUnaryArithmeticFloating, TrigSinh) { + this->SetNansEqual(true); + auto sinh = [](const Datum& arg, ArithmeticOptions, ExecContext* ctx) { + return Sinh(arg, ctx); + }; + + this->SetNansEqual(true); + this->AssertUnaryOp(sinh, "[Inf, -Inf]", "[Inf, -Inf]"); + this->AssertUnaryOp(sinh, "[]", "[]"); + this->AssertUnaryOp(sinh, "[null, NaN]", "[null, NaN]"); + this->AssertUnaryOp(sinh, MakeArray(0, M_LN2, M_LN10), "[0, 0.75, 4.95]"); +} + TYPED_TEST(TestUnaryArithmeticFloating, TrigCos) { this->SetNansEqual(true); this->AssertUnaryOp(Cos, "[Inf, -Inf]", "[NaN, NaN]"); @@ -2509,6 +2522,18 @@ TYPED_TEST(TestUnaryArithmeticFloating, TrigCos) { this->AssertUnaryOpRaises(Cos, "[Inf, -Inf]", "domain error"); } +TYPED_TEST(TestUnaryArithmeticFloating, TrigCosh) { + this->SetNansEqual(true); + auto cosh = [](const Datum& arg, ArithmeticOptions, ExecContext* ctx) { + return Cosh(arg, ctx); + }; + + this->AssertUnaryOp(cosh, "[Inf, -Inf]", "[Inf, Inf]"); + this->AssertUnaryOp(cosh, "[]", "[]"); + this->AssertUnaryOp(cosh, "[null, NaN]", "[null, NaN]"); + this->AssertUnaryOp(cosh, MakeArray(0, M_LN2, M_LN10), "[1, 1.25, 5.05]"); +} + TYPED_TEST(TestUnaryArithmeticFloating, TrigTan) { this->SetNansEqual(true); this->AssertUnaryOp(Tan, "[Inf, -Inf]", "[NaN, NaN]"); @@ -2523,6 +2548,18 @@ TYPED_TEST(TestUnaryArithmeticFloating, TrigTan) { this->AssertUnaryOpRaises(Tan, "[Inf, -Inf]", "domain error"); } +TYPED_TEST(TestUnaryArithmeticFloating, TrigTanh) { + this->SetNansEqual(true); + auto tanh = [](const Datum& arg, ArithmeticOptions, ExecContext* ctx) { + return Tanh(arg, ctx); + }; + + this->AssertUnaryOp(tanh, "[Inf, -Inf]", "[1, -1]"); + this->AssertUnaryOp(tanh, "[]", "[]"); + this->AssertUnaryOp(tanh, "[null, NaN]", "[null, NaN]"); + this->AssertUnaryOp(tanh, MakeArray(0, M_LN2), "[0, 0.6]"); +} + TYPED_TEST(TestUnaryArithmeticFloating, TrigAsin) { this->SetNansEqual(true); this->AssertUnaryOp(Asin, "[Inf, -Inf, -2, 2]", "[NaN, NaN, NaN, NaN]"); @@ -2535,6 +2572,18 @@ TYPED_TEST(TestUnaryArithmeticFloating, TrigAsin) { this->AssertUnaryOpRaises(Asin, "[Inf, -Inf, -2, 2]", "domain error"); } +TYPED_TEST(TestUnaryArithmeticFloating, TrigAsinh) { + this->SetNansEqual(true); + auto asinh = [](const Datum& arg, ArithmeticOptions, ExecContext* ctx) { + return Asinh(arg, ctx); + }; + + this->AssertUnaryOp(asinh, "[Inf, -Inf]", "[Inf, -Inf]"); + this->AssertUnaryOp(asinh, "[]", "[]"); + this->AssertUnaryOp(asinh, "[null, NaN]", "[null, NaN]"); + this->AssertUnaryOp(asinh, "[0, 0.75, 4.95]", MakeArray(0, M_LN2, M_LN10)); +} + TYPED_TEST(TestUnaryArithmeticFloating, TrigAcos) { this->SetNansEqual(true); this->AssertUnaryOp(Asin, "[Inf, -Inf, -2, 2]", "[NaN, NaN, NaN, NaN]"); @@ -2547,6 +2596,18 @@ TYPED_TEST(TestUnaryArithmeticFloating, TrigAcos) { this->AssertUnaryOpRaises(Acos, "[Inf, -Inf, -2, 2]", "domain error"); } +TYPED_TEST(TestUnaryArithmeticFloating, TrigAcosh) { + this->SetNansEqual(true); + this->AssertUnaryOp(Acosh, "[0, -1, -Inf]", "[NaN, NaN, NaN]"); + for (auto check_overflow : {false, true}) { + this->SetOverflowCheck(check_overflow); + this->AssertUnaryOp(Acosh, "[]", "[]"); + this->AssertUnaryOp(Acosh, "[null, NaN]", "[null, NaN]"); + this->AssertUnaryOp(Acosh, "[1, 1.25, 5.05]", MakeArray(0, M_LN2, M_LN10)); + } + this->AssertUnaryOpRaises(Acosh, "[0, -1, -Inf]", "domain error"); +} + TYPED_TEST(TestUnaryArithmeticFloating, TrigAtan) { this->SetNansEqual(true); auto atan = [](const Datum& arg, ArithmeticOptions, ExecContext* ctx) { @@ -2572,6 +2633,19 @@ TYPED_TEST(TestBinaryArithmeticFloating, TrigAtan2) { -M_PI_2, 0, M_PI)); } +TYPED_TEST(TestUnaryArithmeticFloating, TrigAtanh) { + this->SetNansEqual(true); + this->AssertUnaryOp(Atanh, "[-Inf, Inf, -2, 2]", "[NaN, NaN, NaN, NaN]"); + for (auto check_overflow : {false, true}) { + this->SetOverflowCheck(check_overflow); + this->AssertUnaryOp(Atanh, "[]", "[]"); + this->AssertUnaryOp(Atanh, "[null, NaN]", "[null, NaN]"); + this->AssertUnaryOp(Atanh, "[-1, 1]", "[-Inf, Inf]"); + this->AssertUnaryOp(Atanh, "[0, 0.6]", MakeArray(0, M_LN2)); + } + this->AssertUnaryOpRaises(Atanh, "[-Inf, Inf, -2, 2]", "domain error"); +} + TYPED_TEST(TestUnaryArithmeticIntegral, Trig) { // Integer arguments promoted to double, sanity check here auto atan = [](const Datum& arg, ArithmeticOptions, ExecContext* ctx) { From 6613dff152ab8e1f10047d9c1150f77768482808 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 20:51:50 +0000 Subject: [PATCH 07/18] linting --- .../compute/kernels/scalar_arithmetic.cc | 45 ++++++++----------- .../compute/kernels/scalar_arithmetic_test.cc | 15 ++++--- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 88bf536f7c5..5d97511004e 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -1263,16 +1263,12 @@ const FunctionDoc sin_doc{"Compute the sine", "to raise an error instead, see \"sin_checked\"."), {"x"}}; - const FunctionDoc sin_checked_doc{"Compute the sine", ("Invalid input values raise an error;\n" "to return NaN instead, see \"sin\"."), {"x"}}; -const FunctionDoc sinh_doc{"Compute the hyperblic sine", -(""), - {"x"}}; - +const FunctionDoc sinh_doc{"Compute the hyperblic sine", (""), {"x"}}; const FunctionDoc cos_doc{"Compute the cosine", ("NaN is returned for invalid input values;\n" @@ -1284,9 +1280,7 @@ const FunctionDoc cos_checked_doc{"Compute the cosine", "to return NaN instead, see \"cos\"."), {"x"}}; -const FunctionDoc cosh_doc{"Compute the hyperbolic cosine", -(""), - {"x"}}; +const FunctionDoc cosh_doc{"Compute the hyperbolic cosine", (""), {"x"}}; const FunctionDoc tan_doc{"Compute the tangent", ("NaN is returned for invalid input values;\n" @@ -1298,9 +1292,7 @@ const FunctionDoc tan_checked_doc{"Compute the tangent", "to return NaN instead, see \"tanh\"."), {"x"}}; -const FunctionDoc tanh_doc{"Compute the hyperbolic tangent", - (""), - {"x"}}; +const FunctionDoc tanh_doc{"Compute the hyperbolic tangent", (""), {"x"}}; const FunctionDoc asin_doc{"Compute the inverse sine", ("NaN is returned for invalid input values;\n" @@ -1312,11 +1304,10 @@ const FunctionDoc asin_checked_doc{"Compute the inverse sine", "to return NaN instead, see \"asin\"."), {"x"}}; - const FunctionDoc asinh_doc{"Compute the inverse hyperbolic sine", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"asinh_checked\"."), - {"x"}}; + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"asinh_checked\"."), + {"x"}}; const FunctionDoc acos_doc{"Compute the inverse cosine", ("NaN is returned for invalid input values;\n" @@ -1329,14 +1320,14 @@ const FunctionDoc acos_checked_doc{"Compute the inverse cosine", {"x"}}; const FunctionDoc acosh_doc{"Compute the inverse hyperbolic cosine", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"acosh_checked\"."), - {"x"}}; + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"acosh_checked\"."), + {"x"}}; const FunctionDoc acosh_checked_doc{"Compute the inverse hyperbolic cosine", - ("Invalid input values raise an error;\n" - "to return NaN instead, see \"acosh\"."), - {"x"}}; + ("Invalid input values raise an error;\n" + "to return NaN instead, see \"acosh\"."), + {"x"}}; const FunctionDoc atan_doc{"Compute the inverse tangent of x", ("The return value is in the range [-pi/2, pi/2];\n" @@ -1348,14 +1339,14 @@ const FunctionDoc atan2_doc{"Compute the inverse tangent of y/x", {"y", "x"}}; const FunctionDoc atanh_doc{"Compute the inverse hyperbolic tangent", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"atanh_checked\"."), - {"x"}}; + ("NaN is returned for invalid input values;\n" + "to raise an error instead, see \"atanh_checked\"."), + {"x"}}; const FunctionDoc atanh_checked_doc{"Compute the inverse hyperbolic tangent", - ("Invalid input values raise an error;\n" - "to return NaN instead, see \"atanh\"."), - {"x"}}; + ("Invalid input values raise an error;\n" + "to return NaN instead, see \"atanh\"."), + {"x"}}; const FunctionDoc ln_doc{ "Compute natural logarithm", diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc index e2ebb741cfb..cb1e07f294c 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc @@ -1186,8 +1186,8 @@ TEST(TestUnaryArithmetic, DispatchBest) { } // Float types (with _checked variant) - for (std::string name : - {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos", "acosh", "atanh"}) { + for (std::string name : {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", + "acos", "acosh", "atanh"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; for (const auto& ty : {float32(), float64()}) { @@ -1206,8 +1206,8 @@ TEST(TestUnaryArithmetic, DispatchBest) { } // Integer -> Float64 (with _checked variant) - for (std::string name : - {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", "acos", "acosh", "atanh"}) { + for (std::string name : {"ln", "log2", "log10", "log1p", "sin", "cos", "tan", "asin", + "acos", "acosh", "atanh"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; for (const auto& ty : @@ -1229,15 +1229,16 @@ TEST(TestUnaryArithmetic, DispatchBest) { } TEST(TestUnaryArithmetic, Null) { - for (std::string name : {"abs", "acos", "acosh", "asin", "atanh", "cos", "ln", "log10", "log1p", "log2", - "negate", "sin", "tan"}) { + for (std::string name : {"abs", "acos", "acosh", "asin", "atanh", "cos", "ln", "log10", + "log1p", "log2", "negate", "sin", "tan"}) { for (std::string suffix : {"", "_checked"}) { name += suffix; AssertNullToNull(name); } } - for (std::string name : {"sinh", "cosh", "tanh", "asinh", "atan", "bit_wise_not", "sign"}) { + for (std::string name : + {"sinh", "cosh", "tanh", "asinh", "atan", "bit_wise_not", "sign"}) { AssertNullToNull(name); } } From d69f5ea98073596f44956fb1bc2aad453c30a917 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 21:17:08 +0000 Subject: [PATCH 08/18] fix windows build --- cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc index cb1e07f294c..e45343aac90 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc @@ -15,6 +15,9 @@ // specific language governing permissions and limitations // under the License. +// Required for Windows to define M_LN* constants +#define _USE_MATH_DEFINES + #include #include #include From 3be9bd5598ef4003ed1cc7949ba3032f185d5d91 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sat, 19 Oct 2024 21:17:24 +0000 Subject: [PATCH 09/18] pyarrow tests --- python/pyarrow/tests/test_compute.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index c16d2f9aacf..063036a0d0f 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -3380,6 +3380,8 @@ def create_sample_expressions(): pc.multiply(a, b), pc.power(a, a), pc.sqrt(a), pc.exp(b), pc.cos(b), pc.sin(b), pc.tan(b), pc.acos(b), pc.atan(b), pc.asin(b), pc.atan2(b, b), + pc.sinh(a), pc.cosh(a), pc.tanh(a), + pc.asinh(a), pc.acosh(b), pc.atanh(pa.scalar(0)), pc.abs(b), pc.sign(a), pc.bit_wise_not(a), pc.bit_wise_and(a, a), pc.bit_wise_or(a, a), pc.bit_wise_xor(a, a), pc.is_nan(b), pc.is_finite(b), From df6acf734b3995fda4b73a7a38b052a0860477a7 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Fri, 1 Nov 2024 00:13:00 +0000 Subject: [PATCH 10/18] doc fixes --- .../compute/kernels/scalar_arithmetic.cc | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 5d97511004e..214f53be62e 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -274,18 +274,6 @@ struct Asinh { } }; -struct AsinhChecked { - template - static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { - static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE(std::isinf(val))) { - *st = Status::Invalid("domain error"); - return val; - } - return std::asin(val); - } -}; - struct Acos { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { @@ -1284,12 +1272,12 @@ const FunctionDoc cosh_doc{"Compute the hyperbolic cosine", (""), {"x"}}; const FunctionDoc tan_doc{"Compute the tangent", ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"tanh_checked\"."), + "to raise an error instead, see \"tan_checked\"."), {"x"}}; const FunctionDoc tan_checked_doc{"Compute the tangent", ("Infinite values raise an error;\n" - "to return NaN instead, see \"tanh\"."), + "to return NaN instead, see \"tan\"."), {"x"}}; const FunctionDoc tanh_doc{"Compute the hyperbolic tangent", (""), {"x"}}; @@ -1304,10 +1292,7 @@ const FunctionDoc asin_checked_doc{"Compute the inverse sine", "to return NaN instead, see \"asin\"."), {"x"}}; -const FunctionDoc asinh_doc{"Compute the inverse hyperbolic sine", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"asinh_checked\"."), - {"x"}}; +const FunctionDoc asinh_doc{"Compute the inverse hyperbolic sine", (""), {"x"}}; const FunctionDoc acos_doc{"Compute the inverse cosine", ("NaN is returned for invalid input values;\n" From 55ac76e53919b3e21f0c880274848451c62c60be Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sun, 3 Nov 2024 18:08:19 +0000 Subject: [PATCH 11/18] fix scalar type --- python/pyarrow/tests/test_compute.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index 063036a0d0f..e388851bea1 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -3369,9 +3369,10 @@ def create_sample_expressions(): g = pc.scalar(pa.scalar(1)) h = pc.scalar(np.int64(2)) j = pc.scalar(False) + k = pc.scalar(0) # These expression consist entirely of literals - literal_exprs = [a, b, c, d, e, g, h, j] + literal_exprs = [a, b, c, d, e, g, h, j, k] # These expressions include at least one function call exprs_with_call = [a == b, a != b, a > b, c & j, c | j, ~c, d.is_valid(), @@ -3381,7 +3382,7 @@ def create_sample_expressions(): pc.exp(b), pc.cos(b), pc.sin(b), pc.tan(b), pc.acos(b), pc.atan(b), pc.asin(b), pc.atan2(b, b), pc.sinh(a), pc.cosh(a), pc.tanh(a), - pc.asinh(a), pc.acosh(b), pc.atanh(pa.scalar(0)), + pc.asinh(a), pc.acosh(b), pc.atanh(k), pc.abs(b), pc.sign(a), pc.bit_wise_not(a), pc.bit_wise_and(a, a), pc.bit_wise_or(a, a), pc.bit_wise_xor(a, a), pc.is_nan(b), pc.is_finite(b), From 5b4315ff98da6e8a644cca3039ac62cc8b5f9c5b Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sun, 3 Nov 2024 18:30:15 +0000 Subject: [PATCH 12/18] add to substrait --- cpp/src/arrow/engine/substrait/extension_set.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/engine/substrait/extension_set.cc b/cpp/src/arrow/engine/substrait/extension_set.cc index cefe53d2847..0c65355489f 100644 --- a/cpp/src/arrow/engine/substrait/extension_set.cc +++ b/cpp/src/arrow/engine/substrait/extension_set.cc @@ -1071,7 +1071,7 @@ struct DefaultExtensionIdRegistry : ExtensionIdRegistryImpl { // Mappings either without a _checked variant or substrait has no overflow option for (const auto& function_name : - {"exp", "sign", "cos", "sin", "tan", "acos", "asin", "atan", "atan2"}) { + {"exp", "sign", "cos", "cosh", "sin", "sinh", "tan", "tanh", "acos", "acosh", "asin", "asinh", "atan", "atanh", "atan2"}) { DCHECK_OK( AddSubstraitCallToArrow({kSubstraitArithmeticFunctionsUri, function_name}, DecodeOptionlessUncheckedArithmetic(function_name))); @@ -1207,7 +1207,13 @@ struct DefaultExtensionIdRegistry : ExtensionIdRegistryImpl { {kSubstraitArithmeticFunctionsUri, "acos"}, {kSubstraitArithmeticFunctionsUri, "asin"}, {kSubstraitArithmeticFunctionsUri, "atan"}, - {kSubstraitArithmeticFunctionsUri, "atan2"}}) { + {kSubstraitArithmeticFunctionsUri, "atan2"}, + {kSubstraitArithmeticFunctionsUri, "cosh"}, + {kSubstraitArithmeticFunctionsUri, "sinh"}, + {kSubstraitArithmeticFunctionsUri, "tanh"}, + {kSubstraitArithmeticFunctionsUri, "acosh"}, + {kSubstraitArithmeticFunctionsUri, "asinh"}, + {kSubstraitArithmeticFunctionsUri, "atanh"}}) { Id fn_id{fn_pair.first, fn_pair.second}; DCHECK_OK(AddArrowToSubstraitCall(std::string(fn_pair.second), EncodeBasic(fn_id))); } From 4b4c623b1b544fd1ca70fc731e97c7c6e1b1fb97 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sun, 3 Nov 2024 19:38:58 +0000 Subject: [PATCH 13/18] lint --- cpp/src/arrow/engine/substrait/extension_set.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/engine/substrait/extension_set.cc b/cpp/src/arrow/engine/substrait/extension_set.cc index 0c65355489f..ac25eba684b 100644 --- a/cpp/src/arrow/engine/substrait/extension_set.cc +++ b/cpp/src/arrow/engine/substrait/extension_set.cc @@ -1071,7 +1071,8 @@ struct DefaultExtensionIdRegistry : ExtensionIdRegistryImpl { // Mappings either without a _checked variant or substrait has no overflow option for (const auto& function_name : - {"exp", "sign", "cos", "cosh", "sin", "sinh", "tan", "tanh", "acos", "acosh", "asin", "asinh", "atan", "atanh", "atan2"}) { + {"exp", "sign", "cos", "cosh", "sin", "sinh", "tan", "tanh", "acos", "acosh", + "asin", "asinh", "atan", "atanh", "atan2"}) { DCHECK_OK( AddSubstraitCallToArrow({kSubstraitArithmeticFunctionsUri, function_name}, DecodeOptionlessUncheckedArithmetic(function_name))); From 6652fe72992f1e341a8779fc99b88c9da9c5a688 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Thu, 5 Dec 2024 21:12:21 +0000 Subject: [PATCH 14/18] Documentation updates --- cpp/src/arrow/compute/api_scalar.cc | 8 ++--- .../compute/kernels/scalar_arithmetic.cc | 2 +- .../compute/kernels/scalar_arithmetic_test.cc | 1 - docs/source/cpp/compute.rst | 29 +++++++++++++++++++ 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/compute/api_scalar.cc b/cpp/src/arrow/compute/api_scalar.cc index b0a8c94ada1..61a16f5f5eb 100644 --- a/cpp/src/arrow/compute/api_scalar.cc +++ b/cpp/src/arrow/compute/api_scalar.cc @@ -740,18 +740,18 @@ SCALAR_ARITHMETIC_UNARY(Ln, "ln", "ln_checked") SCALAR_ARITHMETIC_UNARY(Log10, "log10", "log10_checked") SCALAR_ARITHMETIC_UNARY(Log1p, "log1p", "log1p_checked") SCALAR_ARITHMETIC_UNARY(Log2, "log2", "log2_checked") -SCALAR_ARITHMETIC_UNARY(Sqrt, "sqrt", "sqrt_checked") SCALAR_ARITHMETIC_UNARY(Negate, "negate", "negate_checked") SCALAR_ARITHMETIC_UNARY(Sin, "sin", "sin_checked") +SCALAR_ARITHMETIC_UNARY(Sqrt, "sqrt", "sqrt_checked") SCALAR_ARITHMETIC_UNARY(Tan, "tan", "tan_checked") SCALAR_EAGER_UNARY(Asinh, "asinh") -SCALAR_EAGER_UNARY(Cosh, "cosh") -SCALAR_EAGER_UNARY(Sinh, "sinh") -SCALAR_EAGER_UNARY(Tanh, "tanh") SCALAR_EAGER_UNARY(Atan, "atan") +SCALAR_EAGER_UNARY(Cosh, "cosh") SCALAR_EAGER_UNARY(Exp, "exp") SCALAR_EAGER_UNARY(Expm1, "expm1") SCALAR_EAGER_UNARY(Sign, "sign") +SCALAR_EAGER_UNARY(Sinh, "sinh") +SCALAR_EAGER_UNARY(Tanh, "tanh") Result Round(const Datum& arg, RoundOptions options, ExecContext* ctx) { return CallFunction("round", {arg}, &options, ctx); diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 214f53be62e..4cd7784e8dd 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -1256,7 +1256,7 @@ const FunctionDoc sin_checked_doc{"Compute the sine", "to return NaN instead, see \"sin\"."), {"x"}}; -const FunctionDoc sinh_doc{"Compute the hyperblic sine", (""), {"x"}}; +const FunctionDoc sinh_doc{"Compute the hyperbolic sine", (""), {"x"}}; const FunctionDoc cos_doc{"Compute the cosine", ("NaN is returned for invalid input values;\n" diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc index e45343aac90..3c53cc1480c 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc @@ -2507,7 +2507,6 @@ TYPED_TEST(TestUnaryArithmeticFloating, TrigSinh) { return Sinh(arg, ctx); }; - this->SetNansEqual(true); this->AssertUnaryOp(sinh, "[Inf, -Inf]", "[Inf, -Inf]"); this->AssertUnaryOp(sinh, "[]", "[]"); this->AssertUnaryOp(sinh, "[null, NaN]", "[null, NaN]"); diff --git a/docs/source/cpp/compute.rst b/docs/source/cpp/compute.rst index 3c264fb4767..ec53fb04688 100644 --- a/docs/source/cpp/compute.rst +++ b/docs/source/cpp/compute.rst @@ -721,6 +721,35 @@ Decimal values are accepted, but are cast to Float64 first. | tan_checked | Unary | Float32/Float64/Decimal | Float32/Float64 | +--------------------------+------------+-------------------------+---------------------+ +Hyperbolic trigonometric functions +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Hyperbolic trigonometric functions are also supported, and, where applicable, also offer +``_checked`` variants that check for domain errors if needed. + +Decimal values are accepted, but are cast to Float64 first. + ++--------------------------+------------+-------------------------+---------------------+ +| Function name | Arity | Input types | Output type | ++==========================+============+=========================+=====================+ +| acosh | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ +| acosh_checked | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ +| asinh | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ +| atanh | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ +| atanh_checked | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ +| cosh | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ +| sinh | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ +| tanh | Unary | Float32/Float64/Decimal | Float32/Float64 | ++--------------------------+------------+-------------------------+---------------------+ + + Comparisons ~~~~~~~~~~~ From 547f3eb3a636a23db64a66fa86b8c9d0b319f104 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Fri, 6 Dec 2024 00:24:31 +0000 Subject: [PATCH 15/18] trig ranges --- .../compute/kernels/scalar_arithmetic.cc | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 4cd7784e8dd..606a7c7aa05 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -333,6 +333,9 @@ struct Atanh { static enable_if_floating_value Call(KernelContext*, Arg0 val, Status*) { static_assert(std::is_same::value, ""); if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0))) { + // N.B. This predicate does *not* match the predicate in AtanhChecked. In + // GH-44630 it was decided that the checked version should error when asked + // for +/- 1 as an input and the unchecked version should return +/- oo return std::numeric_limits::quiet_NaN(); } return std::atanh(val); @@ -343,7 +346,10 @@ struct AtanhChecked { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0))) { + if (ARROW_PREDICT_FALSE((val <= -1.0 || val => 1.0))) { + // N.B. This predicate does *not* match the predicate in Atanh. In GH-44630 it was + // decided that the checked version should error when asked for +/- 1 as an input + // and the unchecked version should return +/- oo *st = Status::Invalid("domain error"); return val; } @@ -1305,12 +1311,12 @@ const FunctionDoc acos_checked_doc{"Compute the inverse cosine", {"x"}}; const FunctionDoc acosh_doc{"Compute the inverse hyperbolic cosine", - ("NaN is returned for invalid input values;\n" + ("NaN is returned for input values < 1.0;\n" "to raise an error instead, see \"acosh_checked\"."), {"x"}}; const FunctionDoc acosh_checked_doc{"Compute the inverse hyperbolic cosine", - ("Invalid input values raise an error;\n" + ("Input values < 1.0 raise an error;\n" "to return NaN instead, see \"acosh\"."), {"x"}}; @@ -1324,12 +1330,13 @@ const FunctionDoc atan2_doc{"Compute the inverse tangent of y/x", {"y", "x"}}; const FunctionDoc atanh_doc{"Compute the inverse hyperbolic tangent", - ("NaN is returned for invalid input values;\n" - "to raise an error instead, see \"atanh_checked\"."), + ("NaN is returned for input values x with |x| > 1. " + "At x = +/- 1, returns +/- infinity.\n" + "To raise an error instead, see \"atanh_checked\"."), {"x"}}; const FunctionDoc atanh_checked_doc{"Compute the inverse hyperbolic tangent", - ("Invalid input values raise an error;\n" + ("Input values x with |x| >= 1.0 raise an error\n" "to return NaN instead, see \"atanh\"."), {"x"}}; From 0f63fa714873df120c8a959235dbfe8fa8eef4c3 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Fri, 6 Dec 2024 00:27:51 +0000 Subject: [PATCH 16/18] add test --- cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc index 3c53cc1480c..9a1a569081d 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc @@ -2639,14 +2639,14 @@ TYPED_TEST(TestBinaryArithmeticFloating, TrigAtan2) { TYPED_TEST(TestUnaryArithmeticFloating, TrigAtanh) { this->SetNansEqual(true); this->AssertUnaryOp(Atanh, "[-Inf, Inf, -2, 2]", "[NaN, NaN, NaN, NaN]"); + this->AssertUnaryOp(Atanh, "[-1, 1]", "[-Inf, Inf]"); for (auto check_overflow : {false, true}) { this->SetOverflowCheck(check_overflow); this->AssertUnaryOp(Atanh, "[]", "[]"); this->AssertUnaryOp(Atanh, "[null, NaN]", "[null, NaN]"); - this->AssertUnaryOp(Atanh, "[-1, 1]", "[-Inf, Inf]"); this->AssertUnaryOp(Atanh, "[0, 0.6]", MakeArray(0, M_LN2)); } - this->AssertUnaryOpRaises(Atanh, "[-Inf, Inf, -2, 2]", "domain error"); + this->AssertUnaryOpRaises(Atanh, "[-Inf, Inf, -1, 1, -2, 2]", "domain error"); } TYPED_TEST(TestUnaryArithmeticIntegral, Trig) { From 2acbf5a6d017939b1d28052ae677fd3a4e60c182 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sun, 8 Dec 2024 18:18:55 +0000 Subject: [PATCH 17/18] typo in comparison --- cpp/src/arrow/compute/kernels/scalar_arithmetic.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index 606a7c7aa05..e9250116f71 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -346,7 +346,7 @@ struct AtanhChecked { template static enable_if_floating_value Call(KernelContext*, Arg0 val, Status* st) { static_assert(std::is_same::value, ""); - if (ARROW_PREDICT_FALSE((val <= -1.0 || val => 1.0))) { + if (ARROW_PREDICT_FALSE((val <= -1.0 || val >= 1.0))) { // N.B. This predicate does *not* match the predicate in Atanh. In GH-44630 it was // decided that the checked version should error when asked for +/- 1 as an input // and the unchecked version should return +/- oo From bd3136040e817defb1d9b20a6603334abaffb255 Mon Sep 17 00:00:00 2001 From: Kevin H Wilson Date: Sun, 8 Dec 2024 19:12:19 +0000 Subject: [PATCH 18/18] newline --- cpp/src/arrow/compute/kernels/scalar_arithmetic.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc index e9250116f71..c13dae573a3 100644 --- a/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc +++ b/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc @@ -1330,7 +1330,7 @@ const FunctionDoc atan2_doc{"Compute the inverse tangent of y/x", {"y", "x"}}; const FunctionDoc atanh_doc{"Compute the inverse hyperbolic tangent", - ("NaN is returned for input values x with |x| > 1. " + ("NaN is returned for input values x with |x| > 1.\n" "At x = +/- 1, returns +/- infinity.\n" "To raise an error instead, see \"atanh_checked\"."), {"x"}};