-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RowVector, pinv, and quasi-division #23067
Changes from all commits
3541e80
e47c37c
ff2c974
756a92b
703f942
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -795,6 +795,19 @@ function inv(A::AbstractMatrix{T}) where T | |
A_ldiv_B!(factorize(convert(AbstractMatrix{S}, A)), eye(S0, checksquare(A))) | ||
end | ||
|
||
function pinv(v::AbstractVector{T}, tol::Real=real(zero(T))) where T | ||
res = similar(v, typeof(zero(T) / (abs2(one(T)) + abs2(one(T)))))' | ||
den = sum(abs2, v) | ||
# as tol is the threshold relative to the maximum singular value, for a vector with | ||
# single singular value σ=√den, σ ≦ tol*σ is equivalent to den=0 ∨ tol≥1 | ||
if iszero(den) || tol >= one(tol) | ||
fill!(res, zero(eltype(res))) | ||
else | ||
res .= v' ./ den | ||
end | ||
return res | ||
end | ||
|
||
""" | ||
\\(A, B) | ||
|
||
|
@@ -842,10 +855,11 @@ function (\)(A::AbstractMatrix, B::AbstractVecOrMat) | |
return qrfact(A,Val(true)) \ B | ||
end | ||
|
||
(\)(a::AbstractVector, b::AbstractArray) = reshape(a, length(a), 1) \ b | ||
(\)(a::AbstractVector, b::AbstractArray) = pinv(a) * b | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But should this use |
||
(/)(A::AbstractVecOrMat, B::AbstractVecOrMat) = (B' \ A')' | ||
# \(A::StridedMatrix,x::Number) = inv(A)*x Should be added at some point when the old elementwise version has been deprecated long enough | ||
# /(x::Number,A::StridedMatrix) = x*inv(A) | ||
/(x::Number, v::AbstractVector) = x*pinv(v) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes the result between two versions without any deprecation and have just caught me completely by surprise. One more evidence why giving completely different meaning for operations that used to have elwize meanings makes no sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which versions? It is an error on 0.6. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, there used to be julia> VERSION
v"0.6.0"
julia> 1 / (1:4)
4-element Array{Float64,1}:
1.0
0.5
0.333333
0.25 julia> VERSION
v"0.7.0-DEV.2158"
julia> 1 / (1:4)
1×4 RowVector{Float64,Array{Float64,1}}:
0.0333333 0.0666667 0.1 0.133333 The old method was removed in #22932 without adding an appropriate deprecation. So I guess we could add a deprecated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I may add, when dividing a scalar by a vector, taking the pseudo-inverse of the "vector" and then multiplying it by a constant seems very far from an intuitive behavior to me. I think if one is doing this operation and one is aware it is taking the pseudoinverse, then explicitly calling |
||
|
||
cond(x::Number) = x == 0 ? Inf : 1.0 | ||
cond(x::Number, p) = cond(x) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be
tol::Real=eps(real(float(one(T))))*length(v)
for consistency withpinv(::StridedMatrix{T})
(and likewise below)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I was under the impression that
pinv(::StridedMatrix{T}, tol)
ignores all singular values belowtol
, but the threshold is actuallytol * maximum(singular_values)
, so for the vector case, it only matters whethertol < 1
. So the with the computed defaulttol
consistent with the matrix case, an all-zero vector would be returned if the input is all-zero, which makes sense, or iflength(v) > 1/eps(real(float(one(T))))
, which strikes me as pretty bizarre.I think I'd rather keep the
tol=0
default than aim for maximum consistency here. Opinions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pinv(::Diagonal)
also usestol=0
by default, so there is precedence for inconsistency here. Users striving for consistency across types should probably givetol
explicitly, otherwise exploiting information the type conveys to choose a sensible default seems pretty reasonable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow the reasoning here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A vector has one singular value s, which obvious is the maximum one. That singular value will be ignored (resulting in a zero vector) if s<=s*tol, i.e. tol>=1 or s=0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course.
I can see that the
length(v) > 1/eps(real(float(one(T))))
condition is a bit weird although it might not be a big practical concern since it would be 32 PB forFloat64
. However, I did some simulations and I'm wondering if using the maximum length is the right thing to do. It looks like it is the minimum that determines the error and using the minimum would fix the bizarre case for vectors.