summary_log: make compatible with nmbayes

[kyleam]

summary_log is incompatible with nmbayes models: https://github.com/metrumresearchgroup/bbr.bayes/issues/40

To make it work, we need

• to not collect nested models (handled by https://github.com/metrumresearchgroup/bbr/pull/643)
• adjust the summary_log logic to work with the nmbayes, in particular the object returned by model_summary.bbi_nmbayes_model(). (Edit: The bbr.bayes model_summary methods now error; discussion)

This series handles the second item above.

---

• [x] Verify that bbr.bayes CI passes (drone run, metrumresearchgroup/bbr.bayes#123)

[kyleam]

I've updated the series. I put it on top param_estimates_batch fix (gh-648) and made the follow changes:

• updated model_summaries_onetry() to put NULL as the summary object when a model_summary errors (696ea867)

• dropped the warning handling from summary_log_impl() given the model_summary.bbi_stan_model() to error instead of warn (a443ed4b)

• log message updates for the fact that model_summary.bbi_{stan,nmbayes}_model now error

Once those changes are pushed to https://github.com/metrumresearchgroup/bbr.bayes/pull/123, I'll add a comment here pointing to the Drone run.

range-diff

``` 1: 15c2cbdd = 1: fd3dedfe docs: don't generate Rd for model_summaries_concurrent 2: c08a516d = 2: 5e434199 model-summaries: extract error formatting to top-level function 3: 4936ac42 ! 3: 04b161ee model-summaries: try harder to use concurrent bbi path @@ Commit message As noted by e31fe344 (model_summaries: restrict concurrent bbi path to "all nonmem" case, 2022-08-12), another option would be to split the - list into non-NONMEM and NONMEM models. Do that now because, to - handle NONMEM Bayes model, we're going to need to split the list - anyway. + list into non-NONMEM and NONMEM models. Do that now because, when we + introduce additional handling for Stan and NONMEM Bayes models, we're + going to need to split the list anyway. While touching these lines, switch to using a standard anonymous function rather than a formula because the latter prevents 'R CMD 4: 37753a7e ! 4: 696ea867 model-summaries: add variant that doesn't retry @@ Commit message models that don't inherit from bbi_nonmem_model, leading to the model-specific model_summary() method being called. - For Stan models, the CmdStanMCMC object returned by model_summary() - doesn't trip up the downstream processing in model_summaries_serial(). - So, it's a little odd that model_summaries_serial() is tailored to bbi - (e.g., .bbi_args and the .fail_flags retry), but it happens to work. - - For NONMEM Bayes models, on the other hand, model_summary() returns an - array object, leading to a failure when model_summaries_serial() tries - to access the error_msg list item. - Add a tailored and simplified model_summaries_* variant that is meant to work with model types like Stan and NONMEM Bayes that don't go through bbi. @@ R/model-summaries.R: model_summaries_concurrent <- function(.mods, .bbi_args, .f +model_summaries_onetry <- function(.mods) { + purrr::map(.mods, function(m) { + s <- tryCatch(model_summary(m), error = identity) -+ err <- if (inherits(s, "error")) { -+ format_model_summary_error(s) ++ if (inherits(s, "error")) { ++ err <- format_model_summary_error(s) ++ s <- NULL + } else { -+ NA_character_ ++ err <- NA_character_ + } + rlang::list2( + !!ABS_MOD_PATH := m[[ABS_MOD_PATH]], 5: 0406ce44 = 5: 2b6d6cf7 model-summaries: rename serial and concurrent helpers for clarity 6: f4d26eb5 < -: -------- summary-log: tweak suppressSpecificWarning for nmbayes models -: -------- > 6: a443ed4b summary-log: don't expect warning from model_summary.bbi_stan_model 7: ebc2b23b ! 7: 82268870 summary-log: adjust guard to work with nmbayes models @@ Commit message summary_log_impl() returns early if none of the models inherit from bbi_nonmem_model to avoid the downstream code failing. This guard incorrectly lets through bbi_nmbayes_model objects. Although they - derive from bbi_nonmem_model, model_summary.bbi_nmbayes_model() - returns a draws_array instead of a bbi_nonmem_summary object. + derive from bbi_nonmem_model, model_summary.bbi_nmbayes_model() does + not return a bbi_nonmem_summary object (it currently errors). Adjust the guard to check the relevant feature: whether the "bbi_summary" column is a bbi_nonmem_summary object. 8: c49684e2 = 8: 584ddb55 extract_from_summary docs: fix parameter description 9: 7ff46065 ! 9: e1fdeed1 summary-log: limit extract functions to bbi_nonmem_summary objects @@ Commit message After collecting the main summaries in a data frame, summary_log_impl() calls various custom extract_* functions intended - to pull information out of a list of bbi_nonmem_summary objects. For - logs with Stan models, that includes CmdStanMCMC objects; that works - okay because they trigger the NULL/NA path for each function. + to pull information out of a list of bbi_nonmem_summary objects. - For NONMEM Bayes objects, the list includes draws objects. That - triggers failures like - - Caused by error in `.x[["ofv"]]`: - ! subscript out of bounds + Until recently [*], the model_summary() method for Stan and NONMEM + Bayes models relayed the result of read_fit_model(). In the case of + Stan, sending CmdStanMCMC objects through the extract_* functions + works okay because they trigger the NULL/NA path for each function. + For NONMEM Bayes objects, the result is a draws array, and sending + that through leads to various errors. The core problem is that these functions are expecting an object that looks like what comes out of `bbi nonmem summary --json`, and that's not reasonable when outside packages like bbr.bayes can define custom model_summary() methods. - Add a function that wraps these extract_* functions so that they are - called with a list that only contains bbi_nonmem_summary objects. + The Stan and NONMEM Bayes model_summary() methods now signal an error, + so this is no longer an issue _at the moment_. But the plan is to + eventually implement these. + + [*] https://github.com/metrumresearchgroup/bbr.bayes/pull/123 Re: https://github.com/metrumresearchgroup/bbr.bayes/issues/40 ```

[kyleam]

Once those changes are pushed to metrumresearchgroup/bbr.bayes#123, I'll add a comment here pointing to the Drone run.

That's here: https://github-drone.metrumrg.com/metrumresearchgroup/bbr.bayes/445/1/4. The failure is unrelated and probably due to rate limiting. I'll update this message with all green build once we have one, but this is ready for review.

Edit: Passing builds (on restart):

• https://github-drone.metrumrg.com/metrumresearchgroup/bbr.bayes/446
• https://github-drone.metrumrg.com/metrumresearchgroup/bbr.bayes/447

[kyleam]

@seth127 Thanks for taking another look (and for catching the issue with putting draws in the summary_log output)