Type piracy breaks Base.similar on JuMP containers

[CasBex]

Hi, I've seen that type piracy has already been discussed in the issues here (#87). This issue is to highlight a specific case which is bugging me. I will refrain from philosophizing on #87 here.

julia> import Pkg; Pkg.activate("/home/cas/tmp/bugreport_OffsetArrays.jl/")
  Activating project at `~/tmp/bugreport_OffsetArrays.jl`

julia> import Pkg

julia> Pkg.status()
Status `~/tmp/bugreport_OffsetArrays.jl/Project.toml`
  [4076af6c] JuMP v1.23.5
  [6fe1bfb0] OffsetArrays v1.15.0

julia> using JuMP

julia> tmp = Containers.DenseAxisArray(rand(2, 3), 1:2, 0.0:0.1:0.2)
2-dimensional DenseAxisArray{Float64,2,...} with index sets:
    Dimension 1, 1:2
    Dimension 2, 0.0:0.1:0.2
And data, a 2×3 Matrix{Float64}:
 0.859161  0.444293  0.328422
 0.277883  0.582098  0.209645

julia> similar(tmp[:, 0.0])
1-dimensional DenseAxisArray{Float64,1,...} with index sets:
    Dimension 1, 1:2
And data, a 2-element Vector{Float64}:
 6.91582553623834e-310
 6.91581336938644e-310

julia> using OffsetArrays

julia> similar(tmp[:, 0.0])
ERROR: MethodError: similar(::JuMP.Containers.DenseAxisArray{Float64, 1, Tuple{…}, Tuple{…}}, ::Type{Float64}, ::Tuple{UnitRange{…}}) is ambiguous.

Candidates:
  similar(A::JuMP.Containers.DenseAxisArray{T, N, Ax, L} where L<:NTuple{N, JuMP.Containers._AxisLookup}, ::Type{S}, axes::Ax) where {T, N, Ax<:(Tuple{var"#s31"} where var"#s31"<:(AbstractVector)), S}
    @ JuMP.Containers ~/.julia/packages/JuMP/i68GU/src/Containers/DenseAxisArray.jl:282
  similar(A::AbstractArray, ::Type{T}, shape::Tuple{Union{Integer, AbstractUnitRange}, Vararg{Union{Integer, AbstractUnitRange}}}) where T
    @ OffsetArrays ~/.julia/packages/OffsetArrays/HLmxQ/src/OffsetArrays.jl:320

Possible fix, define
  similar(::JuMP.Containers.DenseAxisArray{…} where L<:NTuple{…}, ::Type{…}, ::Tuple{…}) where {…}

Stacktrace:
 [1] similar(a::JuMP.Containers.DenseAxisArray{Float64, 1, Tuple{…}, Tuple{…}}, ::Type{Float64})
   @ Base ./abstractarray.jl:821
 [2] similar(a::JuMP.Containers.DenseAxisArray{Float64, 1, Tuple{UnitRange{…}}, Tuple{JuMP.Containers._AxisLookup{…}}})
   @ Base ./abstractarray.jl:820
 [3] top-level scope
   @ REPL[8]:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> show(err)
1-element ExceptionStack:
MethodError: similar(::JuMP.Containers.DenseAxisArray{Float64, 1, Tuple{UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}}, ::Type{Float64}, ::Tuple{UnitRange{Int64}}) is ambiguous.

Candidates:
  similar(A::JuMP.Containers.DenseAxisArray{T, N, Ax, L} where L<:NTuple{N, JuMP.Containers._AxisLookup}, ::Type{S}, axes::Ax) where {T, N, Ax<:(Tuple{var"#s31"} where var"#s31"<:(AbstractVector)), S}
    @ JuMP.Containers ~/.julia/packages/JuMP/i68GU/src/Containers/DenseAxisArray.jl:282
  similar(A::AbstractArray, ::Type{T}, shape::Tuple{Union{Integer, AbstractUnitRange}, Vararg{Union{Integer, AbstractUnitRange}}}) where T
    @ OffsetArrays ~/.julia/packages/OffsetArrays/HLmxQ/src/OffsetArrays.jl:320

Possible fix, define
  similar(::JuMP.Containers.DenseAxisArray{T, N, Ax, L} where L<:NTuple{N, JuMP.Containers._AxisLookup}, ::Type{T}, ::Tuple{AbstractUnitRange}) where {T, T, N, Ax<:(Tuple{var"#s31"} where var"#s31"<:(AbstractVector))}

Stacktrace:
 [1] similar(a::JuMP.Containers.DenseAxisArray{Float64, 1, Tuple{UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}}, ::Type{Float64})
   @ Base ./abstractarray.jl:821
 [2] similar(a::JuMP.Containers.DenseAxisArray{Float64, 1, Tuple{UnitRange{Int64}}, Tuple{JuMP.Containers._AxisLookup{Tuple{Int64, Int64}}}})
   @ Base ./abstractarray.jl:820
 [3] top-level scope
   @ REPL[8]:1
julia> 

The JuMP implementation for the Base.similar(a::JuMP.Containers.DenseAxisArray, ::Type, axes) seems pretty reasonable to me (pasted below) but breaks due to the type piracy in this package.

# We specify `Ax` for the type of `axes` to avoid conflict where `axes` has type
# `Tuple{Vararg{Int,N}}`.
function Base.similar(
    A::DenseAxisArray{T,N,Ax},
    ::Type{S},
    axes::Ax,
) where {T,N,Ax<:Tuple{<:AbstractVector},S}
    return construct_undef_array(S, axes)
end

# Avoid conflict with method defined in Julia Base when the axes of the
# `DenseAxisArray` are all `Base.OneTo`:
function Base.similar(
    ::DenseAxisArray{T,N,Ax},
    ::Type{S},
    axes::Ax,
) where {T,N,Ax<:Tuple{Base.OneTo,Vararg{Base.OneTo}},S}
    return construct_undef_array(S, axes)
end

[aplavin]

MethodError: similar(...) is ambiguous.

Not defending type piracy here, but ambiguity errors like this can arise without any piracy at all. And this happens regularly!
For a recent example, see
https://github.com/JuliaArrays/StructArrays.jl/blob/master/ext/StructArraysStaticArraysExt.jl#L43-L45
that we had to add to StructArrays to avoid similar and reshape ambiguity with StaticArrays. Neither package pirates reshape.

[CasBex]

I would argue that StaticArrays is doing some fishy stuff with the type unions there. I don't want to go into the rabbit hole of getting the exact type piracy definition as mentioned in JuliaArrays/StaticArrays.jl#1248 where you could say that the wider Union does not affect dispatch in Base.

It seems reasonable to me that neither of below should be defined in package A:

function packageB.func(data::packageC.sometype) end
function packageB.func(data::Union{packageA.sometype,packageC.sometype}) end
function packageB.func(data::packageD.sometype{T}) where {T<:packageC.sometype} end

[aplavin]

It seems reasonable to me that neither of below should be defined in package A: <...>

Yes, as I said I'm not defending piracy here :)

[jishnub]

Type-piracy aside, the method defined in JuMP should define the method for AbstractUnitRanges anyway.

function Base.similar(
    A::DenseAxisArray{T,N,Ax},
    ::Type{S},
    axes::Ax,
-) where {T,N,Ax<:Tuple{<:AbstractVector},S}
+) where {T,N,Ax<:Tuple{AbstractUnitRange{<:Integer}},S}
    return construct_undef_array(S, axes)
end

These are the only types valid as axes of an AbstractArray. Incidentally, this would also avoid the method ambiguity seen here.

[CasBex]

I think the point of the DenseAxisArray in JuMP is that you can have more flexible indices than just the AbstractUnitRange (see their documentation). At that point the DenseAxisArray becomes a regular Array I guess?