metrumresearchgroup / bbr

R interface for model and project management
https://metrumresearchgroup.github.io/bbr/
Other
22 stars 2 forks source link

`inherit_param_estimates` fails when flags needed for `model_summary()` call #637

Closed seth127 closed 5 months ago

seth127 commented 5 months ago

Summary of problem

In trying out the inherit_param_estimates() feature (new in 1.8.0) a user noticed the following problem:

It seems that under the hood it uses model_summary in some way, and if the previous model has not converged correctly or $COV has not been evaluated (no .grd file) the function won’t evaluate. According to the error text I should be able to suppress this by using the bbi argument –no-grd-file. However, inherit_param_estimates does not seem to have an option to input bbi arguments (i.e. there is no way to pass through .bbi_args)?

The user shared the following screen shot:

Screenshot 2024-01-17 at 10 03 32 AM

I verified that we do indeed call model_summary() to get the final estimates from the parent model, but provide no interface for passing through arguments to it.

Proposed solution

I lean towards handling this the way we do in nm_join(), which is another function that calls model_summary() internally and only intends to use a portion of the returned summary object: add .bbi_args, defaulting to no_grd_file = TRUE, no_shk_file = TRUE to the inherit_param_estimates() interface. (See 'nm_join()` docs for analogous rationale.)

With this approach:

  1. The default would handle the (likely most common) case that the user reported here.
  2. We would cover other unanticipated cases by giving users an interface to pass arguments through to model_summary().

@barrettk @kyleam any thoughts on that, or alternatives that we might consider?

kyleam commented 5 months ago

Your suggested approach makes sense to me.