-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add helper to compute log_likelihood
and stop computing it by default
#6374
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
Add helper to compute log_likelihood
and stop computing it by default
#6374
Conversation
log_likelihood
and stop computing it by default
041c705
to
65b8bf0
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6374 +/- ##
==========================================
+ Coverage 94.71% 94.76% +0.04%
==========================================
Files 132 133 +1
Lines 26771 26825 +54
==========================================
+ Hits 25357 25421 +64
+ Misses 1414 1404 -10
|
65b8bf0
to
6b60804
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.
I would not call the file model comparison. It is not doing any model comparison, it is only an intermediate result that can be used for that, and it is also not specific to model comparison, it is also used in loo_pit which is model checking similar to plot_ppc.
To add to docs, list is somewhere in https://github.com/pymc-devs/pymc/tree/main/docs/source/api. It doesn't really matter if it is "the wrong place" as #5282 is still open and the API docs need significant work.
Most important thing is the comment on dims and coords. If they are indeed preserved, we should add a test for that, if they are not we should fix it and add a test for that.
6b60804
to
c14ad14
Compare
c14ad14
to
f10952a
Compare
f10952a
to
888fe7b
Compare
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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, thanks!
Closes #6266
Not sure about the function placement...
Major / Breaking Changes
sample
no longer returns thelog_likelihood
group by default. Explicitly setidata_kwargs={"log_likelihood": True}
or call the helpercompute_log_likelihood(idata)
after sampling. This is needed when usingarviz.compare