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

Vignette prior predictive #465

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

lauken13
Copy link
Collaborator

Hi Jonah,

Just wanted to follow up with this vignette, is it still useful?

@jgabry
Copy link
Member

jgabry commented Aug 26, 2020

Thanks for reminding me! Somehow this wasn't on my to-do list anymore but should have been. I'll try to spend a bit of time on it ASAP. I think we should include it in the next rstanarm release.

@jgabry
Copy link
Member

jgabry commented Aug 26, 2020

Is it ok if I make edits/additions directly or do you prefer that I just make comments?

@lauken13
Copy link
Collaborator Author

lauken13 commented Aug 26, 2020 via email

@lauken13 I should have already done this after we merged the MRP vignette but looks like I forgot, sorry!
@jgabry
Copy link
Member

jgabry commented Aug 26, 2020

Ok cool will do!

@jgabry
Copy link
Member

jgabry commented Aug 27, 2020

@lauken13 I made a bunch of edits to the first half (it looks like I changed more than I actually did because some changes are just reflowing lines). I'll go through the second half (the part about priors for the slope) soon.

A few general things that I noticed:

  • Because rstanarm doesn't list dplyr or tidyr in the DESCRIPTION file we'll need to either not use dplyr and tidyr or do something similar to the MRP vignette where we pre-processed anything that needed those packages. Alternatively we can add them to the Suggests field in the DESCRIPTION file if @bgoodri is OK with it.

  • We might want to consider adding another example that isn't logistic regression. It occurred to me that logistic regression is actually a special case where it makes more sense to look at the latent Pr(y = 1) instead of actual values of y simulated from the bernoulli using Pr(y = 1). Because we don't actually simulate outcomes this example technically doesn't use the prior predictive distribution, which could be confusing. I tried to mention this in some of my edits, but it's still a bit weird not to have an actual example of drawing from the predictive distribution. What do you think?

@lauken13
Copy link
Collaborator Author

lauken13 commented Aug 27, 2020 via email

@jgabry
Copy link
Member

jgabry commented Aug 27, 2020

Yeah I think that sounds like a good idea. Do you have any suggestions for other examples? Normal linear regression?

Yeah I think linear regression would good. I don't think we need anything too complicated, just something to illustrate using posterior_predict() instead of posterior_epred().

I'd be fine to redo the data manipulation with base R, if that makes more sense for the package!

If that's not too much of a pain that would be great!

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.

2 participants