JuliaClimate / JuliaClimate/MeshArrays.jl

Copying a 1-element "empty" gcmvector errors

Open
#103 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Julia
Stars
47
Forks
14
Avg merge
3h 4m
Merged PRs (30d)
2

Description

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

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the gcmvector implementation in src/Type_gcmvector.jl, then inspect the GridLoad path in src/Grids.jl and the related test entry in test/runtests.jl. Reproduce the issue with a 1-element gcmvector containing an empty CartesianIndex vector and copy(idxs). Done means copying this value no longer raises the shown conversion error and the relevant tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.