Skip to content

Commit 4716e0a

Browse files
committed
remove short circuit for correctness
This optimization gives wrong result, if `NaN` and `missing` both exist. add missing test
1 parent dbbc022 commit 4716e0a

File tree

2 files changed

+3
-5
lines changed

2 files changed

+3
-5
lines changed

base/reduce.jl

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -637,11 +637,6 @@ function mapreduce_impl(f, op::Union{typeof(max), typeof(min)},
637637
start = first + 1
638638
simdstop = start + chunk_len - 4
639639
while simdstop <= last - 3
640-
# short circuit in case of NaN or missing
641-
(v1 == v1) === true || return v1
642-
(v2 == v2) === true || return v2
643-
(v3 == v3) === true || return v3
644-
(v4 == v4) === true || return v4
645640
@inbounds for i in start:4:simdstop
646641
v1 = _fast(op, v1, f(A[i+0]))
647642
v2 = _fast(op, v2, f(A[i+1]))

test/reduce.jl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,9 @@ A = circshift(reshape(1:24,2,3,4), (0,1,1))
395395
@test maximum(x) === minimum(x) === missing
396396
@test extrema(x) === (missing, missing)
397397
end
398+
# inputs containing both missing and NaN
399+
minimum([NaN;zeros(255);missing]) === missing
400+
maximum([NaN;zeros(255);missing]) === missing
398401
end
399402

400403
# findmin, findmax, argmin, argmax

0 commit comments

Comments
 (0)