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

Generalize autodiff functions #24

Merged
merged 28 commits into from
Aug 11, 2020
Merged

Generalize autodiff functions #24

merged 28 commits into from
Aug 11, 2020

Conversation

lyndond
Copy link
Member

@lyndond lyndond commented Jan 31, 2020

Info:
Methods vector_jacobian_product and jacobian_vector_product can now take multiple vectors (useful for so-called block iterative methods).

Summary:

  • vjp and jvp can take blocks of vectors and compute products along each direction
  • fixed bug in jacobian(); handles edge-case where dim_y = 1 and it returned a flat (dim_x,) instead of (1, dim_x) tensor
  • jacobian() issues a warning when input dimensionality is >1E6
  • tidied up documentation for each function
  • removed some user-facing control to handle creating/retaining graph during vjp/jvp
  • vjp now has optional Bool argument to detach output
  • created tests for all autodiff functions in test_synthesis.py
  • functions in eigendistortion.py were changed accordingly to accommodate new changes

@billbrod billbrod added this to Review in progress in Roadmap Feb 7, 2020
@billbrod
Copy link
Member

billbrod commented Feb 7, 2020

I'll look at this this afternoon, but this is what's in #10 , right?

tests/test_synthesis.py Outdated Show resolved Hide resolved
Copy link
Member

@billbrod billbrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small changes requested, largely on docs

@billbrod
Copy link
Member

billbrod commented Feb 7, 2020

Yeah and just to reiterate my question from earlier, does this accomplish what I put in issue #10 ? That is, can they now accept 3d/4d inputs or is the reshaping trick still necessary?

EDIT: Answering my own question, no they do not. That's fine for this commit, but should think about whether we want that eventually.

…ic into fix-autodiff

Updating my branch to master.
@billbrod billbrod mentioned this pull request Jun 15, 2020
4 tasks
@billbrod
Copy link
Member

I added some small comments, overall this looks good to me. Tutorial in particular looks good.

@lyndond lyndond merged commit 4535771 into master Aug 11, 2020
Roadmap automation moved this from Review in progress to Done Aug 11, 2020
@lyndond lyndond deleted the fix-autodiff branch August 14, 2020 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Roadmap
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants