-
Notifications
You must be signed in to change notification settings - Fork 4
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
Model with cholesky_factor_cov data errors every other time #76
Comments
Hi Seth, thanks for your heads-up! This is what I see when I run your program in Stan.jl. Will try a few more things to see if I can reproduce the issue. Below DataFrames have 4000 rows, it seems all 4 chains are ok. Your example program:
Running the program:
Output (abbreviated):
and:
My setup on MacOS:
and:
|
@goedman, yes, I see the same with your example. But if I add a 2nd |
I forgot below quick check:
But that looks ok. |
Aah, let me try that! |
Yip, same problem here! Will have a look, seems to trigger an old memory about this issue ... |
@sethaxen Hmmm, very weird. Haven't made much progress except that below version seems to work:
Which is 90% identical to what happens in stan_sample(). Will dig further. If you need this, above construct might be a temporary work around. |
Hi Seth ( @sethaxen ), Turns out a better work around for now is to use:
instead of a Dict. Will figure out what's wrong in handling Dicts! |
Found the problem. StanSample.jl v7.7.0 (just merged) should fix this. |
@goedman thanks for the fix! What was it? The only change I see in v7.7.0 is that Distributed is no longer loaded. |
@sethaxen Hmm, a bit embarrassing. It took me quite a while to figure out what caused this bug. At some point I even started to doubt how StanSample uses multiple cores and for that reason I briefly switched to using Distributed.jl, but it's not really needed in StanSample.jl, so I removed it again. The real issue was introduced when I switched to JSON input files. For cmdstan input files I need to permute dimensions of arrays and used a very poor construct:
Of course, formulated without an explicit deepcopy(d) (if d is a Dict), this will cause the behavior you noticed. I should have paid attention to that old memory I mentioned above! Thanks again for your help! On a side note, I found it very interesting to see PosterorDB show up in the testing of Stan's new Pathfinder method! StanPathfinder.jl should be merged later this week (I hope, just waiting the 3 days as it is a new package). By the way, StanIO.jl is also done. In StanSample.jl that approach is used in output_format |
Ah, okay, makes sense! Thanks again for the fix!
Ah, nice, thanks for adding this! There's also Pathfinder.jl, which supports Stan models via StanLogDensityProblems.jl. Having a StanPathfinder will allow benchmark comparisons between the two (for another time).
Thanks, I'll take a look when I next get back to thinking about conversions to |
Hi Seth, thanks for the link to Pathfinder.jl. Just FYI, I did add a StanPathfinder.jl a few weeks ago. |
Here
L_Psi
is a valid lower Cholesky factor. If we annotate it as acholesky_factor_cov
and sample, everything works fine the 1st time and every odd time but errors every even time:The error seems to indicate that the upper triangle is being filled with the entries in the lower triangle. I would normally thing this was a cmdstan issue, but it runs fine in cmdstanr.
Environment
The text was updated successfully, but these errors were encountered: