Support spline-based distribution for flexsurv

[mattwarkentin]

Feature

I posted this over on the RStudio Community forum, but I thought it might be easier to track/discuss over here, so I will copy and paste the post.

Oh and while I'm thinking of it, I would suggest supporting flexsurv::flexsurvspline() in addition to flexsurv::flexsurvreg() for survival_reg(). The latter is based on known parametric distributions (e.g. Weibull, etc.), while the former supports arbitrarily flexible baseline (cumulative) hazards with natural cubic splines (Royston-Parmar model).

Maybe some sentinel function like dist_spline(k, knots, bknots, scale = 'hazard', timescale = 'log') that defines the spline model, where k is mutually exclusive with knots/bknots. An API like:

survival_reg(engine = 'flexsurv', dist = dist_spline(k = tune()))

This might require adding a dials::knots() function, which would be very similar to the dials::deg_free() function.

[mattwarkentin]

As always, happy to be a contributor and not just a beggar.

[mattwarkentin]

Could also just add 'spline' as a distribution in dials::values_flexsurv_dist and add the argument k to parsnip::survival_reg():

survival_reg(engine = 'flexsurv', dist = 'spline', k = tune()) # or knots = tune()

Less flexible, but more consistent with the parsnip API.

[hfrick]

Having flexsurvspline() models in censored would be great! Thanks for offering to contribute!

Putting this in a new engine would work better than the distinction via the dist argument. We can rename the existing flexsurv engine for survival_reg() to flexsurvreg and name the new engine flexsurvspline.

The number of knots and the scale would be engine-specific arguments (set in set_engine()) rather than main arguments to survival_reg(). They both should get corresponding dials objects to make them tunable. Calling the argument (and dials parameter) for the number of knots num_knots lets us avoid the clash with the knots argument to flexsurvspline (for the location of the knots) and is consistent with several other num_* parameter objects in dials. The scale argument should also get a new name that's more descriptive than just "scale" but we don't have a particular suggestion yet.

[mattwarkentin]

I think you are right and it makes sense for this to be implemented as a separate engine. I am working on updating the dials PR as we speak.

For the 'scale' argument, I guess I can propose surv_scale as a name option? Very open to suggestions.

[hfrick]

closed in #213

[github-actions]

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.