-
-
Notifications
You must be signed in to change notification settings - Fork 407
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 weight_predictions function #2147
Conversation
I think this is a fantastic idea. I always found
`pm.sample_posterior_predictive_w` awkward.
One check that I think we need is something ensuring all models have
identical observed data.
…On Sat, 29 Oct 2022, 15:04 Osvaldo A Martin, ***@***.***> wrote:
This function will take a list of idatas with posterior_predictive groups
and a list of model weights (computed using az.compare or something else)
and it will return a new inference data with a posterior_predictive group
composed of weighted samples from the input idatas.
@zaxtax <https://github.com/zaxtax> maybe we can focus on this and forgot
about pm.sample_posterior_predictive_w
- Follows official
<https://github.com/arviz-devs/arviz/blob/main/CONTRIBUTING.md#pull-request-checklist>
PR format
- Includes a sample plot to visually illustrate the changes (only for
plot-related functions)
- New features are properly documented (with an example if
appropriate)?
- Includes new or updated tests to cover the new feature
- Code style correct (follows pylint and black guidelines)
- Changes are listed in changelog
<https://github.com/arviz-devs/arviz/blob/main/CHANGELOG.md#v0xx-unreleased>
------------------------------
You can view, comment on, or merge this pull request online at:
#2147
Commit Summary
- b9a599b
<b9a599b>
add weight_predictions
File Changes
(1 file <https://github.com/arviz-devs/arviz/pull/2147/files>)
- *M* arviz/stats/stats.py
<https://github.com/arviz-devs/arviz/pull/2147/files#diff-f1da0a2c4f56d6fa32279f759e04b4b1057a1266dc36f94d22347e5111a03bdc>
(24)
Patch Links:
- https://github.com/arviz-devs/arviz/pull/2147.patch
- https://github.com/arviz-devs/arviz/pull/2147.diff
—
Reply to this email directly, view it on GitHub
<#2147>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAACCUKXHQGE7MRCB3ZLM2DWFUOFVANCNFSM6AAAAAARRZAIRA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Should we just remove sample_posterior_predictive_w or throw an error
pointing people to this function?
…On Sat, 29 Oct 2022, 16:16 Rob Zinkov, ***@***.***> wrote:
I think this is a fantastic idea. I always found
`pm.sample_posterior_predictive_w` awkward.
One check that I think we need is something ensuring all models have
identical observed data.
On Sat, 29 Oct 2022, 15:04 Osvaldo A Martin, ***@***.***>
wrote:
> This function will take a list of idatas with posterior_predictive
> groups and a list of model weights (computed using az.compare or
> something else) and it will return a new inference data with a
> posterior_predictive group composed of weighted samples from the input
> idatas.
>
> @zaxtax <https://github.com/zaxtax> maybe we can focus on this and
> forgot about pm.sample_posterior_predictive_w
>
> - Follows official
> <https://github.com/arviz-devs/arviz/blob/main/CONTRIBUTING.md#pull-request-checklist>
> PR format
> - Includes a sample plot to visually illustrate the changes (only for
> plot-related functions)
> - New features are properly documented (with an example if
> appropriate)?
> - Includes new or updated tests to cover the new feature
> - Code style correct (follows pylint and black guidelines)
> - Changes are listed in changelog
> <https://github.com/arviz-devs/arviz/blob/main/CHANGELOG.md#v0xx-unreleased>
>
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
> #2147
> Commit Summary
>
> - b9a599b
> <b9a599b>
> add weight_predictions
>
> File Changes
>
> (1 file <https://github.com/arviz-devs/arviz/pull/2147/files>)
>
> - *M* arviz/stats/stats.py
> <https://github.com/arviz-devs/arviz/pull/2147/files#diff-f1da0a2c4f56d6fa32279f759e04b4b1057a1266dc36f94d22347e5111a03bdc>
> (24)
>
> Patch Links:
>
> - https://github.com/arviz-devs/arviz/pull/2147.patch
> - https://github.com/arviz-devs/arviz/pull/2147.diff
>
> —
> Reply to this email directly, view it on GitHub
> <#2147>, or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAACCUKXHQGE7MRCB3ZLM2DWFUOFVANCNFSM6AAAAAARRZAIRA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
glad you like it. yeah we are assuming a few things here, we should add some check. We may want to deprecate |
Codecov Report
@@ Coverage Diff @@
## main #2147 +/- ##
==========================================
- Coverage 90.70% 90.67% -0.03%
==========================================
Files 120 120
Lines 12647 12667 +20
==========================================
+ Hits 11471 11486 +15
- Misses 1176 1181 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
Couple of comments
ready for review or merge, depending on how benevolent you are feeling today |
|
Hi @oussamadhaoui, did you check that also for the future, it is usually the best idea to open a new issue ticket instead of adding comments to merged PRs. Check this guide |
This function will take a list of idatas with
posterior_predictive
groups and a list of model weights (computed usingaz.compare
or something else) and it will return a new inference data with aposterior_predictive
group composed of weighted samples from the input idatas.@zaxtax maybe we can focus on this and forgot about
pm.sample_posterior_predictive_w
📚 Documentation preview 📚: https://arviz--2147.org.readthedocs.build/en/2147/