Fix loo/add pp_checks #19

be-green · 2019-09-07T20:26:21Z

Fixes looic, adds standard errors to both ELPD and looic, and tests against the BRMS package's meta-analysis models. It also updates the documentation for the loocv() function.

Previous commits of mine add posterior_predictive checks and a predict function to the package (only implemented for the rubin model thus far).

…t now it makes very strong assumptions about the data structure; this will likely be revisited later. The main changes are everything that has been added in R/predict.R, which implements both the predict and pp_check methods.

…Also added standard errors for both the looic and the ELPD of the model. The documentation has been updated and the print.baggr_cv function reurns more information. There is a testing script that runs the same model in brms and baggr for comparison purposes.

be-green · 2019-09-11T21:40:15Z

FYI, I've added a formal test for LOO, but it looks like the failing test is in the test_mutau section.

wwiecek · 2019-10-04T09:44:32Z

Travis says that the check failed because you have a dependence on brms.

Rather than fitting with a brm() function it's best to save the output object (matrix?) to check against. Otherwise we have 1/ compatibility issue, 2/ are running extra code every time we test, just to get the same output matrix from it, OK?

wwiecek · 2019-10-04T09:47:51Z

R/loocv.R

@@ -18,26 +11,26 @@
 #' full model, prior values and _lpd_ of each model are also returned.
 #' These can be examined by using `attributes()` function.
 #'
+#' @references Gelman, Andrew, et al. Bayesian data analysis. Chapman and Hall/CRC, 2013.


Maybe a reference like this is not the best, i.e. referencing a whole book

wwiecek · 2019-10-04T09:48:57Z

tests/testthat/test_loo.R

+fit <- brm(tau | se(se) ~ 1 + (1 | group),
+           data = schools,
+           control = list(adapt_delta = 0.95),
+           prior = c(set_prior("normal(0,100)", class = "Intercept"),
+                     set_prior("uniform(0, 104.44)", class = "sd")),
+           file = "misc/brms_kfold_test")
+


See my comment elsewhere

wwiecek · 2019-10-04T09:56:09Z

If you could just make sure that

CMD check runs on your side
Travis does not fail
your edits are rebased to devel (I think they are, but I find github interface very confusing so thx if you can double check)

I will then review locally

be-green · 2019-10-04T11:45:57Z

I'll make these changes and push them. Thanks for the comments. I'll keep the function in there but commented out in case we want to re-run later? I can just save the kfold output object, it should be pretty small.

wwiecek · 2019-10-04T11:51:45Z

Yes, that's exactly what I had in mind. I think testthat has a lot of functionality for testing against predefined objects, maybe you can quickly check what is Hadley's recommendation? I never used it.

…

On Fri, Oct 4, 2019 at 12:45 PM be-green ***@***.***> wrote: I'll make these changes and push them. Thanks for the comments. I'll keep the function in there but commented out in case we want to re-run later? I can just save the kfold output object, it should be pretty small. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#19?email_source=notifications&email_token=AE5QPJSQWV7LQW7HW4B27W3QM4UHLA5CNFSM4IURGMIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEALMQEI#issuecomment-538363921>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE5QPJUZY2P34ZKOKOID423QM4UHLANCNFSM4IURGMIA> .

wwiecek · 2019-10-04T11:54:57Z

PS: Same idea applies to 8 schools check of correctness. You did it recently (and it worked OK) but it's just not included in this pull request, right?

…

On Fri, Oct 4, 2019 at 12:45 PM be-green ***@***.***> wrote: I'll make these changes and push them. Thanks for the comments. I'll keep the function in there but commented out in case we want to re-run later? I can just save the kfold output object, it should be pretty small. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#19?email_source=notifications&email_token=AE5QPJSQWV7LQW7HW4B27W3QM4UHLA5CNFSM4IURGMIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEALMQEI#issuecomment-538363921>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE5QPJUZY2P34ZKOKOID423QM4UHLANCNFSM4IURGMIA> .

be-green · 2019-10-04T12:06:12Z

Yes, I can add it in. I didn't use pre-specified values since I didn't have BDA3, I just used BRMS to fit the same model with the same priors and compared. I can also include that check.

be-green · 2019-10-04T12:14:35Z

It might just be my local machine, but I've removed the references to BRMS and the tests seem to fail here:

rather than at the test_loo stage.

…t now it makes very strong assumptions about the data structure; this will likely be revisited later. The main changes are everything that has been added in R/predict.R, which implements both the predict and pp_check methods.

…Also added standard errors for both the looic and the ELPD of the model. The documentation has been updated and the print.baggr_cv function reurns more information. There is a testing script that runs the same model in brms and baggr for comparison purposes.

be-green · 2019-10-04T13:29:05Z

Alright, I've added everything in, made sure to rebase on your devel branch, and I'm going to push as soon as devtools::check passes locally. The error I'm running into is a compilation error, something about "did not create dll". @wwiecek have you ever seen that before?

wwiecek · 2019-10-04T14:59:27Z

That usually means that you didn't unload the dynamic library. R is trying to overwrite the dll file but is not allowed to do it. Make sure you don't have multiple R sessions running, restart R or RStudio prior to running checks.

…

On Fri, 4 Oct 2019, 14:29 be-green, ***@***.***> wrote: Alright, I've added everything in, made sure to rebase on your devel branch, and I'm going to push as soon as devtools::check passes locally. The error I'm running into is a compilation error, something about "did not create dll". @wwiecek <https://github.com/wwiecek> have you ever seen that before? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19?email_source=notifications&email_token=AE5QPJXBUVNGKRFJM6Y6O6TQM5AKDA5CNFSM4IURGMIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEALUQ6I#issuecomment-538396793>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE5QPJUJ2M2AFB4WPVPEWMTQM5AKDANCNFSM4IURGMIA> .

wwiecek · 2019-10-04T15:00:13Z

Try checking which test fails and met me know!

…

On Fri, 4 Oct 2019, 13:14 be-green, ***@***.***> wrote: It might just be my local machine, but I've removed the references to BRMS and the tests seem to fail here: [image: image] <https://user-images.githubusercontent.com/23174585/66206729-901f2380-e676-11e9-8445-b70ef824b001.png> rather than at the test_loo stage. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#19?email_source=notifications&email_token=AE5QPJXWREI24BOVNI3YNBTQM4XSXA5CNFSM4IURGMIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEALOPDQ#issuecomment-538371982>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE5QPJVUL7JB2UZQWFTSZQTQM4XSXANCNFSM4IURGMIA> .

be-green · 2019-10-04T19:32:38Z

The first mu-tau test consistently crashes my RStudio for some reason. I think I may be having build issues that are as-of-yet diagnosed, so I'll let travis decide what's working.

wwiecek · 2019-10-07T16:37:48Z

Travis failed again at brms, can you just push the test without brms so that Travis can check for any errors elsewhere?

The first mu-tau test consistently crashes my RStudio for some reason. I think I may be having build issues that are as-of-yet diagnosed, so I'll let travis decide what's working.

The first check that I see is expect_error(baggr(df_mutau, "made_up_model"), "Unrecognised model"), is that the one?
Does model="mutau" work at all when you devtools::load_all()?

be-green · 2019-10-07T22:48:39Z

Looks like all the errors are fixed in travis, still some warnings though. I'll check the devtools::load_all with the mutau model. Again, the package is still not building via cmd check on my machine (even running it via RScript and no open interactive sessions) so I think it must be an issue with my compiler or something. I had a similar issue with a different model at one point that no one else could replicate.

wwiecek · 2019-10-08T12:39:11Z

Looks like all the errors are fixed in travis, still some warnings though. I'll check the devtools::load_all with the mutau model. Again, the package is still not building via cmd check on my machine (even running it via RScript and no open interactive sessions) so I think it must be an issue with my compiler or something. I had a similar issue with a different model at one point that no one else could replicate.

Thanks, this looks OK! I flagged two check NOTEs/WARNINGs FYI, but no need to fix - I will edit myself.

wwiecek · 2019-10-08T12:35:49Z

R/predict.R

 predict.baggr <- function(x, newdata = NULL,
-                          allow_new_levels = T, nsamples, ...) {
+                          allow_new_levels = T, nsamples = 100, ...) {
  switch(x$model,
         rubin = predict_rubin(x, newdata = newdata,
                               allow_new_levels = allow_new_levels,


* checking S3 generic/method consistency ... WARNING predict: function(object, ...) predict.baggr: function(x, newdata, allow_new_levels, nsamples, ...) See section ‘Generic functions and methods’ in the ‘Writing R Extensions’ manual.

wwiecek · 2019-10-08T12:36:33Z

R/predict.R

 pp_check.baggr <- function(x, type = "dens_overlay", nsamples = 40) {
  pp_fun <- getFromNamespace(paste0("ppc_",type),ns = "bayesplot")


pp_check.baggr: no visible global function definition for ‘getFromNamespace’ Undefined global functions or variables: getFromNamespace Consider adding importFrom("utils", "getFromNamespace")

wwiecek · 2019-10-11T16:49:31Z

I reviewed this -- I need to rethink the predict functions, add extra functionality and write the full documentation. Therefore I pushed to a new devel-ppcheck branch. I will let you know once I'm done.

I also rebased your code & made various small fixes, I think now it will pass checks both locally & on Travis.

be-green · 2019-10-11T17:25:49Z

@wwiecek when you have an idea of what you'd like to change/implement, let me know so I can help. Happy to write documentation or change the api.

be-green added 10 commits July 1, 2019 16:20

Update docs

4bc7deb

Merge branch 'master' of https://github.com/be-green/baggr

980452d

commit local changes for prepare_ma

c1cc040

Update to be in sync

0f17b7d

update docs

7e7072f

bringing remote branch up to date

6bd6e34

Fix needless namespace dependence.

26a7800

wwiecek reviewed Oct 4, 2019

View reviewed changes

be-green added 8 commits October 4, 2019 07:47

Update docs

7677c50

update docs

29b572e

bringing remote branch up to date

b9e9b62

Fix needless namespace dependence.

d70004e

updating loo and estimate testing

616f7dd

be-green added 2 commits October 4, 2019 14:07

update namespace and documentation for loo

f65ae29

fix merge conflict

e86b9f5

wwiecek mentioned this pull request Oct 7, 2019

Prior and posterior predictive functions #20

Closed

update tests

0734092

wwiecek reviewed Oct 8, 2019

View reviewed changes

break out 8schools as a separate example

46b7980

wwiecek closed this Oct 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix loo/add pp_checks #19

Fix loo/add pp_checks #19

be-green commented Sep 7, 2019

be-green commented Sep 11, 2019

wwiecek commented Oct 4, 2019

wwiecek Oct 4, 2019

wwiecek Oct 4, 2019

wwiecek commented Oct 4, 2019

be-green commented Oct 4, 2019

wwiecek commented Oct 4, 2019 via email

wwiecek commented Oct 4, 2019 via email

be-green commented Oct 4, 2019

be-green commented Oct 4, 2019

be-green commented Oct 4, 2019

wwiecek commented Oct 4, 2019 via email

wwiecek commented Oct 4, 2019 via email

be-green commented Oct 4, 2019

wwiecek commented Oct 7, 2019

be-green commented Oct 7, 2019

wwiecek commented Oct 8, 2019

wwiecek Oct 8, 2019

wwiecek Oct 8, 2019

wwiecek commented Oct 11, 2019

be-green commented Oct 11, 2019

		pp_check.baggr <- function(x, type = "dens_overlay", nsamples = 40) {
		pp_fun <- getFromNamespace(paste0("ppc_",type),ns = "bayesplot")

Fix loo/add pp_checks #19

Fix loo/add pp_checks #19

Conversation

be-green commented Sep 7, 2019

be-green commented Sep 11, 2019

wwiecek commented Oct 4, 2019

wwiecek Oct 4, 2019

Choose a reason for hiding this comment

wwiecek Oct 4, 2019

Choose a reason for hiding this comment

wwiecek commented Oct 4, 2019

be-green commented Oct 4, 2019

wwiecek commented Oct 4, 2019 via email

wwiecek commented Oct 4, 2019 via email

be-green commented Oct 4, 2019

be-green commented Oct 4, 2019

be-green commented Oct 4, 2019

wwiecek commented Oct 4, 2019 via email

wwiecek commented Oct 4, 2019 via email

be-green commented Oct 4, 2019

wwiecek commented Oct 7, 2019

be-green commented Oct 7, 2019

wwiecek commented Oct 8, 2019

wwiecek Oct 8, 2019

Choose a reason for hiding this comment

wwiecek Oct 8, 2019

Choose a reason for hiding this comment

wwiecek commented Oct 11, 2019

be-green commented Oct 11, 2019