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.

Sign up for GitHub

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

make findmin/findmax behavior match min/max (fix #23209) #23227

Merged
merged 15 commits into from
Aug 31, 2017
+428 −95
Merged

make findmin/findmax behavior match min/max (fix #23209) #23227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b4113e7
findmin findmax in array.jl and reducedim.jl ( fix #23209)
KlausC Aug 11, 2017
172c9ef
fixed issues with Inf, BigInt, and String
KlausC Aug 12, 2017
915d719
fixed reducedim versions of minimum/maximum
KlausC Aug 13, 2017
519184b
removed definition of zero2
KlausC Aug 16, 2017
faf4927
cover sparse matrices #23209
KlausC Aug 16, 2017
2ad37fe
gitspaces...
KlausC Aug 16, 2017
829f0c5
off-by-one bound-check bug in _findz fixed
KlausC Aug 17, 2017
57329bf
Merge remote-tracking branch 'upstream/master' into fix#23209
KlausC Aug 17, 2017
37d8bf3
avoid breaking of test "distributed"
KlausC Aug 17, 2017
6c64a20
offsetarray compatibility - consistend empty case
KlausC Aug 18, 2017
d6cd550
offsetarray compatibility - consistent empty case
KlausC Aug 18, 2017
78bbd14
Merge branch 'fix#23209' of https://github.com/KlausC/julia into fix#…
KlausC Aug 18, 2017
6532ba8
use isequal in tests
KlausC Aug 22, 2017
be0f449
Remove unrelated distributed change
mbauman Aug 24, 2017
a52c2aa
Add NEWS.md
mbauman Aug 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ This section lists changes that do not have deprecation warnings.
`Bidiagonal{T,V<:AbstractVector{T}}` and `SymTridiagonal{T,V<:AbstractVector{T}}`
respectively ([#22718], [#22925], [#23035]).

* When called with an argument that contains `NaN` elements, `findmin` and `findmax` now return the
first `NaN` found and its corresponding index. Previously, `NaN` elements were ignored.
The new behavior matches that of `min`, `max`, `minimum`, and `maximum`.

* `isapprox(x,y)` now tests `norm(x-y) <= max(atol, rtol*max(norm(x), norm(y)))`
rather than `norm(x-y) <= atol + ...`, and `rtol` defaults to zero
if an `atol > 0` is specified ([#22742]).
Expand Down
36 changes: 16 additions & 20 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ function fill!(a::Array{T}, x) where T<:Union{Integer,AbstractFloat}
return a
end


"""
fill(x, dims)

Expand Down Expand Up @@ -985,7 +984,6 @@ function _prepend!(a, ::IteratorSize, iter)
a
end


"""
resize!(a::Vector, n::Integer) -> Vector

Expand Down Expand Up @@ -1504,7 +1502,6 @@ function reverse!(v::AbstractVector, s=first(linearindices(v)), n=last(linearind
return v
end


# concatenations of homogeneous combinations of vectors, horizontal and vertical

vcat() = Array{Any,1}(0)
Expand Down Expand Up @@ -1549,7 +1546,6 @@ end

cat(n::Integer, x::Integer...) = reshape([x...], (ntuple(x->1, n-1)..., length(x)))


## find ##

"""
Expand Down Expand Up @@ -2019,8 +2015,9 @@ end
findmax(itr) -> (x, index)

Returns the maximum element of the collection `itr` and its index. If there are multiple
maximal elements, then the first one will be returned. `NaN` values are ignored, unless
all elements are `NaN`. Other than the treatment of `NaN`, the result is in line with `max`.
maximal elements, then the first one will be returned.
If any data element is `NaN`, this element is returned.
The result is in line with `max`.

The collection must not be empty.

Expand All @@ -2033,7 +2030,7 @@ julia> findmax([1,7,7,6])
(7, 2)

julia> findmax([1,7,7,NaN])
(7.0, 2)
(NaN, 4)
```
"""
function findmax(a)
Expand All @@ -2044,10 +2041,10 @@ function findmax(a)
mi = i = 1
m, s = next(a, s)
while !done(a, s)
m != m && break
ai, s = next(a, s)
i += 1
ai != ai && continue # assume x != x => x is a NaN
if m != m || isless(m, ai)
if ai != ai || isless(m, ai)
Copy link
Member

Choose a reason for hiding this comment

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

In the ai != ai case this can short-circuit, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if ai != ai I expect isless is not called. That shall cover the NaN- case. This can occur at most once, since the next loop circle is broken by the m != m condition few lines before.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes, I didn't notice the m != m && break check previously.

m = ai
mi = i
end
Expand All @@ -2059,8 +2056,9 @@ end
findmin(itr) -> (x, index)

Returns the minimum element of the collection `itr` and its index. If there are multiple
minimal elements, then the first one will be returned. `NaN` values are ignored, unless
all elements are `NaN`. Other than the treatment of `NaN`, the result is in line with `min`.
minimal elements, then the first one will be returned.
If any data element is `NaN`, this element is returned.
The result is in line with `min`.

The collection must not be empty.

Expand All @@ -2073,7 +2071,7 @@ julia> findmin([7,1,1,6])
(1, 2)

julia> findmin([7,1,1,NaN])
(1.0, 2)
(NaN, 4)
```
"""
function findmin(a)
Expand All @@ -2084,10 +2082,10 @@ function findmin(a)
mi = i = 1
m, s = next(a, s)
while !done(a, s)
m != m && break
ai, s = next(a, s)
i += 1
ai != ai && continue
if m != m || isless(ai, m)
if ai != ai || isless(ai, m)
m = ai
mi = i
end
Expand All @@ -2099,8 +2097,7 @@ end
indmax(itr) -> Integer

Returns the index of the maximum element in a collection. If there are multiple maximal
elements, then the first one will be returned. `NaN` values are ignored, unless all
elements are `NaN`.
elements, then the first one will be returned.

The collection must not be empty.

Expand All @@ -2113,7 +2110,7 @@ julia> indmax([1,7,7,6])
2

julia> indmax([1,7,7,NaN])
2
4
```
"""
indmax(a) = findmax(a)[2]
Expand All @@ -2122,8 +2119,7 @@ indmax(a) = findmax(a)[2]
indmin(itr) -> Integer

Returns the index of the minimum element in a collection. If there are multiple minimal
elements, then the first one will be returned. `NaN` values are ignored, unless all
elements are `NaN`.
elements, then the first one will be returned.

The collection must not be empty.

Expand All @@ -2136,7 +2132,7 @@ julia> indmin([7,1,1,6])
2

julia> indmin([7,1,1,NaN])
2
4
```
"""
indmin(a) = findmin(a)[2]
Expand Down
Loading