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

Update Cholesky and SVD to use Composite #151

Closed
2 tasks done
oxinabox opened this issue Jan 16, 2020 · 3 comments
Closed
2 tasks done

Update Cholesky and SVD to use Composite #151

oxinabox opened this issue Jan 16, 2020 · 3 comments
Labels
good first issue Good for newcomers

Comments

@oxinabox
Copy link
Member

oxinabox commented Jan 16, 2020

Composite was made for this case,
but we haven't changed it over, and it still uses a hack.

  • SVD
  • Cholesky
@nickrobinson251 nickrobinson251 added the good first issue Good for newcomers label Jan 16, 2020
@nickrobinson251
Copy link
Contributor

I'd like to take a go at this, to understand better whatsup with Cmposite, but will probably need some guidance. Will try to open a PR this weekend.

For SVD, will we have svd_pullback(Ȳ::Composite{<:SVD}) and then in getproperty_svd_pullback change all uses of NamedTuples to Composites?

@oxinabox
Copy link
Member Author

Will try to open a PR this weekend.

It's the weekend.


For SVD, will we have svd_pullback(Ȳ::Composite{<:SVD}) and then in getproperty_svd_pullback change all uses of NamedTuples to Composites?

That sounds right.

The important part to remember about Composites in reverse mode,
is if your primal argument was a struct, then your pullback return is a Composite.
So that is your getproperty_svd_pullback

And conversely:
if the primal return value is a struct, your pullback argument is a Composite.
so that is your svd_pullback

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

No branches or pull requests

2 participants