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.

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

Failure in mixed indexing with multidimensional logicals #38192

Closed
timholy opened this issue Oct 27, 2020 · 5 comments · Fixed by #38193
Closed

Failure in mixed indexing with multidimensional logicals #38192

timholy opened this issue Oct 27, 2020 · 5 comments · Fixed by #38193

Comments

@timholy
Copy link
Member

timholy commented Oct 27, 2020

Inspired by https://stackoverflow.com/questions/64549136/i-want-to-do-logical-masking-on-an-image-in-julia

julia> img = cat([1 2; 3 4], [1 5; 6 7]; dims=3)
2×2×2 Array{Int64, 3}:
[:, :, 1] =
 1  2
 3  4

[:, :, 2] =
 1  5
 6  7

julia> mask = img[:,:,1] .== img[:,:,2]
2×2 BitMatrix:
 1  0
 0  0

julia> img[mask,2] .= 0
ERROR: BoundsError: attempt to access 2×2×2 Array{Int64, 3} at index [Bool[1 0; 0 0], 2]
Stacktrace:
 [1] throw_boundserror(A::Array{Int64, 3}, I::Tuple{Base.LogicalIndex{CartesianIndex{2}, BitMatrix}, Int64})
   @ Base ./abstractarray.jl:601
 [2] checkbounds
   @ ./abstractarray.jl:566 [inlined]
 [3] view
   @ ./subarray.jl:158 [inlined]
 [4] maybeview
   @ ./views.jl:133 [inlined]
 [5] dotview(::Array{Int64, 3}, ::BitMatrix, ::Int64)
   @ Base.Broadcast ./broadcast.jl:1160
 [6] top-level scope
   @ REPL[3]:1
@mcabbott
Copy link
Contributor

A possible downside is that this will make #35681 easier to hit: img[mask, end] won't work correctly (with say img = rand(1:5, (3,5,2)) so the sizes differ).

@timholy
Copy link
Member Author

timholy commented Oct 27, 2020

#35681 (comment)

@lxvm
Copy link
Contributor

lxvm commented Jan 9, 2024

Here is a slight modification of the MWE that also errors

julia> img[2,mask] .= 0
ERROR: BoundsError: attempt to access 2×2×2 Array{Int64, 3} at index [2, 2×2 BitMatrix]
Stacktrace:
 [1] throw_boundserror(A::Array{Int64, 3}, I::Tuple{Int64, Base.LogicalIndex{Int64, BitMatrix}})
   @ Base ./abstractarray.jl:734
 [2] checkbounds
   @ Base ./abstractarray.jl:699 [inlined]
 [3] view
   @ Base ./subarray.jl:179 [inlined]
 [4] maybeview
   @ Base ./views.jl:148 [inlined]
 [5] dotview(::Array{Int64, 3}, ::Int64, ::BitMatrix)
   @ Base.Broadcast ./broadcast.jl:1244
 [6] top-level scope
   @ REPL[65]:1

I'll try opening a pr to fix it

@mbauman
Copy link
Member

mbauman commented Jan 9, 2024

@lxvm this should be fixed by #45869:

julia> img = rand(Int, 2,2,2);

julia> img[2, trues(2,2)] .= 0
4-element view(::Array{Int64, 3}, 2, CartesianIndex{2}[CartesianIndex(1, 1), CartesianIndex(2, 1), CartesianIndex(1, 2), CartesianIndex(2, 2)]) with eltype Int64:
 0
 0
 0
 0

julia> img
2×2×2 Array{Int64, 3}:
[:, :, 1] =
 615342675984579180  -924825086713411107
                  0                    0

[:, :, 2] =
 -3776513274107704031  2722564769933431785
                    0                    0

@lxvm
Copy link
Contributor

lxvm commented Jan 9, 2024

@mbauman Thanks! It looks like getindex is also fixed. I was concerned that the logic of _maybe_linear_logical_index in base/multidimensional.jl is incorrect. Does the following work for you?

julia> ones(2,2,2)[:,iseven.(ones(2,2))]
2×0 Matrix{Float64}

Edit: I rebuilt Julia and it is indeed fixed since _maybe_linear_logical_index is no longer called for trailing indices

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants