dev #176
dev #176
Conversation
|
I really like what you did in example 2! I'll go over it tomorrow (my tomorrow?) and polish the BF stuff a bit. I'm not sure how I feel about removing BFs from the reporting guidelines... Now that I think about it, it might be useful (also for the JOSS submission) to re-structured it into 2 or 3 parts (just like there are two vignettes: estimation / inference):
I think regardless of what we might find in the upcoming paper, we should give our users guidelines to reporting the common indices (that we provide the methods for) - this includes the BF. As you say yourself in the Bayes factor vignette:
|
Just ran it successfully on my end :/ |
well played, well played ^^ I am indeed conflicted on this one. As much as I like the idea of neutrally providing methods and explanations for all indices, I also think some of them are superior (depending on the context of course). and I would like to help the beginners by guiding them into what to report if they have no strong opinions. Of course, an experimented user that understands the stuff can decide to do what is appropriate for its case. As I am writing this, in fact, I changed my mind: the BF should definitely be included as it is the only index that can be used in other useful contexts, such as model comparison. The guidelines need to be restructured. Maybe as follows (as it is implied currently it the README and the JOSS paper):
As the BF can be used to compare models (literally models or "data generation processes", i.e., hypotheses), I feel like it deserves a separate entry (as it goes beyond simply models' parameters description, - the context for which its use is prone to important considerations). |
Codecov Report
@@ Coverage Diff @@
## master #176 +/- ##
=======================================
Coverage 70.44% 70.44%
=======================================
Files 40 40
Lines 1925 1925
=======================================
Hits 1356 1356
Misses 569 569
Continue to review full report at Codecov.
|
|
My bad, the issue seems to lie in the plotting of pd with the new library(rstanarm)
library(bayestestR)
library(see)
data <- iris # Use the iris data
model <- stan_glm(Sepal.Length ~ Sepal.Width, data=data, refresh=0) # Fit model
plot(p_direction(model))
#> Error in if (ncol(data) > 1) {: argument is of length zeroCreated on 2019-06-18 by the reprex package (v0.3.0) @strengejacke any quick thoughts? |
|
I'll read the vignettes closely once we merged this PR and updated the website, to give my feedback on this and the JOSS draft. |
|
I think posterior estimation (median, hdi) should have their own section, as they are very "descriptive" (as opposed to lending themselves directly to inference). Thinking aloud here: Hypothesis testing frameworkpd is a measure of existence based only on the posterior - it is the maximal percent of the posterior that is on some side of zero.
ROPE is a measure of significance that is based on the posterior and on some pre-conceived notion of a "small" effect.
BF is a relative measure of evidence. In the case where one model is the point null, it tests the relative probability of the data between the models.
(there's also the p-MAP, which isn't getting much love by us...) What is common between the indices
I think the main vignette and guidelines should be along one or more of these ^ lines... |
This perspective is really interesting. It would be interesting to formalize it and develop it even further. Maybe starting with a blogpost? "Hypothesis testing in the Bayesian framework". Such conceptualisation could potentially also be integrated as a paragraph in the intro of the significance paper. (for pd I think h0 is the smaller side and h1 is the most probable side)
We are waiting for the feedback of @tszanalytics (Prof Jeff Mills; from which this index was inspired (and surely distorted by my poor understanding 😬)), but unfortunately, he's quite busy these days I'll re-add BF in the guidelines, and will bump to 0.2.5 (or do we keep 0.2.1?). Then we go over a general check and we can submit the fix to CRAN. Then for v3 we will start working seriously on the guidelines and all that comes before. |
|
So when is this PR ready for merging? |
|
Hopefully in a few hours 😅 |
This would make the hypothesis a post-hoc one, no? From my understanding, the pd is a measure of certainty - how certain we are that theta is not 0 (by testing how probable is the most probable sign of theta). But we can shelf this discussion for when working on v3. |
|
|
@mattansb I have one error in BF SD for BRMs: This test is skipped on travis and CRAN, but still :) |
|
@strengejacke @mattansb do we keep the version at 0.2.1 or do we arbitrarily bump to something else (0.2.5?) |
|
I always prefer to set the version number for CRAN higher than the GitHub version number. Else, users who installed from GitHub in between, might miss the next "official" CRAN release ("update packages" in RStudio would ignore the CRAN version, if it has no higher number than the installed package) that probably already has further fixes and improvements. Thus, looking at the news, we might increase to 0.2.x, higher than 0.2.1, and anything that looks good (0.2.5, as you suggested). |
Agreed. 0.2.2 sounds interesting and is logical (since we are currently at 0.2.1. What do you say? |
|
We can merge this whenever the BF SD error for BRMS is addressed/dismissed. We can then check the documentation/vignettes, and once I have your greenlights I'll build the 0.2.2 and submit. |
|
Agree about the version number
…--
Mattan S. Ben-Shachar, PhD student
Department of Psychology & Zlotowski Center for Neuroscience
Ben-Gurion University of the Negev
The Developmental ERP Lab
On Tue, Jun 18, 2019, 16:19 Dominique Makowski ***@***.***> wrote:
We can merge this whenever the BF SD error for BRMS is
addressed/dismissed. We can then check the documentation/vignettes, and
once I have your greenlights I'll build the 0.2.2 and submit.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#176?email_source=notifications&email_token=AINRP6F4LF4DOUGR7PRMEHTP3DOG7A5CNFSM4HYYG2Z2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX6S5PA#issuecomment-503131836>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AINRP6BY2EL5MFRD2JJOKTTP3DOG7ANCNFSM4HYYG2ZQ>
.
|
|
@DominiqueMakowski Sorry to say that I'm not getting that same error.... 🤔 Can you run it manually by you and see what exactly the error is? |
library(brms)
brms_mixed_6 <- insight::download_model("brms_mixed_6")
set.seed(222)
bfsd <- bayestestR::bayesfactor_savagedickey(brms_mixed_6, effects = "fixed")
#> Computation of Bayes factors: sampling priors, please wait...
#> Error in .update_to_priors.brmsfit(posterior, verbose = verbose): Error in sink(type = "output") : invalid connectionCreated on 2019-06-18 by the reprex package (v0.3.0) 😕 @strengejacke does line 43 of test-BF-sd works at your place? |
|
That must be in some |
|
How come it fails at my place and runs and yours though (I have brms 2.8.9) |
|
I have the most recent from CRAN(not in front of the computer right now). This is just a guess, because I don't call |
|
Let's merge it, so I can check here, too. |
|
works fine for me. library(brms)
#> Loading required package: Rcpp
#> Registered S3 method overwritten by 'xts':
#> method from
#> as.zoo.xts zoo
#> Loading 'brms' package (version 2.9.0). Useful instructions
#> can be found by typing help('brms'). A more detailed introduction
#> to the package is available through vignette('brms_overview').
brms_mixed_6 <- insight::download_model("brms_mixed_6")
set.seed(222)
bfsd <- bayestestR::bayesfactor_savagedickey(brms_mixed_6, effects = "fixed")
#> Computation of Bayes factors: sampling priors, please wait...
bfsd
#> # Bayes Factor (Savage-Dickey density ratio)
#>
#> Parameter Bayes Factor
#> b_Intercept 2.45e-03
#> b_Age 2.30e-03
#> b_Base 1.94
#> b_Trt1 0.07
#> b_Base.Trt1 6.17e-04
#> ---
#> Evidence Against Test Value: 0
sessionInfo()
#> R version 3.6.0 (2019-04-26)
#> Platform: x86_64-w64-mingw32/x64 (64-bit)
#> Running under: Windows 10 x64 (build 17763)
#>
#> Matrix products: default
#>
#> locale:
#> [1] LC_COLLATE=German_Germany.1252 LC_CTYPE=German_Germany.1252
#> [3] LC_MONETARY=German_Germany.1252 LC_NUMERIC=C
#> [5] LC_TIME=German_Germany.1252
#>
#> attached base packages:
#> [1] stats graphics grDevices utils datasets methods base
#>
#> other attached packages:
#> [1] brms_2.9.0 Rcpp_1.0.1
#>
#> loaded via a namespace (and not attached):
#> [1] httr_1.4.0 Brobdingnag_1.2-6 gtools_3.8.1
#> [4] StanHeaders_2.18.1-10 threejs_0.3.1 shiny_1.3.2
#> [7] assertthat_0.2.1 highr_0.8 stats4_3.6.0
#> [10] bayestestR_0.2.1 yaml_2.2.0 backports_1.1.4
#> [13] pillar_1.4.1 lattice_0.20-38 glue_1.3.1
#> [16] digest_0.6.19 promises_1.0.1 colorspace_1.4-1
#> [19] htmltools_0.3.6 httpuv_1.5.1 Matrix_1.2-17
#> [22] plyr_1.8.4 dygraphs_1.1.1.6 pkgconfig_2.0.2
#> [25] rstan_2.18.2 logspline_2.1.13 purrr_0.3.2
#> [28] xtable_1.8-4 mvtnorm_1.0-10 scales_1.0.0
#> [31] processx_3.3.1 later_0.8.0 tibble_2.1.3
#> [34] bayesplot_1.7.0 ggplot2_3.2.0 DT_0.7
#> [37] withr_2.1.2 shinyjs_1.0 lazyeval_0.2.2
#> [40] cli_1.1.0 magrittr_1.5 crayon_1.3.4
#> [43] mime_0.7 evaluate_0.14 ps_1.3.0
#> [46] nlme_3.1-140 xts_0.11-2 pkgbuild_1.0.3
#> [49] colourpicker_1.0 rsconnect_0.8.13 tools_3.6.0
#> [52] loo_2.1.0 prettyunits_1.0.2 matrixStats_0.54.0
#> [55] stringr_1.4.0 munsell_0.5.0 callr_3.2.0
#> [58] compiler_3.6.0 rlang_0.3.4 grid_3.6.0
#> [61] ggridges_0.5.1 htmlwidgets_1.3 crosstalk_1.0.0
#> [64] igraph_1.2.4.1 miniUI_0.1.1.1 base64enc_0.1-3
#> [67] rmarkdown_1.13 codetools_0.2-16 gtable_0.3.0
#> [70] curl_3.3 inline_0.3.15 abind_1.4-5
#> [73] markdown_1.0 reshape2_1.4.3 R6_2.4.0
#> [76] gridExtra_2.3 rstantools_1.5.1 zoo_1.8-6
#> [79] knitr_1.23 bridgesampling_0.6-0 dplyr_0.8.1
#> [82] shinystan_2.5.0 shinythemes_1.1.2 insight_0.3.0.9000
#> [85] stringi_1.4.3 parallel_3.6.0 tidyselect_0.2.5
#> [88] xfun_0.7 coda_0.19-2Created on 2019-06-18 by the reprex package (v0.3.0) |
|
Just ran the test-command, and all tests pass. |
|
Updated brms to 0.2.9, still doesn't work... Okaaay nevermind, I've always had problems with brms anyway :) |
I reorganized the vignette a bit, in particular, I transferred the BF introduction to the second tutorial.I think it is smoother to introduce it that way, with simpler models, rather than just dropping it at the end of the first tutorial (which is more focused at intuitive descriptions of the posterior). I think it works quite nicely this way, as we can also showcase the "support" (to be improved) of BayesFactor and draw a nice continuation between basic tests and regression models. The second tutorial also aims at introducing the plotting abilities (TBD). Then, diagnostic indices and model visualisation will be added or in the 3rd tutorial.
I've also transferred the BF reporting paragraph to the
report. The reason is that the "guidelines" vignette is pretty much meant to be our guidelines for reporting models, that we will draw from that future paper. We'll see what to put there once the paper is finished, but until then, I prefer to stick with the preliminary data, favouring (BF = 42) the dual use of more straightforward indices of effect existence and significance (pd and rope). Anyway, we will re-discuss these guidelines soon 😉What do you think?
One a side note, the bayes_factor vignette also fails in local at my place. Will investigate tomorrow.