-
Notifications
You must be signed in to change notification settings - Fork 26
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
jvp incorrect result rather than erroring #90
Comments
Continuing the discussion from JuliaDiff/ChainRules.jl#196 (comment):
Well it's part of the interface, so not really an implementation detail.
Isn't converting
I don't understand. Why do we need to add tangent vectors to vectors / primals? The definition
requires only scaling and addition in the domain and range. Furthermore, I don't see how we could reasonably define derivatives without these properties. Long story short, I would prefer seeing this issue fixed by making
return Of course, I'm just a random guy on the internet who has not contributed any code to this package yet, so please apologise if my opinion is unsolicited. |
Your opinion is very much valued. Please keep pointing out where you think that things could be improved :)
I agree. The way that I would love to see
If we use Pullbacks / gradients / Jacobians are a bit slightly trickier, and we don't currently have the code to implement this properly. To do this we would need some mechanism for constructing the standard basis for any type (inc. structs etc), which requires some thought but is definitely doable. We would also need a way to apply cotangents to tangents -- this is arguably a missing feature of Either way I would like to see this resolved because more and more I'm finding that |
Can you provide an example where the third item is indeed tricky to implement? As far as I can tell, finite differences only makes sense if both the domain and range are affine spaces, and in this case there is no problem with Item 3. |
No conceptual difficulty, just tricky in the sense that you would need to implement stuff to actually do this. Not all of the types that we would be working with placing nicely with scaling / subtraction (e.g. arbitrary structs) so we would need to roll our own functionality to do that. Were we to do this I imagine we would implement a function |
Or FiniteDifferences could just refuse to work with such types on the grounds that if they do not implement the interface of an affine vector space, then there is no sensible definition of finite differences. |
One way to resolve this would be to 1) fix a generic interface for dual vectors / cotangents and 2) require domain types to implement their own methods for Examples for 1), where
Out of these three, I would choose Examples for 2), assuming the
|
I don't think that this would be a good solution. For example, it would preclude the use of
I agree.
I like this proposal a lot. This would just leave the question of how to represent Jacobians. Perhaps a collection of tangents, one for each input, would make most sense? And perhaps this could be specialised to a matrix in the I do wonder whether we really need to be able to compute vjps / gradients at all though. If one accepts that the sole purpose of _, pullback = rrule(f, x)
dot(ȳ, jvp(fdm, f, x, ẋ)) ≈ dot(pullback(ȳ), ẋ) to test reverse-mode. Perhaps this is a step too far though. It's entirely possible that people are using this package to compute gradients, so losing that functionality would make for a bad user experience. |
Manifolds.jl's default "AD" backend is FiniteDifferences, which it uses in internal functions to compute the gradient. It's placeholder functionality though and currently not used for any higher order functions. 🤷♂️ |
Tuples could be fixed either by wrapping them in an |
Certainly wrapper types would work as alternative to implementing a special function.
|
There appears to be a problem with
jvp
when thev
supplied is real and the primal is complex:The correct, answer here is
1.5
. This should produce an error.The text was updated successfully, but these errors were encountered: