Constructors for common base types?

[rafaqz]

Tim Besard just made the point

julia> Flatten.modify(CuArray, view(rand(2,2),:,:), Array)
ERROR: MethodError: no method matching SubArray(::CuArray{Float64,2}, ::Tuple{Base.Slice{Base.OneTo{Int64}},Base.Slice{Base.OneTo{Int64}}}, ::Int64, ::Int64)

And of course

using Setfield
sa = view(zeros(10, 10), :, :)
julia> @set sa.offset1 = 2
ERROR: MethodError: no method matching SubArray(::Array{Float64,2}, ::Tuple{Base.Slice{Base.OneTo{Int64}},Base.Slice{Base.OneTo{Int64}}}, ::Int64, ::Int64)

Should we add some constructors for common base wrapper types like SubArray?

[maleadt]

Some more:

  Expression: Flatten.modify(CustomArray, PermutedDimsArray(mat.arr, (2, 1)), Array) == PermutedDimsArray(mat, (2, 1))
  MethodError: no method matching PermutedDimsArray(::CustomArray{Float64,2})

  Expression: Flatten.modify(CustomArray, Base.LogicalIndex(mat_bools.arr), Array) == Base.LogicalIndex(mat_bools)
  MethodError: no method matching Base.LogicalIndex(::CustomArray{Bool,2}, ::Int64)

  Expression: Flatten.modify(CustomArray, reinterpret(Int64, mat.arr), Array) == reinterpret(Int64, mat)
  MethodError: no method matching Base.ReinterpretArray(::CustomArray{Float64,2}, ::Bool, ::Bool)

  Expression: Flatten.modify(CustomArray, Tridiagonal(dl.arr, d.arr, du.arr), Array) == Tridiagonal(dl, d, du)
  UndefRefError: access to undefined reference

[maleadt]

These are from running the Adapt.jl test suite, https://github.com/JuliaGPU/Adapt.jl/blob/master/test/runtests.jl, with the following test macro:

julia> macro test_adapt(to, src, dst, typ=nothing)
           quote
               @test Flatten.modify(CustomArray, $src, Array) == $dst
               @test typeof(Flatten.modify(CustomArray, $src, Array)) == typeof($dst)
               if $typ !== nothing
                   @test typeof($dst) <: $typ
               end
           end
       end

[jw3126]

As a general rule, I think if a type satisfies the following:

• Its properties are public.
• It is defined in Base
  we should probably support constructorof (and setproperties).
  Not sure if that holds for SubArray though?

[rafaqz]

Yeah its an edge case I guess, but mostly because the constructor is view.

But it's the most widely used, and not defining it means other packages might have to pirate constructorof

[jw3126]

Maybe of interest to the gpu use case: jw3126/Setfield.jl#149

[rafaqz]

Most of that discussion is solved with Flatten.modify

[jw3126]

Yeah its an edge case I guess, but mostly because the constructor is view.

But it's the most widely used, and not defining it means other packages might have to pirate constructorof

Ok, it may be pragmatic to supportSubArray even if it is an edge case.

[rafaqz]

This is kind of complicated in practise, and brings up some questions for field setting in gerneral.

In SubArray, offset1 and stride1 fields are precomputed from parent and indices, or are just zero.

https://github.com/JuliaLang/julia/blob/master/base/subarray.jl#L37

So setting them without changing parent or indices actually makes no sense to do. This means the constructororof
a SubArray could be something like:

struct SubArrayConstructor end
constructorof(::Type{<:SubArray}) = SubArrayConstructor()
(::SubArrayConstructor)(parent, indices, args...) = view(parent, indices)

Which just throws out stride1 and offset1 and recomputes them/uses zero. But it feels a bit weird to let users set offset1 and just return the same object ignoring the change.

But we could clearly document that I guess - base fields that are always precomputed cannot be modified manually.
I would guess quite a few other Base types with free type parameters share this property.

[jw3126]

Yes it is not ideal. But I think we could add the general rule that constructorof is free to ignore "private" fields and this would be a case where that rule applies. I think conceptually

constructorof(obj)(fieldvalues(obj, public=true)...) == obj

and

fieldvalues(subarray, public=true) == (subarray.parent, subarray.indices)

[rafaqz]

Another interesting finding... Tridiagonal has an optional, usually undef field.

I'm not sure how common that is in base, but I hope it's rare... that's so awful to work with.