Move flexsurv and parametric to {survivalmodels}

[RaphaelS1]

No description provided.

[RaphaelS1]

This shouldn't be closed. There is too much glue code in these learners. But I can transfer the issue to {survivalmodels}

[sebffischer]

Is there any plan on doing this? :)

[sebffischer]

@RaphaelS1 can you please transfer the issue (I am trying to clean house right now)
Or @bblodfon are you interested in doing this?

[RaphaelS1]

Meeting with John today. Will discuss then.

[RaphaelS1]

@bblodfon I tried to use the new predict.flexsurvreg method and sometimes get the same results as my implementation in this package and sometimes quite different, in the snippet below they're quite close. Would you mind checking out the code below and letting me know your thoughts? Then if we think our method is better we'll port all the code to extralearners, or if we think we should use the method from flexsurv (even if it's "worse" it might still be better just to use the native method) then we'll delete all the extra code here (I'd always prefer the method built into the underlying implementation where possible)

library(mlr3verse)
#> Loading required package: mlr3
library(mlr3extralearners)
set.seed(237498324)
l = lrn("surv.flexible")
t = tsk("rats")
split = partition(t)


# train learner
l$train(t, row_ids = split$train)

## 1) using extralearners method
pred = l$predict(t, row_ids = split$test)


## 2) using flexsurv method
# get survival probabilities
p = predict(l$model, type = "surv", newdata = t$data(split$test))$.pred

# check all times the same
times = p[[1]]$.time
x = all(unlist(lapply(p, function(.x) all.equal(times, .x$.time))))
ut = unique(times)

if (x) {
  # remove survival probabilities at duplicated time points 
  dup = !duplicated(times)
  surv = t(vapply(
    p, function(.x) .x$.pred_survival[dup],
    numeric(length(ut))
  ))
  colnames(surv) = ut

  # create Matdist
  out = distr6::as.Distribution(1 - surv, fun = "cdf", decorators = "ExoticStatistics")
}

## 3) compare
matplot(as.data.frame(round(pred$distr$survival(1:100), 4)), type = "l")

matplot(as.data.frame(round(out$survival(1:100), 4)), type = "l")

^{Created on 2023-10-20 with reprex v2.0.2}

[bblodfon]

I checked the example and I also think we should use the predict of the package. So simplifying the predict code here in mlr3extralearners would be the way to go for lrn("surv.flexible").

What about the surv.parametric, seems complicated enough as well, lots of distr6 code in there :)

[RaphaelS1]

Thanks @bblodfon could you you open a PR to update flexsurv then (which is primarily deleting code!) And I'll review parametric today

[RaphaelS1]

@sebffischer @bblodfon I'm working on this in 2 steps. First I'm adding a new implementation of predict for survreg and will ask @bblodfon to review - we can keep it on branch. Once that's passing and we're happy I'll then move everything to survivalmodels. This is just to reduce moving parts

[bblodfon]

Need this first => RaphaelS1/survivalmodels#45

[bblodfon]

closed via #345

[bblodfon]

@jemus42 FYI, only thing that has changed is that type = "aft" is now form = "aft" + solved some minor issues when fitting the survreg model

[jemus42]

Hmm our benchmark has an renv.lock witha fixed extralearners version - I'm now wondering wether it's better to update and change that one argument in the setup code so it'll work correctly for the new version vs. keep the old version in the benchmark because that's technically more accurate as that's the version we actually ran the benchmark with.

[bblodfon]

when I use renv I install in these cases the specific commit that I need, as the package version in not enough

[jemus42]

Yeah that's what I mean, the lock file currently enforces an older version (0cbdb72) and I'm wondering now if I should bother updating or just leave it, as the actual compute part is done now.

[bblodfon]

Nah, just leave it