-
Notifications
You must be signed in to change notification settings - Fork 25
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
Implement rand_tangent and difference #91
Conversation
test/difference.jl
Outdated
|
||
@testset "difference" begin | ||
|
||
@testset "$(typeof(x))" for (ε, x) in [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests cover everything covered in the to_vec
tests other than Dict
, which doesn't currently play nicely / I'm not entirely sure how best to construct a tangent, nor how to add it to the primal.
CI failure is weird, pretty sure it's a one-off. edit: by which I mean it's ready for review |
Looks like the Manifest probably just needed re-generating -- it uses a relative path to get the correct version of |
It is afaik. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small suggestion. otherwise LGTM - thanks!
Co-authored-by: Nick Robinson <[email protected]>
@nickrobinson251 this PR no longer exports |
This PR lays some foundations for (hopefully) moving
FiniteDifferences
over toChainRules
types, and contains exactly two things:rand_tangent
over fromChainRulesTestUtils
. It turns out that it's useful here, and sinceChainRulesTestUtils
depends on this package, there shouldn't be any harm implementing them here.difference
, which is more or less what you might expect. If the docstring doesn't make it clear, it would be good to know.edit: Also makes docs build on the latest Julia version.
edit-2: Also changes versions on which this is tested to
1.0
and1
, and disallows 1.3 failure on nightly. I'm guessing this just hadn't been updated for some reason.