giabaio / survHE

Survival analysis in health economic evaluation Contains a suite of functions to systematise the workflow involving survival analysis in health economic evaluation. survHE can fit a large range of survival models using both a frequentist approach (by calling the R package flexsurv) and a Bayesian perspective.
https://gianluca.statistica.it/software/survhe/
41 stars 19 forks source link

Possibility to fit "all" models even if they do not converge #46

Open fthielen opened 2 years ago

fthielen commented 2 years ago

Dear Gianluca,

I would love to use survHE::fit.models() to estimate "all possible" models in one go. Similar to:

all_distributions <- c("exponential", "gamma","genf","gengamma","gompertz",
"weibull", "weibullPH","loglogistic","lognormal")

x <- survHE::fit.models(formula=Surv(time,status)~ph.ecog,data=lung,
 distr = all_distributions)

This example works fine, but for some data, some distribution cannot be estimated, resulting in an error. This interrupts the computation, although not being able to fit could as well be a valid outcome of the analysis. Is there a way to not let the estimation stop with an error but instead proceed and leave the place in the result list for the not estimated model empty? (e.g., x$models$exponential "Not estimated").

I wrote my own function using tryCatch(error = function(e) NULL). In this way, the execution is not interrupted.

Best, Frederick

giabaio commented 2 years ago

I can see that interrupting the analysis because of errors in one of the models may be annoying... BUT: to be honest, if I'm doing real work, I would probably recommend using separate calls to fit.models --- the rest of survHE can cope with this and, for instance, you can still plot many separate objects created by fit.models with little coding (and all embedded in the call to the plot method...).

I'm a bit hesitant to make your proposal the default choice, for two reasons:

  1. An expert user can get around the problem, if they want --- for instance exactly as you have
  2. It may be informative to actually capture the reason why the model has failed. Perhaps you get an indication as to why convergence is failing --- or where the hold up is; in your proposed alternative, you may hold your hands up in the air a bit too quickly --- it may be that you only need to change priors (say) and all would work...

All in all, I think I would rather keep things as they are, if that's OK? Gianluca

fthielen commented 2 years ago

Thank you for your prompt reply.

I understand your reasoning and of course it’s OK to leave things as they are.

Just for completeness, I want to clarify the reason for my initial request because it is also a showcase of some real-world application of these functions.

Currently, we are working on a health economic model that is built on patient-level data from a foreign country. Diverse rules and regulation do not allow us to retrieve patient-level data directly and hence we need to work with this “remotely”. Flexsurv objects need to be stripped from patient level data before we can receive the model objects. This necessity seems to concern other users as well, and recently the flexsurv package was adapted to allow for predictions with such objects (see here: https://github.com/chjackson/flexsurv-dev/issues/127#issuecomment-1082099132).

For this analysis, we have to provide the R script, and we can indeed get around the described problem by writing own functions. I however prefer using functionalities of CRAN packages if available (I feel that there is some extra level of reliability/validity with those). Hence, my request.

The proposed functionality does not need to (and should not) be the default. An extra argument such as proceed_error = TRUE could enable this functionality. And instead of returning an empty list entry, the actual reason for the failure (error message) could be captured.

Best, Frederick