Fix mapreduce_impl() for 1-element range (#19325)

* fix mapreduce_impl() for 1-element range

* mapreduce_impl() type stability tests

* wrap long lines

* mapreduce_impl(): add @inbounds

ifirst:ilast should be a valid A indices range (checked by the caller)

* reduce code duplication in _mapreduce()

- let mapreduce_impl() handle all but empty collection cases
- don't test mapreduce_impl() directly
- extend mapreduce() tests with empty/long collections

* mapreduce_impl(f, min/max): don't skip NaNs

- update mapreduce_impl(f, min/max) to 0.6 behaviour of treating NaNs,
  also stop scanning the array once NaN is detected
- add NaN minimum/maximum() tests for 2-element and long arrays

* mapreduce(f, op): don't apply @inbounds for `f`

* clarify how mapreduce_impl() should be used

base/reduce.jl

@@ -167,15 +167,25 @@ foldr(op, itr) = mapfoldr(identity, op, itr)
 
 ## reduce & mapreduce
 
+# `mapreduce_impl()` is called by `mapreduce()` (via `_mapreduce()`, when `A`
+# supports linear indexing) and does actual calculations (for `A[ifirst:ilast]` subset).
+# For efficiency, no parameter validity checks are done, it's the caller responsibility.
+# `ifirst:ilast` range is assumed to be a valid non-empty subset of `A` indices.
+
+# This is a generic implementation of `mapreduce_impl()`,
+# certain `op` (e.g. `min` and `max`) may have their own specialized versions.
 function mapreduce_impl(f, op, A::AbstractArray, ifirst::Integer, ilast::Integer, blksize::Int=pairwise_blocksize(f, op))
-    if ifirst + blksize > ilast
+    if ifirst == ilast
+        @inbounds a1 = A[ifirst]
+        return r_promote(op, f(a1))
+    elseif ifirst + blksize > ilast
         # sequential portion
-        fx1 = r_promote(op, f(A[ifirst]))
-        fx2 = r_promote(op, f(A[ifirst + 1]))
-        v = op(fx1, fx2)
+        @inbounds a1 = A[ifirst]
+        @inbounds a2 = A[ifirst+1]
+        v = op(r_promote(op, f(a1)), r_promote(op, f(a2)))
         @simd for i = ifirst + 2 : ilast
-            @inbounds Ai = A[i]
-            v = op(v, f(Ai))
+            @inbounds ai = A[i]
+            v = op(v, f(ai))
         end
         return v
     else
@@ -250,26 +260,8 @@ _mapreduce(f, op, A::AbstractArray) = _mapreduce(f, op, IndexStyle(A), A)
 
 function _mapreduce{T}(f, op, ::IndexLinear, A::AbstractArray{T})
     inds = linearindices(A)
-    n = length(inds)
-    @inbounds begin
-        if n == 0
-            return mr_empty(f, op, T)
-        elseif n == 1
-            return r_promote(op, f(A[inds[1]]))
-        elseif n < 16
-            fx1 = r_promote(op, f(A[inds[1]]))
-            fx2 = r_promote(op, f(A[inds[2]]))
-            s = op(fx1, fx2)
-            i = inds[2]
-            while i < last(inds)
-                Ai = A[i+=1]
-                s = op(s, f(Ai))
-            end
-            return s
-        else
-            return mapreduce_impl(f, op, A, first(inds), last(inds))
-        end
-    end
+    length(inds) > 0 ? mapreduce_impl(f, op, A, first(inds), last(inds)) :
+                       mr_empty(f, op, T)
 end
 
 _mapreduce(f, op, ::IndexCartesian, A::AbstractArray) = mapfoldl(f, op, A)
@@ -415,17 +407,12 @@ function mapreduce_impl(f, op::Union{typeof(scalarmax),
                                      typeof(min)},
                         A::AbstractArray, first::Int, last::Int)
     # locate the first non NaN number
-    v = f(A[first])
+    @inbounds a1 = A[first]
+    v = f(a1)
     i = first + 1
-    while v != v && i <= last
-        @inbounds Ai = A[i]
-        v = f(Ai)
-        i += 1
-    end
-    while i <= last
-        @inbounds Ai = A[i]
-        x = f(Ai)
-        v = op(v, x)
+    while (v == v) && (i <= last)
+        @inbounds ai = A[i]
+        v = op(v, f(ai))
         i += 1
     end
     v

test/reduce.jl

@@ -26,17 +26,40 @@
 @test Base.mapfoldr(abs2, -, 2:5) == -14
 @test Base.mapfoldr(abs2, -, 10, 2:5) == -4
 
-# reduce & mapreduce
+# reduce
 @test reduce(+, Int64[]) === Int64(0) # In reference to issue #20144 (PR #20160)
 @test reduce(+, Int16[]) === Int32(0)
 @test reduce((x,y)->"($x+$y)", 9:11) == "((9+10)+11)"
 @test reduce(max, [8 6 7 5 3 0 9]) == 9
 @test reduce(+, 1000, 1:5) == (1000 + 1 + 2 + 3 + 4 + 5)
 @test reduce(+,1) == 1
 
+# mapreduce
 @test mapreduce(-, +, [-10 -9 -3]) == ((10 + 9) + 3)
 @test mapreduce((x)->x[1:3], (x,y)->"($x+$y)", ["abcd", "efgh", "01234"]) == "((abc+efg)+012)"
 
+# mapreduce() for 1- 2- and n-sized blocks (PR #19325)
+@test mapreduce(-, +, [-10]) == 10
+@test mapreduce(abs2, +, [-9, -3]) == 81 + 9
+@test mapreduce(-, +, [-9, -3, -4, 8, -2]) == (9 + 3 + 4 - 8 + 2)
+@test mapreduce(-, +, collect(linspace(1.0, 10000.0, 10000))) == -50005000.0
+# mapreduce() type stability
+@test typeof(mapreduce(*, +, Int8[10])) ===
+      typeof(mapreduce(*, +, Int8[10, 11])) ===
+      typeof(mapreduce(*, +, Int8[10, 11, 12, 13]))
+@test typeof(mapreduce(*, +, Float32[10.0])) ===
+      typeof(mapreduce(*, +, Float32[10, 11])) ===
+      typeof(mapreduce(*, +, Float32[10, 11, 12, 13]))
+# mapreduce() type stability when f supports empty collections
+@test typeof(mapreduce(abs, +, Int8[])) ===
+      typeof(mapreduce(abs, +, Int8[10])) ===
+      typeof(mapreduce(abs, +, Int8[10, 11])) ===
+      typeof(mapreduce(abs, +, Int8[10, 11, 12, 13]))
+@test typeof(mapreduce(abs, +, Float32[])) ===
+      typeof(mapreduce(abs, +, Float32[10])) ===
+      typeof(mapreduce(abs, +, Float32[10, 11])) ===
+      typeof(mapreduce(abs, +, Float32[10, 11, 12, 13]))
+
 # sum
 
 @test sum(Int8[]) === Int32(0)
@@ -146,6 +169,10 @@ prod2(itr) = invoke(prod, Tuple{Any}, itr)
 @test isnan(minimum([NaN]))
 @test isequal(extrema([NaN]), (NaN, NaN))
 
+@test isnan(maximum([NaN, 2.]))
+@test isnan(minimum([NaN, 2.]))
+@test isequal(extrema([NaN, 2.]), (NaN,NaN))
+
 @test isnan(maximum([NaN, 2., 3.]))
 @test isnan(minimum([NaN, 2., 3.]))
 @test isequal(extrema([NaN, 2., 3.]), (NaN,NaN))
@@ -154,6 +181,14 @@ prod2(itr) = invoke(prod, Tuple{Any}, itr)
 @test isnan(minimum([4., 3., NaN, 5., 2.]))
 @test isequal(extrema([4., 3., NaN, 5., 2.]), (NaN,NaN))
 
+ # test long arrays
+@test isnan(maximum([NaN; 1.:10000.]))
+@test isnan(maximum([1.:10000.; NaN]))
+@test isnan(minimum([NaN; 1.:10000.]))
+@test isnan(minimum([1.:10000.; NaN]))
+@test isequal(extrema([1.:10000.; NaN]), (NaN,NaN))
+@test isequal(extrema([NaN; 1.:10000.]), (NaN,NaN))
+
 @test maximum(abs2, 3:7) == 49
 @test minimum(abs2, 3:7) == 9