Closed gerlero closed 7 months ago
Attention: 1 lines
in your changes are missing coverage. Please review.
Comparison is base (
211b675
) 82.94% compared to head (72e4201
) 83.37%. Report is 2 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
src/AbstractDifferentiation.jl | 93.33% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I think the change is welcome here and coherent regardless of whether it makes it into ForwardDiff. However it probably needs tests too, which I guess is why the PR remains a draft. Thank you so much for taking the time!
@gdalle Thanks. Tests are now live and passing so this is ready for review
Gentle bump @gdalle; can you be a reviewer in this PR?
@gdalle @devmotion Thanks for the reviews.
Any reason why you didn't add tests for Tracker and RuleConfig backends?
I simply omitted them because I saw that those backends don't have tests for hessian
either. I've now tried adding the tests: RuleConfig
passes, but Tracker
fails (ArgumentError: Tracker backend does not support nested differentiation.
). So, I've now added the RuleConfig
backend tests, but left Tracker
out.
EDIT: the RuleConfig
tests fail with Julia nightly though..
As soon as I understand what's going on with the tags, it's good to merge on my end
Thanks. Can we get a new release with this feature?
Draft PR for changes discussed in https://github.com/JuliaDiff/ForwardDiff.jl/pull/678
Fixes #71.