Skip to content

Commit

Permalink
Fix argmin and argmax (#98)
Browse files Browse the repository at this point in the history
* Fix argmin and argmax

* Revert bqd fix

* Add two small tests

* Moar tests to figure out what's wrong

* Fix tests

* Fix comparison order

* last fix

* One comp only
  • Loading branch information
gdalle authored May 10, 2024
1 parent fa840f9 commit c047384
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
9 changes: 5 additions & 4 deletions src/chainedvector.jl
Original file line number Diff line number Diff line change
Expand Up @@ -825,8 +825,9 @@ function findXwithfirst(comp, f, x, y, i)
for A in x.arrays
for y′ in A
y′′ = f(y′)
y = ifelse(comp(y′′, y), y′′, y)
i = ifelse(comp(y′′, y), i′, i)
c = comp(y′′, y) # store this before y changes
y = ifelse(c, y′′, y)
i = ifelse(c, i′, i)
i′ += 1
end
end
Expand All @@ -835,8 +836,8 @@ end

Base.findmax(x::ChainedVector) = findmax(identity, x)
Base.findmin(x::ChainedVector) = findmin(identity, x)
Base.argmax(x::ChainedVector) = findmax(identity, x)[1]
Base.argmin(x::ChainedVector) = findmin(identity, x)[1]
Base.argmax(x::ChainedVector) = findmax(identity, x)[2]
Base.argmin(x::ChainedVector) = findmin(identity, x)[2]
Base.argmax(f::F, x::ChainedVector) where {F} = x[findmax(f, x)[2]]
Base.argmin(f::F, x::ChainedVector) where {F} = x[findmin(f, x)[2]]

Expand Down
11 changes: 11 additions & 0 deletions test/chainedvector.jl
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,17 @@
@test all(==(2), x)
@test length(x) == 5

# https://github.com/JuliaData/SentinelArrays.jl/issues/97
x = ChainedVector([[18, 70, 92, 15, 65], [25, 14, 95, 54, 57]])
@test findmax(x) == (95, 8)
@test findmin(x) == (14, 7)
@test argmax(x) == 8
@test argmin(x) == 7
@test findmax(inv, x) == (inv(14), 7)
@test findmin(inv, x) == (inv(95), 8)
@test argmax(inv, x) == 14
@test argmin(inv, x) == 95

x = ChainedVector(Vector{Float64}[])
@test !any(x -> iseven(x), x)
@test !any(map(x -> iseven(x), x))
Expand Down

0 comments on commit c047384

Please sign in to comment.