-
Notifications
You must be signed in to change notification settings - Fork 227
Fix hessian bug #1232
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
Fix hessian bug #1232
Conversation
@cpfiffer It seems some tests are broken with this PR |
Yeah, it requires TuringLang/DynamicPPL.jl#83 to be merged & released before it should work, though I'm not sure what stuff is in the air over there. @mohamed82008 could TuringLang/DynamicPPL.jl#83 be merged and released without too much issue at DynamicPPL? |
2661769
to
701f5d2
Compare
Codecov Report
@@ Coverage Diff @@
## master #1232 +/- ##
=======================================
Coverage 67.11% 67.11%
=======================================
Files 25 25
Lines 1341 1341
=======================================
Hits 900 900
Misses 441 441
Continue to review full report at Codecov.
|
The test failures here seem unrelated. |
I think they're not actually. Using |
See, e.g., https://github.com/TuringLang/DynamicPPL.jl/blob/c3d430c774628e1d7f5f8ee0167c0704a84df265/test/compiler.jl#L365 and TuringLang/DynamicPPL.jl@43e8eee#diff-692d6d33d7ed80773e898f56e60b88ab (for the functional alternative |
Oh, I see what you mean. I hadn't noticed that this syntax didn't work for Julia 1.0. Let me try this and see what happens. |
Alright, looks like tests are passing. |
Fixes #1231, which is necessary for #1230. Adds a test to make sure the Hessian is being correctly calculated with ReverseDiff and ForwardDiff.