pitrou commented on a change in pull request #10567:
URL: https://github.com/apache/arrow/pull/10567#discussion_r662440835
##########
File path: cpp/src/arrow/compute/api_scalar.h
##########
@@ -396,6 +396,52 @@ Result<Datum> Atan(const Datum& arg, ExecContext* ctx =
NULLPTR);
ARROW_EXPORT
Result<Datum> Atan2(const Datum& y, const Datum& x, ExecContext* ctx =
NULLPTR);
+/// \brief Get the natural log of a value. Array values can be of arbitrary
+/// length. If argument is null the result will be null.
+///
+/// \param[in] arg the value transformed
Review comment:
Why "transformed"?
##########
File path: cpp/src/arrow/compute/api_scalar.h
##########
@@ -396,6 +396,52 @@ Result<Datum> Atan(const Datum& arg, ExecContext* ctx =
NULLPTR);
ARROW_EXPORT
Result<Datum> Atan2(const Datum& y, const Datum& x, ExecContext* ctx =
NULLPTR);
+/// \brief Get the natural log of a value. Array values can be of arbitrary
+/// length. If argument is null the result will be null.
Review comment:
I'm not sure that "Array values can be of arbitrary length" is a useful
mention. It's normally true of all scalar (elemen-wise) functions.
Also, can we keep the `\brief` sentence a single one-liner, and put the
description after a newline?
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -686,6 +686,118 @@ struct Atan2 {
}
};
+struct LogNatural {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status*) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ return -std::numeric_limits<T>::infinity();
+ } else if (arg < 0.0) {
+ return std::numeric_limits<T>::quiet_NaN();
+ }
+ return std::log(arg);
+ }
+};
+
+struct LogNaturalChecked {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status* st) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ *st = Status::Invalid("divide by zero");
+ return arg;
+ } else if (arg < 0.0) {
+ *st = Status::Invalid("domain error");
Review comment:
Perhaps something more precise, e.g. "logarithm of negative number"?
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -686,6 +686,118 @@ struct Atan2 {
}
};
+struct LogNatural {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status*) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ return -std::numeric_limits<T>::infinity();
+ } else if (arg < 0.0) {
+ return std::numeric_limits<T>::quiet_NaN();
+ }
+ return std::log(arg);
+ }
+};
+
+struct LogNaturalChecked {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status* st) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ *st = Status::Invalid("divide by zero");
+ return arg;
+ } else if (arg < 0.0) {
+ *st = Status::Invalid("domain error");
+ return arg;
+ }
+ return std::log(arg);
+ }
+};
+
+struct Log10 {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status*) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ return -std::numeric_limits<T>::infinity();
+ } else if (arg < 0.0) {
+ return std::numeric_limits<T>::quiet_NaN();
+ }
+ return std::log10(arg);
+ }
+};
+
+struct Log10Checked {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status* st) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0) {
+ *st = Status::Invalid("divide by zero");
+ return arg;
+ } else if (arg < 0) {
+ *st = Status::Invalid("domain error");
+ return arg;
+ }
+ return std::log10(arg);
+ }
+};
+
+struct Log2 {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status*) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ return -std::numeric_limits<T>::infinity();
+ } else if (arg < 0.0) {
+ return std::numeric_limits<T>::quiet_NaN();
+ }
+ return std::log2(arg);
+ }
+};
+
+struct Log2Checked {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status* st) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ *st = Status::Invalid("divide by zero");
+ return arg;
+ } else if (arg < 0.0) {
+ *st = Status::Invalid("domain error");
+ return arg;
+ }
+ return std::log2(arg);
+ }
+};
Review comment:
I don't know if that's worth it, but these three kernels have very
similar implementations, maybe something could be shared. Or perhaps that's
pointless generalization.
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -686,6 +686,118 @@ struct Atan2 {
}
};
+struct LogNatural {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status*) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ return -std::numeric_limits<T>::infinity();
+ } else if (arg < 0.0) {
+ return std::numeric_limits<T>::quiet_NaN();
+ }
+ return std::log(arg);
+ }
+};
+
+struct LogNaturalChecked {
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext*, Arg arg,
Status* st) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ if (arg == 0.0) {
+ *st = Status::Invalid("divide by zero");
Review comment:
I don't know if that's the best error message. Perhaps "logarithm of
zero"?
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -1295,6 +1407,60 @@ const FunctionDoc atan2_doc{
"Compute the inverse tangent using argument signs to determine the
quadrant",
("Integer arguments return double values."),
{"y", "x"}};
+
+const FunctionDoc ln_doc{
+ "Take natural log of arguments element-wise",
Review comment:
We use "Compute" above.
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
##########
@@ -1821,5 +1821,57 @@ TYPED_TEST(TestBinaryArithmeticFloating, TrigAtan2) {
-M_PI_2, 0, M_PI));
}
+TYPED_TEST(TestUnaryArithmeticFloating, Log) {
+ using CType = typename TestFixture::CType;
+ auto ty = this->type_singleton();
+ this->SetNansEqual(true);
+ for (auto check_overflow : {false, true}) {
+ this->SetOverflowCheck(check_overflow);
+ this->AssertUnaryOp(Ln, "[1, 2.7182818284590452354, null, NaN, Inf]",
+ "[0, 1, null, NaN, Inf]");
+ // N.B. min() for float types is smallest normal number > 0
+ this->AssertUnaryOp(Ln, std::numeric_limits<CType>::min(),
+ std::log(std::numeric_limits<CType>::min()));
+ this->AssertUnaryOp(Ln, std::numeric_limits<CType>::max(),
+ std::log(std::numeric_limits<CType>::max()));
+ this->AssertUnaryOp(Log10, "[1, 10, null, NaN, Inf]", "[0, 1, null, NaN,
Inf]");
+ this->AssertUnaryOp(Log10, std::numeric_limits<CType>::min(),
+ std::log10(std::numeric_limits<CType>::min()));
+ this->AssertUnaryOp(Log10, std::numeric_limits<CType>::max(),
+ std::log10(std::numeric_limits<CType>::max()));
+ this->AssertUnaryOp(Log2, "[1, 2, null, NaN, Inf]", "[0, 1, null, NaN,
Inf]");
+ this->AssertUnaryOp(Log2, std::numeric_limits<CType>::min(),
+ std::log2(std::numeric_limits<CType>::min()));
+ this->AssertUnaryOp(Log2, std::numeric_limits<CType>::max(),
+ std::log2(std::numeric_limits<CType>::max()));
+ this->AssertUnaryOp(Log1p, "[0, 1.7182818284590452354, null, NaN, Inf]",
+ "[0, 1, null, NaN, Inf]");
+ this->AssertUnaryOp(Log1p, std::numeric_limits<CType>::min(),
+ std::log1p(std::numeric_limits<CType>::min()));
+ this->AssertUnaryOp(Log1p, std::numeric_limits<CType>::max(),
+ std::log1p(std::numeric_limits<CType>::max()));
+ }
+ this->AssertUnaryOpRaises(Ln, "[0]", "divide by zero");
+ this->AssertUnaryOpRaises(Ln, "[-1]", "domain error");
+ this->AssertUnaryOpRaises(Ln, "[-Inf]", "domain error");
+ this->AssertUnaryOpRaises(Ln,
MakeArray(std::numeric_limits<CType>::lowest()),
+ "domain error");
+ this->AssertUnaryOpRaises(Log10, "[0]", "divide by zero");
+ this->AssertUnaryOpRaises(Log10, "[-1]", "domain error");
+ this->AssertUnaryOpRaises(Log10, "[-Inf]", "domain error");
+ this->AssertUnaryOpRaises(Log10,
MakeArray(std::numeric_limits<CType>::lowest()),
+ "domain error");
+ this->AssertUnaryOpRaises(Log2, "[0]", "divide by zero");
+ this->AssertUnaryOpRaises(Log2, "[-1]", "domain error");
+ this->AssertUnaryOpRaises(Log2, "[-Inf]", "domain error");
+ this->AssertUnaryOpRaises(Log2,
MakeArray(std::numeric_limits<CType>::lowest()),
+ "domain error");
+ this->AssertUnaryOpRaises(Log1p, "[-1]", "divide by zero");
+ this->AssertUnaryOpRaises(Log1p, "[-2]", "domain error");
+ this->AssertUnaryOpRaises(Log1p, "[-Inf]", "domain error");
+ this->AssertUnaryOpRaises(Log1p,
MakeArray(std::numeric_limits<CType>::lowest()),
+ "domain error");
Review comment:
Can we check the error cases for the non-checked variants as well?
--
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]
