-
Notifications
You must be signed in to change notification settings - Fork 230
Fix for #1352 #1567
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 for #1352 #1567
Conversation
Co-authored-by: David Widmann <[email protected]>
|
The tests fail since Libtask_jll does not support Julia 1.6.0 yet - it will be fixed by JuliaPackaging/Yggdrasil#2176 but the PR depends on JuliaPackaging/Yggdrasil#1987 (see #1554 for more details). |
|
Maybe we should run Turing tests with Julia 1.5, here and in DynamicPPL? |
Sounds like a good idea to me 👍 |
|
Just for reference: this is now only waiting on packages to get up-to-date with Bijectors.jl and running #1568 |
|
The test error is strange (negative standard deviation in a Normal distribution), do you know what's going on there? |
Very strange indeed. And no, I don't. It shouldn't have anything to do with this particular PR though 😕 |
|
It's all the Gibbs-sampler btw |
|
This should finally be good to go now, once tests pass 👍 |
Pull Request Test Coverage Report for Build 739138834
💛 - Coveralls |
Codecov Report
@@ Coverage Diff @@
## master #1567 +/- ##
==========================================
+ Coverage 78.16% 78.18% +0.01%
==========================================
Files 23 23
Lines 1429 1421 -8
==========================================
- Hits 1117 1111 -6
+ Misses 312 310 -2
Continue to review full report at Codecov.
|
devmotion
left a comment
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.
Looks good, just had a minor suggestion. It seems you also have to update the version number 🙂
|
What in the world. Gibbs is failing again |
|
I'm very confused by that test. Nothing that I did in the fix for MH and linking changed the behavior of Is it okay if I just disable that particular comparison for now and make an issue for it? Really want to get the PR merged, and the only thing holding it back is tests unrelated to this PR failing due to random seed 😅 |
Happy to merge this PR as-is and open an issue for the failed test. |
Fixes #1352 and increases the lower-bound of DynamicPPL (the recent version includes bug-fixes for
predictandgenerated_quantities).