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

Copying a 1-element "empty" gcmvector errors #103

Open
mbauman opened this issue Aug 8, 2023 · 2 comments
Open

Copying a 1-element "empty" gcmvector errors #103

mbauman opened this issue Aug 8, 2023 · 2 comments

Comments

@mbauman
Copy link

mbauman commented Aug 8, 2023

This is a bit of a silly thing to ask, but we're (again) seeing errors over in JuliaLang/julia#26237. This time it seems to be stemming from

XS[findall(XS.<Xmin)]=XS[findall(XS.<Xmin)].+360;
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 JuliaLang/julia#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:

function Base.similar(bc::Broadcast.Broadcasted{Broadcast.ArrayStyle{gcmvector}}, ::Type{ElType}) where ElType
# Scan the inputs for the gcmarray:
A = find_gcmvector(bc)
# Create the gcmvector output:
return gcmvector{ElType,ndims(A)}(A.grid,similar(A.f),A.fSize,A.fIndex)

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.)

@gaelforget
Copy link
Member

Thanks for bringing this up.

As this seemed related to #46 , I thought that maybe adding copy in the similar methods would help here too.

Could you please try with #105 and tell me if that works?

@gaelforget
Copy link
Member

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

Does the issue persist with MeshArrays v0.3.0 ?

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

No branches or pull requests

2 participants