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

Add SVD factorization rrule #31

Merged
merged 1 commit into from
May 24, 2019
Merged

Add SVD factorization rrule #31

merged 1 commit into from
May 24, 2019

Conversation

ararslan
Copy link
Member

This adds rrules for the SVD factorization as well as an accompanying rrule for getproperty on SVD objects. The definitions are ported from Nabla.

This uses the scheme agreed upon in #21, in which evaluating a Rule from rrule returns a NamedTuple, rather than rrule returning a NamedTuple of Rules. I believe this PR shows that it is the most consistent and sensible choice.

This adds `rrule`s for the SVD factorization as well as an accompanying
`rrule` for `getproperty` on `SVD` objects. The definitions are ported
from Nabla.
@ararslan ararslan requested a review from willtebbutt May 11, 2019 21:59
@GiggleLiu
Copy link

Does it support complex values? If not, here is a version with tested complex valued AD
https://github.com/GiggleLiu/LinalgBackwards.jl/blob/master/src/svd.jl

Hope it can be helpful to ChainRules.

@ararslan
Copy link
Member Author

Cool, thanks! We don't currently have a mechanism in place to automatically test complex-valued derivatives, since we use FDM for comparison against finite differencing, and I don't believe FDM supports complex values. But I'll look into incorporating your definition (I actually think it's basically the same as what's implemented here) once I figure out an appropriate way to test it within the ChainRules framework.

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

Successfully merging this pull request may close these issues.

2 participants