LinAlg between Triangular and KeyedArray is ambiguous

[molet]

NamedDimsArray works nicely with triangular matrices but KeyedArray gives an error:

julia> using AxisKeys, LinearAlgebra, Statistics
julia> data = rand(3, 5);
julia> KA = KeyedArray(data, features=[:a, :b, :c], time=1:5);
julia> U = cholesky(cov(data')).U;
julia> U \ KA.data
3×5 NamedDimsArray(::Matrix{Float64}, (:features, :time)):
            → time
↓ features  -4.95547  -4.3829    -3.22884  -2.74354  -2.12881
             2.49055   0.510476   2.61172   1.07843   0.476692
             7.43526   7.28534    4.5931    5.92775   2.64983
julia> U \ KA
ERROR: MethodError: \(::UpperTriangular{Float64, Matrix{Float64}}, ::KeyedArray{Float64, 2, NamedDimsArray{(:features, :time), Float64, 2, Matrix{Float64}}, Tuple{Vector{Symbol}, UnitRange{Int64}}}) is ambiguous. Candidates:
  \(A::Union{LowerTriangular, UpperTriangular}, B::AbstractMatrix{T} where T) in LinearAlgebra at /Applications/Julia-1.6.app/Contents/Resources/julia/share/julia/stdlib/v1.6/LinearAlgebra/src/triangular.jl:1668
  \(x::AbstractMatrix{T} where T, y::KeyedArray{T, 2, AT, RT} where {T, AT, RT}) in AxisKeys at /Users/molet/.julia/packages/AxisKeys/VS8DQ/src/functions.jl:302
Possible fix, define
  \(::Union{LowerTriangular, UpperTriangular}, ::KeyedArray{T, 2, AT, RT} where {T, AT, RT})
Stacktrace:
 [1] top-level scope
   @ REPL[435]:1

[mcabbott]

Dispatch is tricky... it would be possible to add more methods here to catch this:
https://github.com/mcabbott/AxisKeys.jl/blob/master/src/functions.jl#L287-L303

I wish there was a better approach though. For scalars, Base has clever promote_rule logic which avoids having to make ever more specific 2-arg dispatches for every new type.

Note that NamedDims isn't handling this at all:

julia> @which U \ KA.data
\(A::Union{LowerTriangular, UpperTriangular}, B::AbstractMatrix{T} where T) in LinearAlgebra at /Applications/Julia-1.6.app/Contents/Resources/julia/share/julia/stdlib/v1.6/LinearAlgebra/src/triangular.jl:1668

The reason that has names is that it calls similar and (in what I think is a bug) that preserves names if the ndims happens to match. I think that since U has no names, the correct result would probably be to remove the name from the first dimension, as * does (or would do):

julia> similar(KA.data, Float64, (3,3))
3×3 NamedDimsArray(::Matrix{Float64}, (:features, :time)):
            → time
↓ features  2.30671e-314  2.30662e-314  2.30662e-314
            0.0           0.0           0.0
            2.30667e-314  2.30667e-314  2.30667e-314

julia> U.data * KA.data
3×5 NamedDimsArray(::Matrix{Float64}, (:_, :time)):
     → time
↓ _   0.159297   -0.197652    0.148984   0.0314685  -0.139759
      0.109955    0.0518916   0.29245    0.0456527   0.02327
     -0.0539889   0.138935   -0.0532439  0.0170399   0.0963143

julia> U * KA.data
ERROR: MethodError: *(::UpperTriangular{Float64, Matrix{Float64}}, ::NamedDimsArray{(:features, :time), Float64, 2, Matrix{Float64}}) is ambiguous. Candidates:
  *(a::AbstractMatrix{T} where T, b::NamedDimsArray{var"#s14", var"#s15", 2, A} where {var"#s14", var"#s15", A<:AbstractMatrix{var"#s15"}}) in NamedDims at /Users/me/.julia/dev/NamedDims/src/functions_math.jl:79
  *(A::LinearAlgebra.AbstractTriangular, B::AbstractMatrix{T} where T) in LinearAlgebra at /Applications/Julia-1.6.app/Contents/Resources/julia/share/julia/stdlib/v1.6/LinearAlgebra/src/triangular.jl:1652
Possible fix, define
  *(::LinearAlgebra.AbstractTriangular, ::NamedDimsArray{var"#s14", var"#s15", 2, A} where {var"#s14", var"#s15", A<:AbstractMatrix{var"#s15"}})

[bencottier]

Just adding that this is also the case for Diagonal.

julia> Diagonal(rand(3,)) \ KeyedArray(rand(3,), features=[:a, :b, :c]);
ERROR: MethodError: \(::Diagonal{Float64, Vector{Float64}}, ::KeyedArray{Float64, 1, NamedDimsArray{(:features,), Float64, 1, Vector{Float64}}, Base.RefValue{Vector{Symbol}}}) is ambiguous. Candidates:
  \(x::AbstractMatrix{T} where T, y::KeyedArray{T, 1, AT, RT} where {T<:Number, AT, RT}) in AxisKeys at /Users/bencottier/.julia/packages/AxisKeys/MhCtB/src/functions.jl:302
  \(x::AbstractMatrix{T} where T, y::KeyedArray{T, 1, AT, RT} where {T, AT, RT}) in AxisKeys at /Users/bencottier/.julia/packages/AxisKeys/MhCtB/src/functions.jl:302
  \(D::Diagonal, b::AbstractVector{T} where T) in LinearAlgebra at /Applications/Julia-1.6.app/Contents/Resources/julia/share/julia/stdlib/v1.6/LinearAlgebra/src/diagonal.jl:627
Possible fix, define
  \(::Diagonal, ::KeyedArray{T, 1, AT, RT} where {T<:Number, AT, RT})
Stacktrace:
 [1] top-level scope
   @ REPL[16]:1

[mcabbott]

Maybe we should add a few more cases if they are useful?

I think the long-term solution is (1) to upgrade LinearAlgebra to work with OffsetArrays, by propagating axes(A) not size(A) every time, and (2) then do #6 so that these hold this package's information. The information is per-axis, so I think trying to re-arrange more things to dispatch on axes, not on whole arrays, would simplify matters.

[oxinabox]

NamedDims.jl has a macro for delcaring overloads for matmul to reduce ambiguities (it might not be perfect or complete but it is a useful idea)
Can we make a similar solution here so we can go and easily resolve these?
https://github.com/invenia/NamedDims.jl/blob/04e65e5b54e07f223a35d2dedb8081d4f525c999/src/functions_math.jl#L59-L85

It would be nice to deal with this, it is causing many problems right now.

It is a general struggle for wrapper arrays, and why people don't love the solution.
But for now we can make easy and good work-arounds but just putting in the overloads that are needed to make it work.

[mcabbott]

I have added a few more methods in 7919f12 , so the examples on this page should (I think) now work.

We are up to 180 methods now for *. Some of which don't have NamedDims counterparts:

julia> count(m -> contains(string(m), "KeyedArray"), methods(*))
180

julia> NamedDimsArray(rand(3,3), (:a, :b)) * UpperTriangular(rand(3,3))
ERROR: MethodError: *(::NamedDimsArray{(:a, :b), Float64, 2, Matrix{Float64}}, ::UpperTriangular{Float64, Matrix{Float64}}) is ambiguous.

[glennmoy]

Thanks @mcabbott, the most recent tag has resolved these issues but in the process revealed another one to do with multiplying a KeyedVector by an adjoint.

julia> ones(2, 2)' * KeyedArray([1, 2]; id=[:a, :b])
ERROR: MethodError: *(::LinearAlgebra.Adjoint{Float64, Matrix{Float64}}, ::KeyedArray{Int64, 1, NamedDimsArray{(:id,), Int64, 1, Vector{Int64}}, Base.RefValue{Vector{Symbol}}}) is ambiguous. Candidates:
  *(adjA::LinearAlgebra.Adjoint{var"#s814", var"#s813"} where {var"#s814", var"#s813"<:AbstractMatrix{T}}, x::AbstractVector{S}) where {T, S} in LinearAlgebra at /Applications/Julia-1.6.app/Contents/Resources/julia/share/julia/stdlib/v1.6/LinearAlgebra/src/matmul.jl:110
  *(x::LinearAlgebra.Adjoint{var"#s49", var"#s7"} where {var"#s49", var"#s7"<:(AbstractMatrix{T} where T)}, y::KeyedArray{T, 1, AT, RT} where {T<:Number, AT, RT}) in AxisKeys at /Users/glenn/.julia/packages/AxisKeys/pm7Fb/src/functions.jl:340
  *(x::AbstractMatrix{T} where T, y::KeyedArray{T, 1, AT, RT} where {T<:Number, AT, RT}) in AxisKeys at /Users/glenn/.julia/packages/AxisKeys/pm7Fb/src/functions.jl:340
  *(x::LinearAlgebra.Adjoint{var"#s49", var"#s7"} where {var"#s49", var"#s7"<:(AbstractMatrix{T} where T)}, y::KeyedArray{T, 1, AT, RT} where {T, AT, RT}) in AxisKeys at /Users/glenn/.julia/packages/AxisKeys/pm7Fb/src/functions.jl:340
  *(x::AbstractMatrix{T} where T, y::KeyedArray{T, 1, AT, RT} where {T, AT, RT}) in AxisKeys at /Users/glenn/.julia/packages/AxisKeys/pm7Fb/src/functions.jl:340
Possible fix, define
  *(::LinearAlgebra.Adjoint{var"#s49", var"#s7"} where {var"#s49", var"#s7"<:AbstractMatrix{T}}, ::KeyedArray{S, 1, AT, RT} where {AT, RT}) where {T, S<:Number}
Stacktrace:
 [1] top-level scope
   @ REPL[49]:1

I gave it a shot at fixing it in #90 but I'm not sure if this is the best way to do it.