Make pinv work for Adjoint #29837
Make pinv work for Adjoint #29837
Conversation
stdlib/LinearAlgebra/src/dense.jl
Outdated
| @@ -1266,10 +1266,10 @@ function pinv(A::StridedMatrix{T}, rtol::Real) where T | |||
| Sinv = zeros(Stype, length(SVD.S)) | |||
| index = SVD.S .> rtol*maximum(SVD.S) | |||
| Sinv[index] = one(Stype) ./ SVD.S[index] | |||
| Sinv[findall(.!isfinite.(Sinv))] .= zero(Stype) | |||
There was a problem hiding this comment.
Why is this change needed?
There was a problem hiding this comment.
It's not needed for this change. However, zero(Stype) isn't needed since the array is already allocated with Stype elements and 0 is easier to read than zero(Stype).
There was a problem hiding this comment.
Aren't there places where zero(T) works but convert(T, 0) fails? I'd let sleeping dogs lie here, especially since we also use one and zeros immediately surrounding this line.
There was a problem hiding this comment.
There are but they don't apply here since Stype is the type the singular values. It's mainly for non-number types like arrays that you can't rely on converting from 0. This change is unrelated to the PR so I've removed it.
There was a problem hiding this comment.
I had also been thinking of unitful numbers but hadn't worked out if/how unitful SVDs work. In any case, thanks for indulging my conservativeness.
There was a problem hiding this comment.
That is true. The singular values have units so that would fail. Unfortunately, it would be a bit complicated to make the svd solver in GenericLinearAlgebra work for Unitful.Quantity because it's only a subtype of Number.
Fixes #29723