lu(A, Val{false}) should be lu(A, pivot=false)

[stevengj]

I use the unpivoted lu function a lot in teaching linear algebra, because its results are more easily compared to hand calculations (where you rarely permute rows), and it is continually annoying to me that you do this by passing Val{false} as the second argument — I feel like I'm exposing an ugly internal. Can't we changed this to a pivot=false keyword?

LU factorization is a fairly expensive operation, so I am mystified by why we would care about dispatch performance here. Surely the cost of a runtime branch is negligible?

cc @andreasnoack

[andreasnoack]

It's for consistency with the other factorizations that support pivoting (Cholesky and QR) because they have special types and we therefore need compile time dispatch for type stability in those cases. I don't like Vals but I wouldn't like different kinds of pivoting arguments for different factorizations. I'd be happy with finding a prettier alternative as long as it is consistent across factorizations and doesn't sacrifice type stability.

[stevengj]

Maybe define a singleton type? e.g. lu(A, NoPivot) or lu(A, Pivot{false})?

[oscardssmith]

I have no idea if this is feasible, but would it be possible to combine keyword arguments and Vals, to make it be pivot = Val{false}? Alternatively, is there any reason Val is needed for dispatch on the caller side? It feels like the caller should just pass false, and then internals should be able to figure out if there are multiple methods depending on the value passed in.

[stevengj]

@oscardssmith, the whole difficulty here is that checking the values can only be done at runtime, not a compile time. This is too late for type-stability if the return type depends on whether pivot == true, as it does for qrfact:

julia> qrfact(rand(5,3), Val{true})
Base.LinAlg.QRPivoted{Float64,Array{Float64,2}} with factors Q and R:
[-0.33213 -0.66443 … -0.454553 -0.484674; -0.153278 -0.529465 … 0.0638774 0.805849; … ; -0.594088 -0.104926 … 0.743544 -0.183901; -0.413672 0.370117 … -0.0415064 -0.0449306]
[-1.65013 -1.12184 -0.475066; 0.0 -0.803798 -0.0116616; 0.0 0.0 0.529326]

julia> qrfact(rand(5,3), Val{false})
Base.LinAlg.QRCompactWY{Float64,Array{Float64,2}} with factors Q and R:
[-0.0727649 0.501108 … -0.087708 -0.694424; -0.240891 -0.318195 … -0.65019 0.295001; … ; -0.468053 -0.507519 … 0.628181 -0.165707; -0.82135 0.187781 … -0.245073 -0.0892836]
[-0.618574 -0.833208 -1.18345; 0.0 0.580143 -0.202586; 0.0 0.0 -0.47824]

[Sacha0]

(Closely related to #24011.)

[andreasnoack]

Ref: #22232

[ChrisRackauckas]

Since 1.0 will have #24362 , we should consider getting rid of the value-type dispatching here and simplifying the API to true/false which can be type-stable on literal arguments when/if it's propagated. This would be a breaking API change, so it would have to be 1.0?

[martinholters]

This would be a breaking API change, so it would have to be 1.0?

We could still allow a Val to be passed, to make the change non-breaking.

[alanedelman]

I hate LU(A,Val{False}) :-(