felipecrv commented on code in PR #44630:
URL: https://github.com/apache/arrow/pull/44630#discussion_r1871937532
##########
cpp/src/arrow/compute/kernels/scalar_arithmetic.cc:
##########
@@ -273,6 +328,29 @@ struct Atan {
}
};
+struct Atanh {
+ template <typename T, typename Arg0>
+ static enable_if_floating_value<Arg0, T> Call(KernelContext*, Arg0 val,
Status*) {
+ static_assert(std::is_same<T, Arg0>::value, "");
+ if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0))) {
Review Comment:
C++ docs also say `If the argument is ±1, a pole error occurs.` and `if the
argument is ±1, ±∞ is returned and [FE_DIVBYZERO] is raised.
They have different names for the errors (range/pole/FE_DIVBYZERO) but that
doesn't change the fact that Mathematically, `atanh` isn't defined at the -1.0
and 1.0 points.
The function **converges to** $\pm\infty$ as $x$ approaches the extremes of
the range which makes returning `-Inf/+Inf` more reasonable than returning
`NaN` at these extremes, but it's not defined at these points per se.
$$\lim_{x \to 1.0}{tanh^{-1}(x)} = \infty$$
$$\lim_{x \to -1.0}{tanh^{-1}(x)} = -\infty$$
So I think you can drop my suggestion on the unchecked version (this one)
and let the implementation return `-Inf/+Inf` instead of `NaN` at these
extremes, but the `atanh_checked` variation should fail at `-1.0/+1.0` instead
of returning the infinite values.
~Docs should be updated accordingly to describe this behavior.~
I updated my suggestions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
