-
Notifications
You must be signed in to change notification settings - Fork 3
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
Allow to fix or predict any parameter #152
Allow to fix or predict any parameter #152
Conversation
- remove bmm$fit_args - add functions for resetting formula environments to avoid storing unnecessary objects -
- also add message about how to turn it off
- a bunch of style changes
- a somewhat major restructuring of how the formula and priors are constructud. As a result, now users can predict parameters that are fixed by default, or to fix free parameters to a constant. - default priors are now specified within the model object - the prior no longer constructed in configure_model.* functions, but in configure_prior. This reduces code duplication and allows for the above functionality - minor style changes throuhout - add_missing_parameters now tags parameters that the user specifies as constant - check_model updates the "fixed_parameters" field in the stored model with any fixed parameters specified by the user
I'll put comments in the code to orient to the main changes (I should have done the stylistic changes separately, now they make it difficult to see key changes unfortunately) |
.github/workflows/R-CMD-check.yaml
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.
to make cmdstanr work on the automatic checks
a = "identity", | ||
c = "identity" | ||
), | ||
fixed_parameters = list(mu1 = 0, mu2 = 0, kappa2 = -100), |
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.
all fixed parameters, including the internal now are part of the model object. Necessary for the new prior method to work. They are still not printed in the summary
formula <- formula + | ||
glue_lf(kappa_unif,' ~ 1') + | ||
glue_lf(mu_unif, ' ~ 1') + | ||
formula <- bmf2bf(model, 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.
simplified the formula construction for all non-target models. now the uniform distribution is kappa2 and mu2, which makes it much easier because their index does not depend on the setsize. Allows to build the constant priors with a general function outside for all models
'(1-', lure_idx_vars[i], ')*(-100)') + | ||
glue_nlf(mu_nts[i], ' ~ ', nt_features[i]) | ||
glue_nlf("{kappa_nts[i]} ~ kappa") + | ||
glue_nlf("{theta_nts[i]} ~ {lure_idx[i]} * a + (1 - {lure_idx[i]}) * (-100)") + |
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 rewrote the glue_nlf function to allow us to use cleaner syntax - variables are specified between {} brackets, the rest is literal text
a=list(main = 'normal(0,1)', effects = 'normal(0,1)', nlpar=T), | ||
c=list(main = 'normal(0,1)', effects = 'normal(0,1)', nlpar=T))) | ||
} | ||
formula$family <- brms::do_call(brms::mixture, vm_list) |
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.
family now must be attached to the formula object
} | ||
formula$family <- brms::do_call(brms::mixture, vm_list) | ||
|
||
nlist(formula, data) |
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.
do not return family or prior any more
a_preds <- rhs_vars(bmm_formula$a) | ||
|
||
#' @export | ||
configure_prior.IMMabc <- function(model, data, formula, user_prior, ...) { |
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.
the only prior that needs to be defined specifically for each model is that which depends on model and data conditions, such as the setsize here. Everything else is handled by configure_prior.bmmmodel()
# assign attribute nl TRUE/FALSE to each component of the formula | ||
assign_nl(formula) | ||
formula <- assign_nl(formula) | ||
assign_constants(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.
determines which parameters should be treated as constants rather than estimated
arg <- dots[[i]] | ||
if (is_formula(arg)) { | ||
par <- all.vars(arg)[1] | ||
} else if (is.numeric(arg) && length(arg) == 1) { |
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.
allows specifying "par = value", which is not actually a formula
R/distributions.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.
only style changes, no changes to functionality
# combine the default prior plus user given prior | ||
config_args$prior <- combine_prior(config_args$prior, prior) | ||
# configure the default prior and combine with user-specified prior | ||
prior <- configure_prior(model, data, config_args$formula, prior) |
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.
configure_prior is now the main way we construct:
- the default prior
- the constant priors
- combine with user prior
R/helpers-data.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.
only stule changes, can ignore
model <- replace_regex_variables(model, data) | ||
model <- change_constants(model, 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 line is key to making everything work - it will read the "constant" attribute from the formula and replace the fixed_parameters field in the model object. This information is then used by all other methods such as the summary, configure_prior, etc
below this line are only stylistic changes
R/helpers-parameters.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.
only style changes, can ignore
@@ -90,151 +91,182 @@ get_model_prior <- function(object, data, model, formula = object, ...) { | |||
#' class="Intercept", dpar=parameter_name) for all fixed parameters in the | |||
#' model | |||
#' @noRd | |||
fixed_pars_priors <- function(model, additional_pars = list()) { | |||
fixed_pars_priors <- function(model, formula, additional_pars = list()) { |
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 now automatically constructs the constant priors, recognizing which parameters should nlpars, which column to put the intercept in, etc. It now takes the brmsformula that is returned from configure_model(), because we can easily extract that information from there
if(!is.list(prior_list[[i]])) { | ||
stop("The prior_list should be a list of lists") | ||
} | ||
set_default_prior <- function(model, data, 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.
now takes the final brmsformula instead of the bmm formula, making it easier to determine which pars are nlpars for the priors without having to specify that manualyl in the priors_list as before
} | ||
|
||
#' @export | ||
configure_prior.bmmmodel <- function(model, data, formula, user_prior, ...) { |
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.
the main new S3 method - called by fit_model() after configure_model(). It flexibly constructs all the priors with very little code
R/utils.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.
only style changes, can ignore
fixed_parameters = list(mu1 = 0, mu2 = 0, kappa2 = -100), | ||
default_priors = list( | ||
kappa = list(main = "normal(2,1)", effects = "normal(0,1)"), | ||
a = list(main = "normal(0,1)", effects = "normal(0,1)"), |
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.
the default priors list is now part of the model object. Same for all models
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 looks all good to me. I think I have also grasped all the relevant changes and will apply them to the M3 models. Should I have any questions, I will get back to you. Thanks also for taking care of the cosmetic changes to the code.
Cool, thanks for reviewing it! |
Summary
A few major changes in this PR:
bmf(mu ~ setsize)
, for example, for the sdmSimple() model, and similarly for mu1 for all the othersbmf(par = value)
. For example, if you want to fix kappa to 2, and not estimate it, you can use a formula likebmf(c ~ 0 + setsize, kappa = 2)
To achieve this, I made the following structural changes:
add_missing_parameters()
now tags each parameter in the bmmformula with an attribute constant. TRUE if it is fixed by default, and not predicted by the user, or if the user fixed a free parameter to a constant. Then it replaces all forms 'par = value' with 'par ~ 1'check_model()
now updates the fixed_parameters field of the model object based on the "constant" attributes in the formula. If a default constant parameter is predicted in the user formula, it removes it from fixed_parameters. If a free parameter is fixed by the user, it adds itprior_list
argument within configure_model(), that specifies the default prior is now specified in the model object as "default_priors".configure_prior()
is called in fit_model() right after configure_model(). Like all other methods, it takes the model object, the data and the user formula, and in addition, the user given prior. The configure_prior.bmmmodel() method combines informaiton from the formula, data and fixed_parameters + default_priors field of the model object, and constructs both the constant priors and the default_priors.I also did a bunch of minor stylistic cleaning and organizing, which is why it will show that a lot of files are changed, although most are just style. Also fixed a few minor bugs not worth describing in detail.
cmdstanr
now the default backend if it is installed. If it is not,rstan
is used instead.Closes #142
Closes #145
Closes #72 (last comment)
Note that after this you will need to update the M3 code to ensure consistency with the new changes. Mainly:
Example
Here's an example of how the new features work and the output:
Tests
[x] Confirm that all tests passed
[x] Confirm that devtools::check() produces no errors
Release notes