BlockArrays.jl
BlockArrays.jl copied to clipboard
Potential issue with `findblock[index]` for `BlockUnitRange` with `first != 1`
Perhaps I am misunderstanding how findblock[index] is supposed to work, but this behavior is confusing to me:
julia> using BlockArrays
julia> r = BlockArrays._BlockedUnitRange(2, [4, 6])
2-blocked 5-element BlockedUnitRange{Vector{Int64}}:
2
3
4
─
5
6
julia> findblock(r, 1)
ERROR: BoundsError: attempt to access 2-blocked 5-element BlockedUnitRange{Vector{Int64}} at index [1]
Stacktrace:
[1] findblock(b::BlockedUnitRange{Vector{Int64}}, k::Int64)
@ BlockArrays ~/.julia/packages/BlockArrays/L5yjb/src/blockaxis.jl:299
[2] top-level scope
@ REPL[27]:1
julia> findblock(r, 2)
Block(1)
julia> findblock(r, 3)
Block(1)
julia> findblock(r, 4)
Block(1)
julia> findblock(r, 5)
Block(2)
I expected it to return:
julia> findblock(r, 1)
Block(1)
julia> findblock(r, 2)
Block(1)
julia> findblock(r, 3)
Block(1)
julia> findblock(r, 4)
Block(2)
julia> findblock(r, 5)
Block(2)
i.e. in a call to findblock(r, k), I was interpreting k as an index, and thought findblock would output the block that index is in, but it seems to be interpreting k as a value, and is outputting the block that value is in.
Maybe the giveaway is that find* functions in Base find the index of a value, so the current behavior of findblock is consistent with that convention, and really I was just hoping for a different function with a different functionality.
I think the current behaviour is correct...
Yeah I think you are right, I just found it surprising. I think ultimately what I wanted was a function that implemented findblock(only(axes(r)), k), which I think is a more useful concept/functionality.
It seems like for it to be well defined beyond sorted collections with unique elements (like unit ranges), it should be called findfirstblock, but that is another issue.
Call it findaxesblock? For matrices would also support findaxesblock(A, k, j) returning a Block{2}
findaxesblock seems fine. I'm still not a huge fan of using the naming convention find* for this kind of operation, which is ultimately a conversion from a cartesian index to a block index, which is what inspired my proposal of BlockIndices(a)[k] in #346. Also findaxesblockindex(A, k) gets a bit long, but I suppose so is the interface proposed in #346, where the best I can come up with for the equivalent of findaxesblock(a, k) would be block(BlockIndices(a)[k]), or maybe a slight shorthand block(BlockIndices(a), k) which could skip some of the extra operations needed to get the offset within the block.