inlabru-org / inlabru

inlabru
https://inlabru-org.github.io/inlabru/
78 stars 21 forks source link

Possible issue with passing bru_options to joint models through the like() function #109

Closed joenomiddlename closed 2 years ago

joenomiddlename commented 3 years ago

Hi inlabru team!

I have been fitting joint models with multiple likelihoods using the like() - bru() approach, building each model using like(), before passing them to bru.

I tried to specify options (in particular control.family options) directly in each like() function call and thought I had been successful. I have just noticed that the control.family arguments of the fitted joint model have actually reverted back to the default settings and this had led to me fitting a model I did not aim to fit.

I am able to successfully specify the options directly as an argument to bru(), but would find it easier if I could pass the arguments directly in the like() function calls. If this is not possible, then I think it would be good to provide a warning message or something similar to alert the user to the fact that the options may not have been set.

I am running inlabru version 2.2.8.9000, and so sorry if this change has already been implemented in a more recent version!

Thanks, Joe

finnlindgren commented 3 years ago

Hi! Yes, I don't think any of the inla.*-options are used or stored by the like() function.

I agree it would be useful to be able to specify likelihood specific options to like(). It may need a a different mechanism, as it can't just pass them on to inla(), like it does with inla-specific options to bru(). In the bru() call, it will then need to combine the like-specific options into the required control.family format. The control.family documentation doesn't say how to specify values for multiple likelihoods.

Regarding adding a warning: I think it's only the control.family option that has this issue, and it doesn't really make sense to set aspect of that in the global options settings, so that's good. However, there is currently no way for like() to know where the options came from; when bru() call like() (for the single-likelihood case), it also supplies the options object, and like() won't know that bru() is the caller. Warning when control.family is supplied to like() would then lead to a warning in ordinary single-likelihood models that set control.family. To not break existing code, bru() would therefore first have to filter out any control.family option before calling like(), and add it back in before calling inla(). With that logic, having warning in like() would be ok. This would then be changed later, depending on how the new feature is implemented.

joenomiddlename commented 3 years ago

Thanks, Finn!

I hadn't considered that a warning message could break single-likelihood models...

With regards to specifying control.family arguments for multiple (K) likelihoods, I thought it was achieved through a list of K lists of options? Such as seen here: https://groups.google.com/g/r-inla-discussion-group/c/GFPFWZN8T1M/m/KlLRmnWZAAAJ

Thanks again for your help and speedy reply! Joe

finnlindgren commented 3 years ago

Thanks for providing the link! It might be as easy to implement the feature as it would be to add the warning logic.

joenomiddlename commented 3 years ago

No problem! That would be great

finnlindgren commented 2 years ago

By allowing the control.inla option to be given directly as an argument to like, the code can give a warning whenever the options=list(control.family=...) approach is used, but some likelihoods also have settings. For direct use of like, the syntax will be like(..., control.family = list(...)). For shortcut single-likelihood bru() calls, use bru(..., control.family = list(...)).