Skip to content
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

Rule for svdvals #587

Closed
mcabbott opened this issue Feb 9, 2022 · 3 comments · Fixed by #791
Closed

Rule for svdvals #587

mcabbott opened this issue Feb 9, 2022 · 3 comments · Fixed by #791

Comments

@mcabbott
Copy link
Member

mcabbott commented Feb 9, 2022

Requested here: https://discourse.julialang.org/t/implementation-of-spectral-normalization-for-machine-learning/76074

The workaround is to call svd(X).S, which is slower forwards. But it looks like the gradient calculation with something like svd_rev((; U=NoTangent(), s=s, V=NoTangnet(), Vt=NoTangent()), NoTangent(), S̄, NoTangent()) is probably fairly efficient, and could easily be extracted to its own rule:

function rrule(::typeof(svd), X::AbstractMatrix{<:Real})
F = svd(X)
svd_pullback(ȳ) = _svd_pullback(ȳ, F)
return F, svd_pullback
end

@sethaxen
Copy link
Member

sethaxen commented Feb 9, 2022

I agree, yes, this should be its own rule. We already did that for Hermitian matrices.

@sethaxen
Copy link
Member

sethaxen commented Feb 9, 2022

function rrule(::typeof(svdvals), A::LinearAlgebra.RealHermSymComplexHerm{<:BLAS.BlasReal,<:StridedMatrix})
λ, back = rrule(eigvals, A)
S = abs.(λ)
p = sortperm(S; rev=true)
permute!(S, p)
function svdvals_pullback(ΔS)
∂λ = real.(ΔS)
invpermute!(∂λ, p)
∂λ .*= sign.(λ)
_, ∂A = back(∂λ)
return NoTangent(), unthunk(∂A)
end
svdvals_pullback(ΔS::AbstractZero) = (NoTangent(), ΔS)
return S, svdvals_pullback
end

There I ended up having a custom rule. I don't remember why. It may have been faster than using NoTangent().

@sethaxen
Copy link
Member

sethaxen commented Feb 9, 2022

Ah, no, it goes via the rrule for eigvals, which uses the pullback for eigen. So yeah, just using the pullback for svd might be best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants