Closed SimonCMills closed 1 year ago
I don't think we should silently modify the priors (if a user mistakenly thinks that the covariate also shows up on detection we should error here). I definitely do think we should try to catch this error ourselves and error informatively.
Perhaps it would be value added to also provide our own set_priors_occupancy
and set_priors_detection
functions that handle the dpar
stuff automatically; the first would be guaranteed to only touch the occupancy parameters; the second would be guaranteed to only touch the detection parameters.
Also a note that whatever we implement here, we should think about how it will eventually scale to models with parameters for initial occupancy, colonization probabilities, and extinction probabilities.
Hmm, the thing that would be nice about flocker having two set_priors functions as a solution is that it avoids users having to engage with the implementation specific stuff (what is dpar, for example). On the other hand there are then three ways of specifying priors which possibly is going to cause more confusion than clarity. It's also difficult because if you do, e.g.
user_prior <- c(set_prior_occupancy("normal(0,1)", class ="b"), set_prior("normal(0,1)", class="Intercept"))
and add a new class to the prior object generated by set_prior_occupancy
(e.g. flockerprior
) to distinguish it from brmsprior
, the user_prior
object will still have the flockerprior
class, even though one of the priors was applied in the brms way rather than with the bespoke function. Not sure what the best solution is. Will mull it over.
We could require a flockerprior
argument to flock
that defaults to something corresponding to the default brms
prior, and thereby force people to use our own functions to create their priors. Not sure whether forcing the brms-savvy into this additional layer of abstraction from the underlying brms
model is a great idea though...
Upon further reflection, I think we want to trust Paul to handle and improve upon priors better/faster than we will, and since there's no need to force users to do something different from what brms does natively, we should just rely on the native brms stuff. However, I do think that this would be a useful issue to file against brms. @SimonCMills , want to do that? I guess you have some clout over there now that you're a contributor and all :)
Done: https://github.com/paul-buerkner/brms/issues/1490.
Will update you on any developments.
If the user provides priors on parameter classes (e.g.
set_priors(normal(0,1), class="b"
,set_priors(normal(0,1), class="sd")
), but these parameter classes only exist in the occupancy component (e.g.f_occ = ~ 1 + covariate
andf_det ~ 1
, orf_occ = ~ 1 + (1|group)
andf_det = ~1
), they get the uninformative error of, e.g.:or
when the parameters do exist, it's just that they aren't in the detection component. A quick solution is to specify that the priors only correspond to the occupancy component by, e.g.
set_priors(normal(0,1), class="b", dpar="occ")
, but not sure what the longer-term fix should be. Most simply it could just be a warning message in the priors section of the vignette, most complicated (but best) would be to check that, if priors are specified on particular classes that only exist in the occupancy component thenflock
either (a) throws an informative error message or (b) just silently updates the priors object. Fwiw, I think (b) is probably best if it's not a nightmare to implement. I can have a stab at it in the next few days.