-
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
A vignette about default prior standata and stancode #176
A vignette about default prior standata and stancode #176
Conversation
…bout-default_prior-standata-and-stancode
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 have made some comments and suggestions for changes. Generally, i think this is a great summary of the most important functionality. I think you should consistently write parameters in code writing. So each time a bmm_parameter
is mention, I would highlight this by writing kappa
, c
, or mu
. This might be personal preference, but I would try to keep this consitent throughout the vignette.
vignettes/extract-info.Rmd
Outdated
|
||
All of the above examples make an important point - priors are always specified on the scale at which the parameters are sampled. You can always check the documentation for a given model to see the links for the parameters (e.g. `?sdmSimple`). | ||
|
||
|
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 think we should add a short section here to point users towards the differences between models that use nlpars
and models that use dpars
. This is something they need to consider when setting priors. So, I would always suggest to quickly run the default_prior
function, to see if the paramteres of a a model are considered nlpars
or dpars
.
The alternative would be to again generalize the prior function into an S3 method and have a bmmversion for it. But I think in this case this is not really feasible. Maybe this is something we could discuss with Paul and see if there is a more user friendly way of implementing the prior specification. But for now, I think it is ok that users have to check if parameters are nlpars
or dpars
.
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 is something that applies generally to setting priors on brms models, so I don't think a detailed section on this is necessary. I added a sentence directing users to the documentation of set_prior
for that.
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.
Hmm... I see your point, but then for brms
users specify their own formula and thus would be aware that they have specified a nlpar
in it, and in all other cases parameters would be dpars
or in case of mu
do not need an explicit label.
For bmm
it is however less straightforward, as it is not apparent from the parameters
list if a parameter is specified as nlpar
or dpar
. That depends on how we implemented the model. If the model can be implemented using the distributions available for brms
then model parameters
will typically be nlpars
. If we implemented the model using a custom_family
then the model parameters
will be dpars
. But for the user it is not self-explanantory when model parameters
are nlpars
and when dpars
that's why I thought it might be good to explain how users can find out.
But maybe, I am also too worried about users here.
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.
but that doesn't matter - the parameter type will be correctly displayed in the default_prior output
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.
For example:
# define formula
ff <- bmmformula(
kappa ~ 0 + set_size,
c ~ 1,
a ~ 1,
s ~ 1
)
# specify the full IMM model with explicit column names for non-target features and distances
model1 <- IMMfull(resp_error = "dev_rad",
nt_features = paste0('col_nt', 1:7),
nt_distances = paste0('dist_nt', 1:7),
set_size = 'set_size')
# fit the model
default_prior(ff, data = data, model = model1)
prior class coef group resp dpar nlpar lb ub source
logistic(0, 1) theta2 -Inf Inf default
(flat) b a default
(flat) b c default
normal(2, 1) b set_size1 kappa <NA> <NA> (vectorized)
normal(2, 1) b set_size2 kappa <NA> <NA> (vectorized)
normal(2, 1) b set_size3 kappa <NA> <NA> (vectorized)
normal(2, 1) b set_size4 kappa <NA> <NA> (vectorized)
normal(2, 1) b set_size5 kappa <NA> <NA> (vectorized)
normal(2, 1) b set_size6 kappa <NA> <NA> (vectorized)
normal(2, 1) b set_size7 kappa <NA> <NA> (vectorized)
normal(2, 1) b set_size8 kappa <NA> <NA> (vectorized)
(flat) b s default
normal(2, 1) b kappa <NA> <NA> user
normal(0, 1) b Intercept a <NA> <NA> user
normal(0, 1) b Intercept c <NA> <NA> user
normal(0, 1) b Intercept s <NA> <NA> user
constant(0) Intercept mu1 <NA> <NA> user
constant(0) Intercept mu2 <NA> <NA> user
constant(-100) Intercept kappa2 <NA> <NA> user
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 just meant that we don't have to instruct them to check default_prior - this is already instructed in the set_prior documentation for brms. Thus, there is nothing specific to bmm about the workflow that we need to make explicit
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.
Ah ok. I see your point now.
I updated the vignette based on your comments |
…bout-default_prior-standata-and-stancode
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.
There is one little think regarding the code chunk for the standata
that you need to review apart from that I think everything is good. I will approve this for now, and then you can merge once you have looked at the issue with the last code chunk.
vignettes/bmm_extract_info.Rmd
Outdated
|
||
If you want to extract the data that would be used for fitting a model, you can use the `standata()` function from `brms`. This function will return a list with the data that would be passed to Stan for fitting the model. | ||
|
||
```{r} |
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 code chunk needs message=FALSE, warning=FALSE
added so that the warning for the setting sort_data = TRUE
does not display. Unless you wanted to show this feature. But then you should add an explanation. I was a bit confused about it.
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.
goood point. I removed the sort_data argument and suppressed the message. Thanks for reviewing!
Summary
Add a vignette about how to use default_prior, standata and stancode
Add information about bmm version to the line // generated with brms ... in the stancode
Add information about the default priors used to the help page for each model
Closes #146
Tests
[X] Confirm that all tests passed
[X] Confirm that devtools::check() produces no errors
Release notes