Skip to content
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

Deprecate sample_posterior_predictive_w #6254

Merged

Conversation

zaxtax
Copy link
Contributor

@zaxtax zaxtax commented Oct 29, 2022

This PR is contingent on arviz-devs/arviz#2147 being merged. But I think arviz.stats.weight_predictions is a better API for model averaging than pm.sample_posterior_predictive_w

The main thing that's need in addition is to update the notebook to use the arviz function, and another PR to update examples that used to use pm.sample_posterior_predictive_w

Checklist

Major / Breaking Changes

Docs / Maintenance

  • No Docs need to change

@codecov
Copy link

codecov bot commented Oct 29, 2022

Codecov Report

Merging #6254 (9f39081) into main (bcffce2) will increase coverage by 3.56%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6254      +/-   ##
==========================================
+ Coverage   90.57%   94.14%   +3.56%     
==========================================
  Files         100      100              
  Lines       21235    21235              
==========================================
+ Hits        19234    19992     +758     
+ Misses       2001     1243     -758     
Impacted Files Coverage Δ
pymc/backends/base.py 87.65% <0.00%> (-0.83%) ⬇️
pymc/parallel_sampling.py 87.03% <0.00%> (+0.34%) ⬆️
pymc/distributions/distribution.py 95.00% <0.00%> (+0.45%) ⬆️
pymc/aesaraf.py 93.54% <0.00%> (+2.32%) ⬆️
pymc/tests/distributions/util.py 90.86% <0.00%> (+2.46%) ⬆️
pymc/distributions/logprob.py 97.26% <0.00%> (+3.42%) ⬆️
pymc/tuning/starting.py 92.50% <0.00%> (+12.50%) ⬆️
pymc/tuning/scaling.py 62.74% <0.00%> (+25.49%) ⬆️
pymc/distributions/dist_math.py 87.42% <0.00%> (+29.71%) ⬆️
pymc/distributions/transforms.py 100.00% <0.00%> (+31.42%) ⬆️
... and 5 more

Copy link
Member

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the function raised a NotImplementedError beforehand, I think we can merge without waiting on the ArviZ release.

What do you think, @aloctavodia ?

@aloctavodia
Copy link
Member

Sounds good to me.

@michaelosthege michaelosthege merged commit 9105d74 into pymc-devs:main Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants