+(Triangular{T<:Number},Triangular{T<:Number}) does not exist

[astrieanna]

The code in linalg/special.jl generates conversions and +/- methods for some special matrix types. (see this loop)

Many of these generated methods call + or - on two Triangulars. This method does not exist.

The problematic methods:

-(Diagonal{T},Triangular{T<:Number})
-(Triangular{T<:Number},Diagonal{T})
-(Bidiagonal{T},Triangular{T<:Number})
-(Triangular{T<:Number},Bidiagonal{T})
-(Tridiagonal{T},Triangular{T<:Number})
-(Triangular{T<:Number},Tridiagonal{T})
-(SymTridiagonal{T},Triangular{T<:Number})
-(Triangular{T<:Number},SymTridiagonal{T})
+(Diagonal{T},Triangular{T<:Number})
+(Triangular{T<:Number},Diagonal{T})
+(Bidiagonal{T},Triangular{T<:Number})
+(Triangular{T<:Number},Bidiagonal{T})
+(Tridiagonal{T},Triangular{T<:Number})
+(Triangular{T<:Number},Tridiagonal{T})
+(SymTridiagonal{T},Triangular{T<:Number})
+(Triangular{T<:Number},SymTridiagonal{T})

(The + methods call +(Triangular,Triangular); the - methods call -(Triangular,Triangular).)

An example of this failing:

julia> t = Triangular([1 0 ; 1 0],:U)
2x2 Triangular{Int64}:
 1  0
 0  0

julia> t + t
ERROR: no method +(Triangular{Int64}, Triangular{Int64})

julia> convert(Diagonal, t)
2x2 Diagonal{Int64}:
 1  0
 0  0

julia> @which d + t
+(A::Diagonal{T},B::Triangular{T<:Number}) at linalg/special.jl:88

julia> d + t
ERROR: no method +(Triangular{Int64}, Triangular{Int64})
 in + at linalg/special.jl:88

[jiahao]

Mea culpa.

Thanks for going through all the linear algebra code, by the way. It's no mean feat.

[astrieanna]

I'm actually using TypeCheck.jl's check_method_calls, which makes finding these easy. (It's exciting when I find out the warning isn't a problem in my code.)

[andreasnoack]

check_method_calls seems pretty useful. I'll have a closer look. Regarding the missing methods, one can argue that it is intended. Originally, these special matrix types were meant for dispatch and not for general arithmetic. It doesn't have to stay that way, but if we want to support general arithmetic there are many methods to cover.

[timholy]

Is there a subset of the types in that codegen loop that actually work? We could delete any definitions that are not going to be supported.

[jiahao]

I've committed +(::Triangular, ::Triangular) and -(::Triangular, ::Triangular) but now I'm wondering if most of the method generation loops in linalg/special.jl which I wrote last year ought to be replaced by promotion rules instead.

[StefanKarpinski]

Wanting these methods to work is pretty strong evidence that Triangular should be split into UpperTriangular and LowerTriangular. I.e. UpperTriangular + UpperTriangular => UpperTriangular and LowerTriangular + LowerTriangular => LowerTriangular but UpperTriangular + LowerTriangular => Array.

[jiahao]

Sounds like we want

typealias UpperTriangular{T,S,isunit} Triangular(T,S,:U,isunit)
typealias LowerTriangular{T,S,isunit} Triangular(T,S,:L,isunit)

? I do agree that :U and :L are not immediately intuitive, and the use of a type parameter here is somewhat marginal since only two valid values of the type parameter exist. Perhaps down the road we can change the internal representation to isupper=true and false, like what is already done for Bidiagonal, and add a type annotation for the isupper type parameter.

[StefanKarpinski]

Yes, that seems like a reasonable way to go. Then you can have

UL(::UpperTriangular) = :U
UL(::LowerTriangular) = :L

when you need to get the symbol. It will be inlined so should be efficient enough.

[andreasnoack]

One of the main reasons to make uplo a parameter was to allow that functions like + and * of triangular matrices can be triangular. Whether to have uplo parameter or two different types is mainly a matter of taste, right? You'll have access to the same functionality from the two definitions. If we decide that UpperTriangular is prettier than Triangular{:U} then we probably want to define UpperUnitTriangular to get rid of the other type parameter as well. Personally, I think I prefer the type parameter versions. Regarding having a boolean parameter instead, I think it is easier to remember :L and :U instead of remembering the bool argument is isupper or islower.

[jiahao]

Just noticed that I hadn't applied the fix to the OP. Feel free to continue the bikeshedding.