RFC: Refactoring to merge the code handling Adjoint and Transpose

[tkf]

I think the majority of code handling Adjoint and Transpose can be merged. The trick is to define a trait function

julia/stdlib/LinearAlgebra/src/adjtrans.jl

Lines 86 to 87 in 1e83dd4

	functor(::Type{<:Adjoint}) = adjoint
	functor(::Type{<:Transpose}) = transpose

so that we can write code like this

julia/stdlib/LinearAlgebra/src/triangular.jl

Lines 855 to 867 in 1e83dd4

	function lmul!(adjA::AdjOrTrans{<:Any,<:UpperTriangular}, B::StridedVecOrMat)
	A = adjA.parent
	f = functor(adjA)
	m, n = size(B, 1), size(B, 2)
	if m != size(A, 1)
	throw(DimensionMismatch("right hand side B needs first dimension of size $(size(A,1)), has size $m"))
	end
	for j = 1:n
	for i = m:-1:1
	Bij = f(A.data[i,i]) * B[i,j]
	for k = 1:i - 1
	Bij += f(A.data[k,i]) * B[k,j]
	end

Note that adjA can be a Adjoint or Transpose in above code. The function f is adjoint or transpose depending on the type of adjA.

I applied the idea to a few places so that you can see the impact of this change. You can already see that many lines can be removed.

What do you think? Does this refactoring makes sense?

[fredrikekre]

Another alternative would be

functor!(::Type{<:Adjoint}) = adjoint!

[antoine-levitt]

+1 on the refactor. The functor name is not great though. BLAS/LAPACK usually refer to this kind of thing as "op" in the doc and "trans" in the argnames, so maybe trans_op? Trans having the advantage that it's short for transform as well as transpose.

[tkf]

I find trans_op confusing (it says "trans" but can return adjoint?) but it's just an internal function so I don't mind changing the name if core devs say so.

[andreasnoack]

I'm also in favor but I agree with @antoine-levitt about the name.

It looks like the change introduces some ambiguities.

[KristofferC]

Also, +1 for the refactor but functor and inplace sounds very generic when it does something quite specific to only two types. Something more direct and ugly like trans_or_adj would make the code clearer without having to look up the docstring of what seems to be a quite generic function.

[tkf]

OK so does

trans_or_adj(::Type{<:Adjoint}) = adjoint
trans_or_adj!(::Type{<:Adjoint}) = adjoint!

look fine for everyone?

It looks like the change introduces some ambiguities.

Can we do make ambiguity situation better? I suggest one simple solution in #33072. If there is no systematic solution I guess I'll do it the old way here...

[tkf]

Actually, I guess it probably should be something like

inplace_trans_or_adj(::Type{<:Adjoint}) = adjoint!

since otherwise

f! = trans_or_adj!(A)
f!(B)

might looke like we are mutating A as well.