-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Keep broadcasting information in make_shared_replacements #4492
Conversation
Nice, that's an easy fix. Can you add a test that fails before and passes now with this? |
Sure! I'll take care of it in the next days :) |
Hi @ExpectationMax ! |
@twiecki I think everything should be ready from my side :) |
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.
I added two small suggestions - @ExpectationMax @twiecki please consider them before merging :)
It seems like broadcasting information gets lost when applying `pm.make_shared_replacements`, leading to problems with the metropolis sampler. Potentially related issues below: - #1083 - #1304 - #1983 This fix was previously suggested in the following issue: - #3337 It could be that further adaptations are necessary as indicated in the issue. Strangely, this does not seem to lead to problems when using NUTS.
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.
LGTM, I don't expect the changes to have any meaningful side-effects and seems to solve the known issues.
Thanks @ExpectationMax |
It seems like broadcasting information gets lost when applying
pm.make_shared_replacements
, leading to problems with the metropolissampler. Potentially related issues below:
This fix was previously suggested in the following issue:
It could be that further adaptations are necessary as indicated in the
issue. Strangely, this does not seem to lead to problems when using
NUTS.
This fixes issue #4491 and #3337
TODOs: