-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Automatic marginalization of finite discrete variables in the logp #91
Conversation
a21e1f8
to
2ca5025
Compare
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
c19a96f
to
afd4740
Compare
1f0c87e
to
4f10d49
Compare
9827aab
to
3f1d0f4
Compare
d049eb9
to
75cb830
Compare
75cb830
to
effce62
Compare
This is ready for review, I decided to leave the posterior recovery of marginalized RVs for a subsequent PR as this one is already big as is |
effce62
to
5f89325
Compare
5f89325
to
2fd36bc
Compare
Need an approving review :) |
2fd36bc
to
c0f622c
Compare
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 to me.
Added a Model transformation, to automatically marginalize discrete variables from the logp graph.
I first attempted to modify the original model in-place so that the
FiniteDiscreteMarginalRVs
are introduced in the model RV graph, but this led to many issues, in theinitial_point
,step sampler assignment
and so on, which don't really need to know about the modified logp graph.My latest approach is to clone the underlying model, modify that in place and request its logp, everytime the logp of the user model is requested.
TODO
Dispatch default_transform and momentFix graphviz for marginalized RVsFix string representation for marginalized RVs