Skip to content

Commit

Permalink
Fix findmin/findmax to return cartesian indices for BitMatrix
Browse files Browse the repository at this point in the history
For consistency with other AbstractArray types.
The call to keys is optimized out by the compiler for vectors thanks to
@inbounds.
  • Loading branch information
nalimilan committed Nov 28, 2018
1 parent 40c2d89 commit a9161d9
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
11 changes: 5 additions & 6 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,6 @@ function findprev(testf::Function, B::BitArray, start::Integer)
end
#findlast(testf::Function, B::BitArray) = findprev(testf, B, 1) ## defined in array.jl


function findmax(a::BitArray)
isempty(a) && throw(ArgumentError("BitArray must be non-empty"))
m, mi = false, 1
Expand All @@ -1519,9 +1518,9 @@ function findmax(a::BitArray)
for i = 1:length(ac)
@inbounds k = trailing_zeros(ac[i])
ti += k
k == 64 || return (true, ti)
k == 64 || return (true, @inbounds keys(a)[ti])
end
return m, mi
return m, @inbounds keys(a)[mi]
end

function findmin(a::BitArray)
Expand All @@ -1532,13 +1531,13 @@ function findmin(a::BitArray)
for i = 1:length(ac)-1
@inbounds k = trailing_ones(ac[i])
ti += k
k == 64 || return (false, ti)
k == 64 || return (false, @inbounds keys(a)[ti])
end
l = Base._mod64(length(a)-1) + 1
@inbounds k = trailing_ones(ac[end] & Base._msk_end(l))
ti += k
k == l || return (false, ti)
return m, mi
k == l || return (false, @inbounds keys(a)[ti])
return (m, @inbounds keys(a)[mi])
end

# findall helper functions
Expand Down
5 changes: 5 additions & 0 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,11 @@ end
@test isnan(findmax([NaN, NaN, 0.0/0.0])[1])
@test findmax([NaN, NaN, 0.0/0.0])[2] == 1

# Check that cartesian indices are returned for matrices
@test argmax([10 12; 9 11]) === CartesianIndex(1, 2)
@test argmin([10 12; 9 11]) === CartesianIndex(2, 1)
@test findmax([10 12; 9 11]) === (12, CartesianIndex(1, 2))
@test findmin([10 12; 9 11]) === (9, CartesianIndex(2, 1))
end

@testset "permutedims" begin
Expand Down
2 changes: 2 additions & 0 deletions test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1523,6 +1523,8 @@ timesofar("linalg")
for b1 in [falses(v1), trues(v1),
BitArray([1,0,1,1,0]),
BitArray([0,0,1,1,0]),
BitArray([1 0; 1 1]),
BitArray([0 0; 1 1]),
bitrand(v1)]
@check_bit_operation findmin(b1)
@check_bit_operation findmax(b1)
Expand Down

0 comments on commit a9161d9

Please sign in to comment.