Enable multi_predict._coxnet()
for all type
s
#282
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 makemulti_predict()
work by leveraging the less strictpredict(type = "raw")
which allows us to pass on that information inopts
.In censored, we cannot use any methods from glmnet directly and instead have
survival_prob_coxnet()
andsurvival_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 apenatly
column whilepredict()
should not), we need themulti
argument to those functions (you could use a single penalty value in either setting). However, we cannot pass that throughpredict()
, see inital comment about its strictness.We could either allow a special in case in parsnip or bypass
predict()
and call thosesurvival_*_coxnet()
functions directly frommulti_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 thetype
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!