JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
44.99k stars 5.42k forks source link

Remove implicit pinvs of vectors #39987

Open antoine-levitt opened 3 years ago

antoine-levitt commented 3 years ago

This is breaking, so probably has to wait until 2.0. At that point, can we remove /(x::Number, v::AbstractVector)? Its (marginal) utility is outmatched by its potential for confusion for new users and subtle bugs even for more experienced coders. More generally there are two and only two places where we call pinv on vectors implicitly:

(\)(a::AbstractVector, b::AbstractArray) = pinv(a) * b
/(x::Number, v::AbstractVector) = x*pinv(v)

I think we should just remove both. Pinv of a vector is just too confusing to be called implicitly. In fact, this is even more confusing because we don't allow number / matrix or matrix \ number, although pinv of a matrix makes way more sense than pinv of a vector. The fact that nobody has bothered adding this makes me think this function won't be missed.

ViralBShah commented 3 years ago

Yes, it does seem like we need to wait until 2.0.

We could prepare a PR and run package evaluator to see how breaking it is. If not, we could just roll it out.

dlfivefifty commented 3 years ago

One could argue that matrix \ vector for least squares is also confusing and that there should be a separate syntax when matrix is not-invertible, e.g., matrix \\ vector, when a user actually really does want least squares

antoine-levitt commented 3 years ago

That'd be even more breaking and a lot more controversial, but I'd be in favor also.

Another possibility is to keep \ as is, but remove / entirely. It's very rarely useful as a matrix-vector op (because of the "vectors are columns" convention), and many people don't even know it exists. I've only ever used it for change of basis (eg P * D / P)

dlfivefifty commented 3 years ago

I use / all the time... e.g. 1 / 2....

I like the principle of / and \ being just left and right versions of the same operation. Anything else feels arbitrary from a mathematical perspective (though I do agree anyone coming from Matlab will only ever use \ for linear algebra.)

It's the fact that either uses least squares that's the problem, particularly since it's completely inconsistent between square and rectangular matrices:

julia> zeros(1,1) \ [1]
1-element Vector{Float64}:
 Inf

julia> zeros(2,1) \ [1,1]
1-element Vector{Float64}:
 0.0

julia> pinv(zeros(1,1)) * [1]
1-element Vector{Float64}:
 0.0

Having a distinct syntax would fix this.

antoine-levitt commented 3 years ago

Pseudo-inverses are A^+, so possibly /+ and \+ could be made to parse as operators? Although you can certainly use least squares without being aware of pseudoinverses, so it might not be useful to reference pseudo inverses

ViralBShah commented 3 years ago

Agree that removing the \ for least squares could be very disruptive, and is something to be considered 2.0.

martinholters commented 3 years ago

Also ref. #28827 regarding making / and \ only do division in the narrower sense. I'd be in favor, but agree that it's obviously 2.0-stuff.

ViralBShah commented 2 years ago

Fix for the scalar case in https://github.com/JuliaLang/julia/pull/44358