-
Notifications
You must be signed in to change notification settings - Fork 2
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
Summary method for bmmfit #144
Conversation
Merge branch 'develop' into feature/37-summary-method-for-bmmfit # Conflicts: # NAMESPACE
This looks great. I like the color and the highlighted labels. There are some typos (at least in the "Additional Fixed Constant Parameters" label). And I think this label is somewhat redundant, I would propose to just say either With respect to the links: I think adding them to the model info makes sense. In fact, for the |
ok, then let's meet one of the following days and you can show me again how you set the links in M3custom, so we can figure out how to set it for all mdoels before I implement that part of the summary method |
|
- import stats and glue::glue - flatten the list in model objects - add links argument - make mu parameter explicit in summary - updates to the restructure function - bmm fit objects are restructure to the latest version first in the summary method - parameters are printed if they are dpars in the formula (not have other parameters on rhs) or part of the model base parameters - new tests
This is now done. Took me much longer than I expected, but now all features should be ironed out. Here's the output for a few models: There is an option "backend" for the summary method, with which you can always see the brms-type summary: > summary(fit, backend='brms')
Family: mixture(von_mises, von_mises, von_mises, von_mises, von_mises)
Links: mu1 = identity; kappa1 = log; mu2 = identity; kappa2 = log; mu3 = identity; kappa3 = log; mu4 = identity; kappa4 = log; mu5 = identity; kappa5 = log; theta1 = identity; theta2 = identity; theta3 = identity; theta4 = identity; theta5 = identity
Formula: dev_rad ~ 1
mu1 ~ 1
kappa ~ 0 + SetSize
a ~ 0 + SetSize
c ~ 0 + SetSize
kappa5 ~ 1
mu5 ~ 1
theta1 ~ c + a
kappa1 ~ kappa
kappa2 ~ kappa
theta2 ~ LureIdx1 * (a) + (1 - LureIdx1) * (-100)
mu2 ~ Item2_Col_rad
kappa3 ~ kappa
theta3 ~ LureIdx2 * (a) + (1 - LureIdx2) * (-100)
mu3 ~ Item3_Col_rad
kappa4 ~ kappa
theta4 ~ LureIdx3 * (a) + (1 - LureIdx3) * (-100)
mu4 ~ Item4_Col_rad
Data: data (Number of observations: 4000)
Draws: 4 chains, each with iter = 500; warmup = 250; thin = 1;
total post-warmup draws = 1000
Regression Coefficients:
Estimate Est.Error l-95% CI u-95% CI Rhat Bulk_ESS Tail_ESS
mu1_Intercept 0.00 0.00 0.00 0.00 NA NA NA
mu5_Intercept 0.00 0.00 0.00 0.00 NA NA NA
kappa5_Intercept -100.00 0.00 -100.00 -100.00 NA NA NA
kappa_SetSize1 3.07 0.05 2.97 3.16 1.01 1689 658
kappa_SetSize2 2.45 0.05 2.35 2.55 1.00 1643 785
kappa_SetSize3 2.14 0.06 2.02 2.26 1.00 1400 753
kappa_SetSize4 2.01 0.09 1.83 2.17 1.00 1361 837
c_SetSize1 3.95 0.27 3.47 4.50 1.01 1028 613
c_SetSize2 3.38 0.24 2.94 3.87 1.00 1039 665
c_SetSize3 2.84 0.17 2.52 3.20 1.00 829 487
c_SetSize4 2.70 0.18 2.39 3.08 1.00 950 638
a_SetSize1 0.00 0.00 0.00 0.00 NA NA NA
a_SetSize2 -0.43 0.34 -1.11 0.25 1.00 1041 709
a_SetSize3 -0.90 0.28 -1.45 -0.38 1.00 921 594
a_SetSize4 -1.66 0.27 -2.20 -1.14 1.00 941 603
Draws were sampled using sample(hmc). For each parameter, Bulk_ESS
and Tail_ESS are effective sample size measures, and Rhat is the potential
scale reduction factor on split chains (at convergence, Rhat = 1). Also closes #149 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I combined the files bmmformula and helpers-formula, since we don't need two for that and it wasn't cclear what should go where
is.formula <- function(x) { | ||
# adds an attribute nl to each component of the the formula indicating if the | ||
# any of the predictors of the component are also predicted in another component | ||
assign_nl <- function(formula) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function is now used in constructing bmmformula, so for the future we can always just check which parameters are non-linear via is_nl() functionbellow, rather than always checking manually if the predictors are also parameters
R/restructure.R
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an initial restructure function allows us to apply new methods to fits using older bmm versions. there are more things to be added but for now it works for the summary function at least
#' @seealso \code{\link{summary.brmsfit}} | ||
#' @note You can turn off the color output by setting the option | ||
#' options(bmm.color_summary = FALSE) or bmm_options(color_summary = FALSE) | ||
#' @export |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
summary.bmmfit first calls the summary.brms method, which extracts all necessary info for printing. then it passes it to print.bmmsummary
x | ||
} | ||
|
||
select_pars <- function(x) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selects which parameters to include in the summary - model parameters and any parameters used in non-linear formulas. because brms does not print out non-linear parameters, we don't either. E.g., if you predict kappa by a non-linear formula, kappa won't be included in the output, but just the parameters by which it is predicted.
Thanks for implementing the additional changes! I am in a workshop tomorrow and will have to see when I get to review this throughly. But if I split it up, I might get to it this evening and tomorrow. |
Another thing that just came to my mind is that we need to update the model template accordingly. If you already did so, then just ignore this comment. |
not yet. I also just realized the vignettes need to be updated. I will update the vignettes and include that here. For the model template, I suggest we wait. We also need to update the dev notes as well. My suggestion is to open an issue to update those, and we do it before the next release, as we might make more changes that affect them |
I updated the vignettes. Also I added a fille bmmexamples in tests/local, which creates several internal datasets:
these are stored inside a R/sysdata.rda file and are accessible internally like bmm:::bmmfit_imm_vignette I reduced the size of each and now use these in the vignettes. Installed size is now down from 32mb to 5.5mb, but still get a note. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I checked, this looks great. I even merged this into the latest dev version for the M3 implementation and the summary also works for the custom models with users naming the parameters they use in the activation formulae.
Btw, locally on my Mac all checks are fine. I do not know what might be the problem regarding the R-CMD-checks performed by GitHub... |
I figured it out, it's because I generated the example bmm fits with cmdstanr, and use one of them in the tests. Turns out that the object "remembers" it was made with cmdstanr and gives a warning that the package doesn't exist in the online check... |
Great that you found the problem! From my side we should be good to merge then. |
Summary
The custom summary method is almost done. Below are some examples. There are two things left to do:
As you can see below, I have not yet implemented the links summary. It's tricky. For the sdm, I can just use the formula. But for the remaining models, because they use non-linear formulas, the link is not actually explicit in the formula - there are links only for the kappa1, kappa2, etc. One idea is to add an argument to the model$info, e.g., model$info$links, where we just specify the links even if we use that only for printing. Thoughts
This is not difficult, but I will do that after we figure out the links as its less important.
(the colors can be turned off with options(bmm.color_summary = FALSE))
sdm BEFORE
sdm AFTER
2p BEFORE
2p AFTER
3p BEFORE
3p AFTER
IMMfull before
IMMfull after
Also closes #149
Tests
[] Confirm that all tests passed
[] Confirm that devtools::check() produces no errors
Release notes