findmin(A; dims) ignores first value if index is 0

[cmcaine]

Index can be zero if you're using OffsetArrays.jl or similar. Also
affects findmax, argmin, argmax.

If dims is omitted or : then you don't get this bug :)

Found while working on #35316 and I'll fix it there edit: I might fix it later, PR already too big.

Reproducible example:

julia> using OffsetArrays

julia> ov = OffsetVector([-1, 1], 0:1)
2-element OffsetArray(::Vector{Int64}, 0:1) with eltype Int64 with indices 0:1:
 -1
  1

julia> findmin(ov, dims=1) # Should be ([-1], [0])
([1], [1])

[FedericoStra]

The problem appears to be that, given A = ov and ri = Base.reduced_indices0(A, 1) as defined in _findmin, we go in the else branch and findminmax! gets called with arguments

julia> Rval = fill!(similar(A, ri), first(A))
1-element OffsetArray(::Vector{Int64}, 0:0) with eltype Int64 with indices 0:0:
 -1

julia> Rind = zeros(eltype(keys(A)), ri)
1-element OffsetArray(::Vector{Int64}, 0:0) with eltype Int64 with indices 0:0:
 0

but, as the comment above findminmax! says, "The initial values of Rval are not used if the corresponding indices in Rind are 0".

Ultimately, the problem is that 0 is a valid sentinel value for an index when working with 1-based arrays, but not when working with OffsetArrays.

[cmcaine]

Yeah, I see why it happens. I guess someone should decide if that behaviour (ignore if Rind is zero(eltype(keys(A)))) is something that should be retained. I don't think it can be retained if zero is a valid index into the array, tho maybe I can find a clever way around it.

…

On Thu, 3 Dec 2020, 17:18 Federico Stra, ***@***.***> wrote: The problem appears to be that, given A = ov and ri = Base.reduced_indices0(A, 1) as defined in _findmin <https://github.com/JuliaLang/julia/blob/master/base/reducedim.jl#L991-L1002>, findminmax! <https://github.com/JuliaLang/julia/blob/master/base/reducedim.jl#L907-L955> gets called with arguments julia> Rval = fill!(similar(A, ri), first(A))1-element OffsetArray(::Vector{Int64}, 0:0) with eltype Int64 with indices 0:0: -1 julia> Rind = zeros(eltype(keys(A)), ri)1-element OffsetArray(::Vector{Int64}, 0:0) with eltype Int64 with indices 0:0: 0 but, as the comment above findminmax! <https://github.com/JuliaLang/julia/blob/master/base/reducedim.jl#L905> says, "The initial values of Rval are not used if the corresponding indices in Rind are 0". — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#38660 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNZA6OYFLALKM4TAUWMXU3SS7B7BANCNFSM4UKV6L6Q> .

[FedericoStra]

maybe I can find a clever way around it

Making Rind::Array{Union{Nothing,Int},N} seems the most obvious and laziest thing to do in order to change the least amount of code, but almost surely there are better ways to work around it in a more efficient manner.

[N5N3]

Maybe we can replace zero(eltype(keys(A))) with prevind(A, first(keys(A))) to indicate the state of initialization.