ArrayPartition copyto! dimension mismatch with Array{T,N} where N>1

[nantonel]

Using Julia 1.1.1:

julia> using RecursiveArrayTools

julia> x = ArrayPartition(rand(10,10),rand(10));

julia> y = zero(x);

julia> copyto!(y,x)
ERROR: DimensionMismatch("array could not be broadcast to match destination")
Stacktrace:
 [1] copyto!(::ArrayPartition{Float64,Tuple{Array{Float64,2},Array{Float64,1}}}, ::ArrayPartition{Float64,Tuple{Array{Float64,2},Array{Float64,1}}}) at ./broadcast.jl:456
 [2] top-level scope at none:0

I've notice there's no error when ArrayPartition is constructed using only vectors:

julia> x = ArrayPartition(rand(2), rand(10)+im.*randn(10));

julia> y = zero(x);

julia> copyto!(y,x)

Seems to be related to #67, but I've opened a new issue because the error I get is different.

[ChrisRackauckas]

yeah I think this is erroring for a different reason too. Not exactly trivial to fix.

[lostella]

What does zero(x) produce? Another ArrayPartition with the same internal layout? In that case, a (local, temporary) fix for this could be that of implementing copyto! by looping through the partition pieces

[ChrisRackauckas]

Actually, this just looks like a missing method.

julia> using RecursiveArrayTools

julia> x = ArrayPartition(rand(10,10),rand(10));

julia> y = zero(x)
([0.0 0.0 … 0.0 0.0; 0.0 0.0 … 0.0 0.0; … ; 0.0 0.0 … 0.0 0.0; 0.0 0.0 … 0.0 0.0], [0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0])

julia> typeof(zero(x))
ArrayPartition{Float64,Tuple{Array{Float64,2},Array{Float64,1}}}

julia> y .= x
([0.365442 0.808784 … 0.253202 0.0657664; 0.244857 0.73212 … 0.978118 0.921098; … ; 0.493738 0.0338712 … 0.177619 0.834252; 0.292743 0.76287 … 0.0713162 0.213752], [0.407017, 0.297832, 0.332515, 0.0152949, 0.855382, 0.185179, 0.896822, 0.136354, 0.0788127, 0.842354])

[lostella]

So copyto! in this case just falls back to something that doesn’t work?

[nantonel]

I've changed the implementation of copyto! from

function Base.copyto!(A::ArrayPartition,src::ArrayPartition)
    @assert length(src) == length(A)
    cur = 1
    @inbounds for i in 1:length(A.x)
        A.x[i] .= @view(src[cur:(cur+length(A.x[i])-1)])
        cur += length(A.x[i])
    end
    A
end

to

function Base.copyto!(A::ArrayPartition,src::ArrayPartition)
    @assert length(src) == length(A)
    cnt = 0
    for i in eachindex(A.x)
      x = A.x[i]
      for k in eachindex(x)
        cnt += 1
        x[k] = src[cnt]
      end
    end
    A
end

test are passing and I could make a PR if you want, but I wonder if it would change any other feature you wanted in the package...

[ChrisRackauckas]

It should try to make use of broadcast so that way things that cannot be indexed are still usable.

function Base.copyto!(A::ArrayPartition,src::ArrayPartition)
    @assert length(src) == length(A)
    cnt = 0
    for i in eachindex(A.x)
      x = vec(A.x[i])
      x .= vec(src.x[i])
    end
    A
end

might be a good solution

[nantonel]

It should try to make use of broadcast so that way things that cannot be indexed are still usable.

function Base.copyto!(A::ArrayPartition,src::ArrayPartition)
    @assert length(src) == length(A)
    cnt = 0
    for i in eachindex(A.x)
      x = vec(A.x[i])
      x .= vec(src.x[i])
    end
    A
end

might be a good solution

the problem here is that if you have differently partitioned array this would fail, e.g.:

  x = ArrayPartition( randn.((3,2),(2,))) ) 
  y = ArrayPartition( zeros.((3,),(3,)(2,))) )

[ChrisRackauckas]

Shouldn't it fail? Usually broadcast respects shape. For an ArrayPartition the definition of "shape" is a bit more complex, but I think it might make sense

(and any fallback to linear index will be slow BTW)

[nantonel]

Ah ok! I thought you wanted to allow different ArrayPartitions to be partitioned differently, but if this is not the case then:

Base.copyto!(x::ArrayPartition, y::ArrayPartition) = x .= y

should fix the problem.

[nantonel]

In the current version of copyto! you allow for this. E.g.:

julia> x = ArrayPartition(randn(10),randn(5));

julia> y = ArrayPartition(randn(5),randn(10));

julia> copyto!(y,x);

but x.=y fails:

julia> x .= y
ERROR: DimensionMismatch("destination axes (Base.OneTo(10),) are not compatible with source axes (Base.OneTo(5),)")
Stacktrace:
 [1] throwdm(::Tuple{Base.OneTo{Int64}}, ::Tuple{Base.OneTo{Int64}}) at ./broadcast.jl:919
 [2] materialize!(::ArrayPartition{Float64,Tuple{Array{Float64,1},Array{Float64,1}}}, ::Base.Broadcast.Broadcasted{RecursiveArrayTools.ArrayPartitionStyle{Base.Broadcast.DefaultArrayStyle{1}},Nothing,typeof(identity),Tuple{ArrayPartition{Float64,Tuple{Array{Float64,1},Array{Float64,1}}}}}) at ./broadcast.jl:833
 [3] top-level scope at none:0

[ChrisRackauckas]

Yeah, I guess we should for copyto!, since copyto! respects different rules than broadcast:

julia> x = rand(5,5)
5×5 Array{Float64,2}:
 0.867039  0.365914   0.598226   0.369013  0.220135
 0.348353  0.0722581  0.849877   0.976608  0.343364
 0.846249  0.546752   0.0314149  0.4509    0.772786
 0.995492  0.114161   0.27764    0.309973  0.464159
 0.442844  0.212946   0.33307    0.277821  0.539598

julia> y = rand(25)
25-element Array{Float64,1}:
 0.3329644471407822
 0.8231218682093961
 0.6235839920054078
 0.2918758394607628
 0.2544781136404477
 0.7390723274932731
 0.8188652474032347
 ⋮
 0.8195172431831688
 0.4566558733111039
 0.6675397640264278
 0.5750037897953051
 0.6429885255692565
 0.3011409769083937
 0.019515019670683476

julia> copyto!(y,x)
25-element Array{Float64,1}:
 0.8670388124493265
 0.34835300650085443
 0.8462485662071375
 0.9954917077217105
 0.44284404527858423
 0.3659141363589964
 0.07225808343685292
 ⋮
 0.3099731205870988
 0.2778213868751336
 0.220134678770745
 0.3433636136264684
 0.7727859924456044
 0.46415887693736235
 0.5395977408796513

[ChrisRackauckas]

I guess let's go with

function Base.copyto!(A::ArrayPartition,src::ArrayPartition)
    @assert length(src) == length(A)
    cnt = 0
    for i in eachindex(A.x)
      x = A.x[i]
      for k in eachindex(x)
        cnt += 1
        x[k] = src[cnt]
      end
    end
    A
end

submit a PR and I'll make a patch immediately.