This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new f95b0015a6 GH-51194: [C++] Fix cumulative_max/min default start for
floating-point types (#51203)
f95b0015a6 is described below
commit f95b0015a6e4850c3cba69c9e8ba1d384037b9e0
Author: Adarsh Mishra <[email protected]>
AuthorDate: Mon Sep 7 15:42:02 2026 +0530
GH-51194: [C++] Fix cumulative_max/min default start for floating-point
types (#51203)
## Rationale for this change
`Identity<Max>` (the implicit default start of `cumulative_max`) used
`std::numeric_limits<T>::min()`, which is the smallest **positive** value for
floating-point types. Any input starting with a non-positive value therefore
never replaced the start as the new maximum:
```python
>>> import pyarrow as pa, pyarrow.compute as pc
>>> pc.cumulative_max(pa.array([-2.5, 2.5])).to_pylist()
[2.2250738585072014e-308, 2.5] # expected [-2.5, 2.5]
```
## What changes are included in this PR?
- `Identity<Max>` now uses `-infinity` for floating-point types (and
half-float), and keeps `lowest()` for integer types. `-infinity` is the only
value satisfying the documented identity property `Op(identity, x) = x for all
x` — `lowest()` (most negative finite value) fails it for a leading `-inf`
input.
- `Identity<Min>` gets the mirror fix: `+infinity` for floating-point
types. Previously `cumulative_min([inf, 1.0])` returned `[DBL_MAX, 1.0]`
instead of `[inf, 1.0]`.
Closes #51194.
## Are these changes tested?
- Added `TestCumulative.NegativeValues` covering negative integers,
negative floats, and `±Inf` inputs for both `cumulative_max` and
`cumulative_min`.
- Added a pyarrow regression test reproducing the issue
(`test_cumulative_max_min_negative_default_start`).
## Are there any user-facing changes?
Yes: `cumulative_max`/`cumulative_min` now return correct results for
floating-point inputs whose first value is non-positive (for max) /
non-negative or `inf` (for min). The documented behavior — 'the default start
is the minimum/maximum value of input type' — is now actually honored.
* GitHub Issue: #51194
Authored-by: Adarsh <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
.../compute/kernels/base_arithmetic_internal.h | 16 ++++++++++--
.../compute/kernels/vector_cumulative_ops_test.cc | 29 ++++++++++++++++++++++
python/pyarrow/tests/test_compute.py | 22 ++++++++++++++++
3 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h
b/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h
index b4840061ae..320c40373c 100644
--- a/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h
+++ b/cpp/src/arrow/compute/kernels/base_arithmetic_internal.h
@@ -759,7 +759,14 @@ template <>
struct Identity<Max> {
template <typename Value>
static constexpr Value value() {
- return std::numeric_limits<Value>::min();
+ // Note that `min()` returns the smallest positive value for
+ // floating-point types, and `lowest()` doesn't satisfy the identity
+ // property for -inf inputs, so use -infinity for those types.
+ if constexpr (std::is_floating_point_v<Value> || std::is_same_v<Float16,
Value>) {
+ return -std::numeric_limits<Value>::infinity();
+ } else {
+ return std::numeric_limits<Value>::lowest();
+ }
}
};
@@ -767,7 +774,12 @@ template <>
struct Identity<Min> {
template <typename Value>
static constexpr Value value() {
- return std::numeric_limits<Value>::max();
+ // Mirror of Identity<Max>: use +infinity for floating-point types.
+ if constexpr (std::is_floating_point_v<Value> || std::is_same_v<Float16,
Value>) {
+ return std::numeric_limits<Value>::infinity();
+ } else {
+ return std::numeric_limits<Value>::max();
+ }
}
};
diff --git a/cpp/src/arrow/compute/kernels/vector_cumulative_ops_test.cc
b/cpp/src/arrow/compute/kernels/vector_cumulative_ops_test.cc
index 53c28032b8..6880b7c272 100644
--- a/cpp/src/arrow/compute/kernels/vector_cumulative_ops_test.cc
+++ b/cpp/src/arrow/compute/kernels/vector_cumulative_ops_test.cc
@@ -949,5 +949,34 @@ TEST(TestCumulative, NaN) {
CheckVectorUnary("cumulative_mean", ArrayFromJSON(float64(), "[5, 4, NaN, 2,
1]"),
ArrayFromJSON(float64(), "[5, 4.5, NaN, NaN, NaN]"));
}
+
+TEST(TestCumulative, NegativeValues) {
+ // GH-51194: the default start for cumulative_max was initialized with
+ // std::numeric_limits<T>::min(), which is the smallest positive value for
+ // floating-point types, so non-positive values never replaced the start
+ CumulativeOptions options;
+ for (auto ty : SignedIntTypes()) {
+ CheckVectorUnary("cumulative_max", ArrayFromJSON(ty, "[-2, -1, -3]"),
+ ArrayFromJSON(ty, "[-2, -1, -1]"), &options);
+ CheckVectorUnary("cumulative_min", ArrayFromJSON(ty, "[-2, -1, -3]"),
+ ArrayFromJSON(ty, "[-2, -2, -3]"), &options);
+ }
+
+ for (auto ty : FloatingPointTypes()) {
+ CheckVectorUnary("cumulative_max", ArrayFromJSON(ty, "[-2.5, 2.5]"),
+ ArrayFromJSON(ty, "[-2.5, 2.5]"), &options);
+ CheckVectorUnary("cumulative_min", ArrayFromJSON(ty, "[-2.5, 2.5]"),
+ ArrayFromJSON(ty, "[-2.5, -2.5]"), &options);
+ CheckVectorUnary("cumulative_max", ArrayFromJSON(ty, "[-2.5, -1.5, -3.5,
-0.5]"),
+ ArrayFromJSON(ty, "[-2.5, -1.5, -1.5, -0.5]"), &options);
+
+ // The default start must compare lower (higher for min) than every value
+ // of the type, including infinities
+ CheckVectorUnary("cumulative_max", ArrayFromJSON(ty, "[-Inf, -2.5]"),
+ ArrayFromJSON(ty, "[-Inf, -2.5]"), &options);
+ CheckVectorUnary("cumulative_min", ArrayFromJSON(ty, "[Inf, 2.5]"),
+ ArrayFromJSON(ty, "[Inf, 2.5]"), &options);
+ }
+}
} // namespace compute
} // namespace arrow
diff --git a/python/pyarrow/tests/test_compute.py
b/python/pyarrow/tests/test_compute.py
index d350c81157..8b2ad2b333 100644
--- a/python/pyarrow/tests/test_compute.py
+++ b/python/pyarrow/tests/test_compute.py
@@ -3625,6 +3625,28 @@ def test_cumulative_max(start, skip_nulls):
pc.cumulative_max([1, 2, 3], start=strt)
[email protected]
+def test_cumulative_max_min_negative_default_start():
+ # GH-51194: the implicit start for cumulative_max was initialized with
+ # std::numeric_limits<T>::min(), which is the smallest positive value for
+ # floating-point types, so non-positive values never replaced the start
+ values = [-2.5, 2.5]
+ arr = pa.array(values, type=pa.float64())
+ assert pc.cumulative_max(arr).to_pylist() == [-2.5, 2.5]
+ assert pc.cumulative_min(arr).to_pylist() == [-2.5, -2.5]
+
+ arr = pa.chunked_array([[-2.5, -1.5], [-3.5, -0.5]])
+ assert pc.cumulative_max(arr).to_pylist() == [-2.5, -1.5, -1.5, -0.5]
+ assert pc.cumulative_min(arr).to_pylist() == [-2.5, -2.5, -3.5, -3.5]
+
+ # The default start must compare lower (higher for min) than every value
+ # of the type, including infinities
+ arr = pa.array([-np.inf, -2.5], type=pa.float64())
+ assert pc.cumulative_max(arr).to_pylist() == [-np.inf, -2.5]
+ arr = pa.array([np.inf, 2.5], type=pa.float64())
+ assert pc.cumulative_min(arr).to_pylist() == [np.inf, 2.5]
+
+
@pytest.mark.numpy
@pytest.mark.parametrize('start', (0.5, 3.5, 6.5))
@pytest.mark.parametrize('skip_nulls', (True, False))
