Skip to content

Commit d9fce2a

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

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
@@ -634,11 +634,6 @@ function mapreduce_impl(f, op::Union{typeof(max), typeof(min)},
634634
start = first + 1
635635
simdstop = start + chunk_len - 4
636636
while simdstop <= last - 3
637-
# short circuit in case of NaN or missing
638-
(v1 == v1) === true || return v1
639-
(v2 == v2) === true || return v2
640-
(v3 == v3) === true || return v3
641-
(v4 == v4) === true || return v4
642637
@inbounds for i in start:4:simdstop
643638
v1 = _fast(op, v1, f(A[i+0]))
644639
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)