New vignette: exemplary comparison of Bayesian and frequentist MMRM fits #71
Conversation
…usethis::use_tidy_description()
… done in the vignette)
| @@ -0,0 +1,189 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Would it be okay to move files asa.csl and bibliography.bib to the vignettes folder?
There was a problem hiding this comment.
Or, does having them in inst/ help with the citations in Rd files too?
There was a problem hiding this comment.
If we had references only in vignettes, I would prefer the vignettes/ folder. However, since we use references elsewhere in the package documenation too, I suggest to use inst/ as a central location. This is also how it was done in the mmrmpackage, see here.
vignettes/comparison.Rmd
Outdated
|
|
||
| ## Pre-processing | ||
|
|
||
| The `fev_dat` dataset is used that is contained in `{mmrm}`: |
There was a problem hiding this comment.
Might be nice to include a short description of the dataset, e.g. the "Note" in https://openpharma.github.io/mmrm/latest-tag/reference/fev_data.html.
Also, I see on that page:
This is an artificial dataset.
Would be nice to use a real clinical dataset in addition.
There was a problem hiding this comment.
I agree. Other datasets in the end did not convince me as good (educational) examples, and I also wanted to avoid further dependencies. OK for you to leave it with fev_data for now and add or switch to another dataset later, perhaps one from clindata?
There was a problem hiding this comment.
A short description has now been added (1a28cb7). OK to add or switch to another dataset later, perhaps one from the clindata-package? The current alternatives weren't convincing for me as good (educational) examples and I also wanted to avoid further dependencies, this is why I sticked to the mmrm-package.
There was a problem hiding this comment.
Thanks @chstock. I like your idea to align with the mmrm package. Indeed, I see a lot of work at https://openpharma.github.io/mmrm/latest-tag/articles/mmrm_review_methods.html which uses both the FEV and BCVA datasets. In the end, I think it would be sufficient to analyze both and refer readers to the results from https://openpharma.github.io/mmrm/latest-tag/articles/mmrm_review_methods.html#marginal-treatment-effect-estimates-comparison. But that could be a future PR.
vignettes/comparison.Rmd
Outdated
|
|
||
| # About {.unnumbered} | ||
|
|
||
| This vignette provides an exemplary comparison of a Bayesian MMRM fit, |
There was a problem hiding this comment.
It this point, it might be good to refer readers to the section at the very bottom, where the differences between models are shown parameter by parameter.
There was a problem hiding this comment.
Agree, this has now been addressed in 6fb042a.
|
Thanks for the review, @wlandau! I believe I have addressed all comments and the PR should be ready for another round of review. |
|
Looks great, @chstock. I think this PR is in great shape for a merge. Going forward, we could think about aligning with #72 and discussing https://openpharma.github.io/mmrm/latest-tag/articles/mmrm_review_methods.html. |
This is a new vignette with an exemplary comparison of Bayesian and frequentist MMRM fits as suggested in issue #68.
I used the the FEV1 dataset (
fev_data) from the{mmrm}package. The endpoint is 'change from baseline in FEV1'. For different reasons, the alternative datasets we had identified appeared not as convincing. We may decide to change the dataset later. Alternatively, we could (later) use a dataset from the new{clindata}package or a simulated one based on realistic parameters as described e.g. here. However, this does not seem urgent or necessary from my point of view. I adapted the code used in the analysis of theorthodontdataset (#66).Please let me know what you think.