Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-13345: [C++] Add basic implementation for log to base b #10898

Closed
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cpp/src/arrow/compute/api_scalar.cc
Expand Up @@ -349,6 +349,7 @@ SCALAR_ARITHMETIC_BINARY(Divide, "divide", "divide_checked")
SCALAR_ARITHMETIC_BINARY(Power, "power", "power_checked")
SCALAR_ARITHMETIC_BINARY(ShiftLeft, "shift_left", "shift_left_checked")
SCALAR_ARITHMETIC_BINARY(ShiftRight, "shift_right", "shift_right_checked")
SCALAR_ARITHMETIC_BINARY(Logb, "logb", "logb_checked")
SCALAR_EAGER_BINARY(Atan2, "atan2")
SCALAR_EAGER_UNARY(Floor, "floor")
SCALAR_EAGER_UNARY(Ceil, "ceil")
Expand Down
14 changes: 14 additions & 0 deletions cpp/src/arrow/compute/api_scalar.h
Expand Up @@ -485,6 +485,20 @@ ARROW_EXPORT
Result<Datum> Log1p(const Datum& arg, ArithmeticOptions options = ArithmeticOptions(),
ExecContext* ctx = NULLPTR);

/// \brief Get the log of a value to the given base.
///
/// If argument is null the result will be null.
///
/// \param[in] arg The values to compute the logarithm for.
/// \param[in] base The given base.
/// \param[in] options arithmetic options (overflow handling), optional
/// \param[in] ctx the function execution context, optional
/// \return the elementwise log to the given base
ARROW_EXPORT
Result<Datum> Logb(const Datum& arg, const Datum& base,
ArithmeticOptions options = ArithmeticOptions(),
ExecContext* ctx = NULLPTR);

/// \brief Round to the nearest integer less than or equal in magnitude to the
/// argument. Array values can be of arbitrary length. If argument is null the
/// result will be null.
Expand Down
67 changes: 67 additions & 0 deletions cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
Expand Up @@ -817,6 +817,40 @@ struct Log1pChecked {
}
};

struct Logb {
template <typename T, typename Arg0, typename Arg1>
static enable_if_floating_point<T> Call(KernelContext*, Arg0 x, Arg1 base, Status*) {
static_assert(std::is_same<T, Arg0>::value, "");
static_assert(std::is_same<Arg0, Arg1>::value, "");
if (x == 0.0) {
if (base == 0.0 || base < 0.0) {
return std::numeric_limits<T>::quiet_NaN();
} else {
return -std::numeric_limits<T>::infinity();
}
} else if (x < 0.0) {
return std::numeric_limits<T>::quiet_NaN();
}
return std::log(x) / std::log(base);
}
};

struct LogbChecked {
template <typename T, typename Arg0, typename Arg1>
static enable_if_floating_point<T> Call(KernelContext*, Arg0 x, Arg1 base, Status* st) {
static_assert(std::is_same<T, Arg0>::value, "");
static_assert(std::is_same<Arg0, Arg1>::value, "");
if (x == 0.0 || base == 0.0) {
*st = Status::Invalid("logarithm of zero");
return x;
} else if (x < 0.0 || base < 0.0) {
*st = Status::Invalid("logarithm of negative number");
return x;
}
return std::log(x) / std::log(base);
}
};

