Add flexsurv survival distributions and knots parameter

[mattwarkentin]

Preliminary work relevant to tidymodels/censored#115

[topepo]

Should the lower range be 1L or 2L?

[mattwarkentin]

k = 0 is the default for flexsurv::flexsurvspline, so I went with that. But I think 0 or 1 for the lower bound could work. I tend to still have preference for 0 for consistency with flexsurv, but I'm open to changing it. Though, k = 0 effectively fits a Weibull model, you really only start to get a spline model with 1+ knots.

[hfrick]

Following up here from tidymodels/censored#115:
Could you change the name from knots to num_knots, please? I'm not sure what you have in mind with values_flexsurv_dist but usually we don't add another set of default values like this. We probably have to think this one through a bit more. Could you take that one out for the moment?

[mattwarkentin]

Could you change the name from knots to num_knots, please?

On it!

I'm not sure what you have in mind with values_flexsurv_dist but usually we don't add another set of default values like this.

I decided to include this because flexsurvreg has parametric distributions that are not part of the set of distributions for survreg. And in some cases, even among the shared distributions, the naming is different (e.g. loglogistic vs llogis). I am happy to remove this, if desired, but I do think that in the future you may wish to support all of the distributions in flexsurvreg and not just those that overlap with survreg.

[topepo]

I'm going to read the source literature but do you think that we should have a dials function for the scale argument? If so, what would be a good name (since scale is pretty generic)?

[mattwarkentin]

I proposed surv_scale (to be similar-feeling to surv_dist) in the other thread (tidymodels/censored#115 (comment)). I do think this is worth including as a parameter because sometimes it is desirable to fit a proportional odds vs proportional hazards model, and vice versa. I have already created this function in my local fork of dials, I was just waiting to see what you/Hannah were thinking about its inclusion/name.

I can push it now and we can continue to iterate?

[topepo]

I can push it now and we can continue to iterate?

Sure.

[hfrick]

I'm fine with surv_scale but open to changes.

It changes the link function g() between the survivor function and the spline function but given that you supply values like "hazard" or "odds" I don't think making the name echo anything related to "link function" would necessarily increase clarity of what the parameter does.

[hfrick]

@mattwarkentin I've spoken with Max and he pointed out that surv_scale might get people thinking about location and scale parameters. He suggests survival_link instead. Would you like to make that change? Otherwise I'm happy to do that and then merge 🙌

[hfrick]

Thanks so much for your contribution @mattwarkentin ! Looking forward to your PR on censored!

[github-actions]

This pull request 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.