search

tidymodels / censored

Parsnip wrappers for survival models
https://censored.tidymodels.org/
Other
123 stars 12 forks source link

Enable `multi_predict._coxnet()` for all `type`s #282

Closed hfrick closed 10 months ago

hfrick commented 11 months ago

Closes #267 and closes #277

This PR redoes multi_predict() for glmnet's Cox models (aka coxnet).

In parsnip, predict() is rather strict about which arguments it accepts. It does not, for example, accept any arguement to pass on multiple penalty values. We make multi_predict() work by leveraging the less strict predict(type = "raw") which allows us to pass on that information in opts.

In censored, we cannot use any methods from glmnet directly and instead have survival_prob_coxnet() and survival_time_coxnet() as the functions that take a fitted model and return predictions. They are capable of dealing with a single or multiple penalty values.

To get the formatting of the output right (multi_predict() should include a penatly column while predict() should not), we need the multi argument to those functions (you could use a single penalty value in either setting). However, we cannot pass that through predict(), see inital comment about its strictness.

We could either allow a special in case in parsnip or bypass predict() and call those survival_*_coxnet() functions directly from multi_predict().

Adding yet more special glmnet code to parsnip, and for survival models in particular, did not seem very appealing so I went with the second option here. It does mean that we do need to carry out some of the check that otherwise predict() would carry out: is the type of prediction available for this model spec, what's in the dots, etc.

I've copied those over from parsnip for now, rather than export them from there because I think they might change a bit when we overhaul checking functions in parsnip. So down the line, we might export them, I've just not done this yet.

I've chatted with Max about the general approach here, but if you think I overlooked something or a different approach is better, please do shout!

hfrick commented 11 months ago

Note to self: add a NEWS item!

hfrick commented 10 months ago

Yeah, not loving the parsnip copies but hoping it's the better decision for now!

github-actions[bot] commented 10 months ago

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.