struct Floor {
template <typename T, typename Arg>
static constexpr enable_if_floating_point<T> Call(KernelContext*, Arg arg, Status*) {
Expand Down Expand Up @@ -1317,6 +1351,19 @@ std::shared_ptr<ScalarFunction> MakeArithmeticFunctionFloatingPoint(
return func;
}

template <typename Op>
std::shared_ptr<ScalarFunction> MakeArithmeticFunctionFloatingPointNotNull(
std::string name, const FunctionDoc* doc) {
auto func =
std::make_shared<ArithmeticFloatingPointFunction>(name, Arity::Binary(), doc);
for (const auto& ty : FloatingPointTypes()) {
auto output = is_integer(ty->id()) ? float64() : ty;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this line is redundant now (ty is never integer). I had filed ARROW-13361 to clean this up.

auto exec = GenerateArithmeticFloatingPoint<ScalarBinaryNotNullEqualTypes, Op>(ty);
DCHECK_OK(func->AddKernel({ty, ty}, output, exec));
}
return func;
}

const FunctionDoc absolute_value_doc{
"Calculate the absolute value of the argument element-wise",
("Results will wrap around on integer overflow.\n"
Expand Down Expand Up @@ -1589,6 +1636,19 @@ const FunctionDoc log1p_checked_doc{
"-inf or NaN."),
{"x"}};

const FunctionDoc logb_doc{
"Compute log of x to base b of arguments element-wise",
("Values <= -1 return -inf or NaN. Null values return null.\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Values <= 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

"Use function \"logb_checked\" if you want non-positive values to raise an error."),
{"x", "b"}};

const FunctionDoc logb_checked_doc{
"Compute log of x to base b of arguments element-wise",
("Values <= -1 return -inf or NaN. Null values return null.\n"
"Use function \"logb\" if you want non-positive values to return "
"-inf or NaN."),
{"x", "b"}};

const FunctionDoc floor_doc{
"Round down to the nearest integer",
("Calculate the nearest integer less than or equal in magnitude to the "
Expand Down Expand Up @@ -1806,6 +1866,13 @@ void RegisterScalarArithmetic(FunctionRegistry* registry) {
"log1p_checked", &log1p_checked_doc);
DCHECK_OK(registry->AddFunction(std::move(log1p_checked)));

auto logb = MakeArithmeticFunctionFloatingPoint<Logb>("logb", &logb_doc);
DCHECK_OK(registry->AddFunction(std::move(logb)));

auto logb_checked = MakeArithmeticFunctionFloatingPointNotNull<LogbChecked>(
"logb_checked", &logb_checked_doc);
DCHECK_OK(registry->AddFunction(std::move(logb_checked)));

// ----------------------------------------------------------------------
// Rounding functions
auto floor = MakeUnaryArithmeticFunctionFloatingPoint<Floor>("floor", &floor_doc);
Expand Down
58 changes: 58 additions & 0 deletions cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc
Expand Up @@ -229,6 +229,16 @@ class TestBinaryArithmetic : public TestBase {
AssertBinop(func, lhs, right, expected);
}

// (Array, Scalar) => Array
void AssertBinop(BinaryFunction func, const std::string& lhs,
const std::shared_ptr<Scalar>& right,
const std::shared_ptr<Array>& expected) {
auto left = ArrayFromJSON(type_singleton(), lhs);

ASSERT_OK_AND_ASSIGN(auto actual, func(left, right, options_, nullptr));
ValidateAndAssertApproxEqual(actual.make_array(), expected);
}

// (Array, Scalar)
void AssertBinop(BinaryFunction func, const std::string& lhs,
const std::shared_ptr<Scalar>& right, const std::string& expected) {
Expand All @@ -248,6 +258,15 @@ class TestBinaryArithmetic : public TestBase {
AssertBinop(func, left, right, expected);
}

// (Array, Array) => Array
void AssertBinop(BinaryFunction func, const std::string& lhs, const std::string& rhs,
const std::shared_ptr<Array>& expected) {
auto left = ArrayFromJSON(type_singleton(), lhs);
auto right = ArrayFromJSON(type_singleton(), rhs);

AssertBinop(func, left, right, expected);
}

// (Array, Array)
void AssertBinop(BinaryFunction func, const std::shared_ptr<Array>& left,
const std::shared_ptr<Array>& right,
Expand Down Expand Up @@ -2001,6 +2020,45 @@ TYPED_TEST(TestUnaryArithmeticIntegral, Log) {
}
}

TYPED_TEST(TestBinaryArithmeticIntegral, Log) {
// Integer arguments promoted to double, sanity check here
this->AssertBinop(Logb, "[1, 10, null]", "[10, 10, null]",
ArrayFromJSON(float64(), "[0, 1, null]"));
this->AssertBinop(Logb, "[1, 2, null]", "[2, 2, null]",
ArrayFromJSON(float64(), "[0, 1, null]"));
this->AssertBinop(Logb, "[10, 100, null]", this->MakeScalar(10),
ArrayFromJSON(float64(), "[1, 2, null]"));
}

TYPED_TEST(TestBinaryArithmeticFloating, Log) {
// Integer arguments promoted to double, sanity check here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think this comment applies here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right.

this->AssertBinop(Logb, "[1.0, 10.0, null]", "[10.0, 10.0, null]", "[0.0, 1.0, null]");
this->AssertBinop(Logb, "[1.0, 2.0, null]", "[2.0, 2.0, null]", "[0.0, 1.0, null]");
this->AssertBinop(Logb, "[10.0, 100.0, 1000.0, null]", this->MakeScalar(10),
"[1.0, 2.0, 3.0, null]");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also test what happens on Log of NaN, Inf, and -Inf. We should also check the error cases below here, with both check_overflow = true and check_overflow = false. Or really, it should look similar to the tests for the unary log functions here:

TYPED_TEST(TestUnaryArithmeticFloating, Log) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More unit tests were added.

}

TYPED_TEST(TestBinaryArithmeticSigned, Log) {
// Integer arguments promoted to double, sanity check here
this->SetNansEqual(true);
this->SetOverflowCheck(false);
this->AssertBinop(Logb, "[-1, 0]", this->MakeScalar(10),
ArrayFromJSON(float64(), "[NaN, -Inf]"));
this->AssertBinop(Logb, "[-1, 0]", this->MakeScalar(2),
ArrayFromJSON(float64(), "[NaN, -Inf]"));
this->AssertBinop(Logb, "[10, 100]", this->MakeScalar(-1),
ArrayFromJSON(float64(), "[NaN, NaN]"));
this->AssertBinop(Logb, "[-1, 0, null]", this->MakeScalar(-1),
ArrayFromJSON(float64(), "[NaN, NaN, null]"));
this->AssertBinop(Logb, "[10, 100]", this->MakeScalar(0),
ArrayFromJSON(float64(), "[0, 0]"));
this->SetOverflowCheck(true);
this->AssertBinopRaises(Logb, "[0]", "[10]", "logarithm of zero");
this->AssertBinopRaises(Logb, "[-1]", "[10]", "logarithm of negative number");
this->AssertBinopRaises(Logb, "[10]", "[0]", "logarithm of zero");
this->AssertBinopRaises(Logb, "[100]", "[-1]", "logarithm of negative number");
}

TYPED_TEST(TestUnaryArithmeticSigned, Log) {
// Integer arguments promoted to double, sanity check here
this->SetNansEqual(true);
Expand Down
4 changes: 4 additions & 0 deletions docs/source/cpp/compute.rst
Expand Up @@ -389,6 +389,10 @@ variants that check for domain errors if needed.
+--------------------------+------------+--------------------+---------------------+
| log2_checked | Unary | Float32/Float64 | Float32/Float64 |
+--------------------------+------------+--------------------+---------------------+
| logb | Binary | Float32/Float64 | Float32/Float64 |
+--------------------------+------------+--------------------+---------------------+
| logb_checked | Binary | Float32/Float64 | Float32/Float64 |
+--------------------------+------------+--------------------+---------------------+

Trigonometric functions
~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
2 changes: 2 additions & 0 deletions docs/source/python/api/compute.rst
Expand Up @@ -104,6 +104,8 @@ variants which detect domain errors.
log1p_checked
log2
log2_checked
logb
logb_checked

Trigonometric Functions
-----------------------
Expand Down