Skip to content

Update Posterior Predictive Checks Notebook #3857

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

Merged
merged 7 commits into from
Mar 30, 2020

Conversation

AlexAndorra
Copy link
Contributor

The Posterior Predictive Checks notebook on the website uses PyMC 3.6 and doesn't use ArviZ. As this is a favorite topic of questions on Discourse and elsewhere, I thought it'd be useful to update and extend it.

I made the following changes:

  • Use latest PyMC and ArviZ
  • Use regression example for PPC section
  • Added prior predictive checks and updates posterior checks for PPC section
  • Updated "Prediction" section

Thanks in advance for the review, and let me know if anything needs edits ✌️

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

@rpgoldman
Copy link
Contributor

I'll have a look. As a PyMC3 user, one thing I find quite difficult is that some of the distributions do not support predictive sampling and, worse, just generate garbage instead of erroring out. This might be worth addressing with a caution.

@codecov
Copy link

codecov bot commented Mar 27, 2020

Codecov Report

Merging #3857 into master will decrease coverage by 0.30%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3857      +/-   ##
==========================================
- Coverage   90.75%   90.45%   -0.31%     
==========================================
  Files         135      135              
  Lines       21184    21184              
==========================================
- Hits        19226    19161      -65     
- Misses       1958     2023      +65     
Impacted Files Coverage Δ
pymc3/tests/models.py 70.24% <0.00%> (-15.71%) ⬇️
pymc3/tests/test_step.py 93.20% <0.00%> (-6.80%) ⬇️
pymc3/step_methods/hmc/base_hmc.py 93.45% <0.00%> (-1.87%) ⬇️
pymc3/tests/test_distributions.py 96.42% <0.00%> (-1.73%) ⬇️
pymc3/tests/test_transforms.py 98.52% <0.00%> (-1.48%) ⬇️
pymc3/tests/test_mixture.py 98.95% <0.00%> (-0.70%) ⬇️
pymc3/distributions/continuous.py 79.69% <0.00%> (-0.41%) ⬇️

@AlexAndorra
Copy link
Contributor Author

Thanks @rpgoldman !
Do you understand how changing this NB can have an impact on tests though?

@rpgoldman
Copy link
Contributor

Thanks @rpgoldman !
Do you understand how changing this NB can have an impact on tests though?

I'm afraid not. To be honest, those codecov outputs are useless to me because I don't know how to interpret them, and they are very "twitchy" -- touching arbitrary bits of the repo seem to make them swing in unpredictable ways.

@AlexAndorra
Copy link
Contributor Author

AlexAndorra commented Mar 27, 2020

Yeah, I feel the same... The Travis tests passed, so I guess it's the most important 🤷‍♂
I really don't know if the failing test causes problems for merging though.

@rpgoldman
Copy link
Contributor

Yeah, I feel the same... The Travis tests passed, so I guessed it's the most important 🤷‍♂
I really don't know if the failing test causes problems for merging though.

I don't believe they do in the sense that there won't be conflicts. I have no idea what they do to the test results of master. I think @ahartikainen has been wrestling with these tests.

@rpgoldman
Copy link
Contributor

@twiecki @AlexAndorra I'll check this out and try to do a quick pass over it in my repo and give you a PR back. Not sure I will get this done today, though.

@AlexAndorra
Copy link
Contributor Author

Thanks @rpgoldman ! If it's easier for you though, you can just make your comments in ReviewNB -- we do that on the "Ressources" repo and it works well 👌
Regarding distributions not supported by predictive sampling, I think it'd indeed be interesting to add a note. But let's bear in mind that these are usually more complex distributions -- the goal of this NB is to display predictive sampling for most, simple distributions.

@rpgoldman
Copy link
Contributor

Thanks @rpgoldman ! If it's easier for you though, you can just make your comments in ReviewNB -- we do that on the "Ressources" repo and it works well 👌

I was looking for a way to do that, and I couldn't figure out how. I could comment on it, but if I just wanted to tweak a paragraph, I couldn't.

Editing text sometimes is just easier to edit than to describe the edits!

Regarding distributions not supported by predictive sampling, I think it'd indeed be interesting to add a note. But let's bear in mind that these are usually more complex distributions -- the goal of this NB is to display predictive sampling for most, simple distributions.

I understand, but that runs the risk of letting users (like me!) think that one can follow the model design - prior predictive evaluation - find posterior - evaluate posterior by sampling process when PyMC3 only fully supports finding the posterior.

@AlexAndorra
Copy link
Contributor Author

I could comment on it, but if I just wanted to tweak a paragraph, I couldn't.

Yeah, I think you can't do that.

I understand, but that runs the risk of letting users (like me!) think that one can follow the model design

Well, I'd say you can for a substantial number of distributions. When you can't do it out-of-the-box, then you have to push the parameters through the model yourself -- which can get quite complicated, indeed.

@ahartikainen
Copy link
Contributor

What tests fail? Codecov? Ignore it for now, I think there is some failure what is compared to what (in arviz repo too).

(It compares latests commits?)

@twiecki
Copy link
Member

twiecki commented Mar 30, 2020

Just took a look through ReviewNB and this is much improved. Many thanks @AlexAndorra!

@twiecki twiecki merged commit e21191f into pymc-devs:master Mar 30, 2020
@AlexAndorra AlexAndorra deleted the update-ppc-nb-2 branch May 13, 2020 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants