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

Update vague prior for normal vignette #66

Closed
abiterry opened this issue Oct 24, 2024 · 2 comments · Fixed by #93
Closed

Update vague prior for normal vignette #66

abiterry opened this issue Oct 24, 2024 · 2 comments · Fixed by #93
Assignees

Comments

@abiterry
Copy link
Collaborator

@nwbean #65

Vague prior is mentioned on lines

  • The prior and the external control SD are optional. If no prior is provided, an improper uniform prior will be used for the initial prior; i.e., $\pi(\theta_C) \propto 1$. If no external control SD or initial prior are specified (i.e., both the `prior` and `external_sd` arguments set as `NULL`), then a non-standardized $t$ power prior will be created (not covered in this vignette). In this example, we define the initial prior to be a vague normal distribution with a mean 50 and SD 10.
  • prior = dist_normal(50, 10),
  • vague <- dist_normal(50, 10)
  • prior = dist_normal(50, 10),
@nwbean
Copy link
Collaborator

nwbean commented Oct 24, 2024

Let's change the mean from 50 to 0.50 which should better correspond to the data.

Additionally, can we change the following items:

  • In the DATASET.R file, change the seed from 123 to 12345. We simulate the data to have a trt effect of about 0.1, but with the current seed, the data by chance have little treatment effect (and the internal control mean response is pretty different from the external control). The seed 12345 should simulate data for the normal endpoint case that better illustrates the methods in the vignette.

  • In the utils.R file, change the plot_dist() function to have a default alpha of 0.5. With the current default option, plots that include multiple priors/posteriors are difficult to read due to lack of transparency in the overlapping distributions.

@abiterry
Copy link
Collaborator Author

Following our discussion, I've left the mean as is for the moment and just removed the vague prior from the plot but made the other changes.

abiterry added a commit that referenced this issue Oct 28, 2024
statasaurus added a commit that referenced this issue Oct 28, 2024
@statasaurus statasaurus linked a pull request Dec 17, 2024 that will close this issue
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 a pull request may close this issue.

3 participants