MeshArrays.jl icon indicating copy to clipboard operation
MeshArrays.jl copied to clipboard

Copying a 1-element "empty" gcmvector errors

Open mbauman opened this issue 1 year ago • 2 comments

This is a bit of a silly thing to ask, but we're (again) seeing errors over in https://github.com/JuliaLang/julia/pull/26237. This time it seems to be stemming from https://github.com/JuliaClimate/MeshArrays.jl/blob/6f390fdc51485f4dc2a7f0d738a49955c7b856b8/src/Grids.jl#L441 while running the tests which do a simple GridLoad. In this case, none of the XS need to be shifted, so we end up with a:

julia> idxs = findall(XS .< -180)
1-element MeshArrays.gcmvector{CartesianIndex{2}, 1}:
 CartesianIndex{2}[]

Copying this leads to:

julia> copy(idxs)
ERROR: MethodError: Cannot `convert` an object of type Vector{CartesianIndex{2}} to an object of type CartesianIndex{2}
Closest candidates are:
  convert(::Type{T}, ::T) where T at Base.jl:61
Stacktrace:
 [1] setindex!(A::Vector{CartesianIndex{2}}, x::Vector{CartesianIndex{2}}, i1::Int64)
   @ Base ./array.jl:966
 [2] copyto_unaliased!
   @ ./abstractarray.jl:1044 [inlined]
 [3] copyto!(dest::Vector{CartesianIndex{2}}, src::MeshArrays.gcmvector{CartesianIndex{2}, 1})
   @ Base ./abstractarray.jl:1018
 [4] copymutable(a::MeshArrays.gcmvector{CartesianIndex{2}, 1})
   @ Base ./abstractarray.jl:1152
 [5] copy(a::MeshArrays.gcmvector{CartesianIndex{2}, 1})
   @ Base ./abstractarray.jl:1095
 [6] top-level scope
   @ REPL[83]:1

(As an aside, it's interesting that the proposed auto-aliasing detection in https://github.com/JuliaLang/julia/pull/26237 is thinking that expressions like X .+ 180 might alias with the output (when you didn't even write what the output was!); what's happening here is that MeshArrays has defined broadcasting to create output arrays that explicitly share some of the same fields:

https://github.com/JuliaClimate/MeshArrays.jl/blob/6f390fdc51485f4dc2a7f0d738a49955c7b856b8/src/Type_gcmvector.jl#L64-L68

The new PR to automatically do deeper introspection of all fields is seeing these shared-memory components and thinking that it'll cause problems, so it's making these defensive copies.)

mbauman avatar Aug 08 '23 14:08 mbauman

Thanks for bringing this up.

As this seemed related to https://github.com/JuliaClimate/MeshArrays.jl/issues/46 , I thought that maybe adding copy in the similar methods would help here too.

Could you please try with https://github.com/JuliaClimate/MeshArrays.jl/pull/105 and tell me if that works?

gaelforget avatar Aug 16 '23 19:08 gaelforget

I have now merged the fix (?) in #105

Does the issue persist with MeshArrays v0.3.0 ?

gaelforget avatar Aug 31 '23 16:08 gaelforget