Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

start antolini implementation #343

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

start antolini implementation #343

wants to merge 3 commits into from

Conversation

RaphaelS1
Copy link
Collaborator

No description provided.

@RaphaelS1 RaphaelS1 linked an issue Dec 22, 2023 that may be closed by this pull request
@RaphaelS1
Copy link
Collaborator Author

Hi @bblodfon this starts an Antolini implementation. I've used a new method for the Cindex just using R vectorisation. To demonstrate it I've shown how you can also use it with Harrell's C and it produces identical results (within 1e-6) at significantly faster time. The current Antolini implementation might be broken though as frequently seeing C<0.5, I suspect the bug is in lines 6-7 when calling the distr6 function, as you did this recently with several models/measures please could you take a look at that line? Once happy I'll code it into an actual measure

R/antolini.R Outdated Show resolved Hide resolved
R/antolini.R Outdated Show resolved Hide resolved
@bblodfon
Copy link
Collaborator

bblodfon commented Feb 19, 2024

with the updated code, the use of the distr6 function is correct now, but still we get Antolini C-indexes < 0.5 so there is probably something wrong in the logic of the code itself?

What doesn't make sense to me is that scores coming from S(T) from Antolini's and the crank from Harrell's are "treated" the same way in the code but haven't checked into more details yet...

I did a small benchmark to check the output values:

set.seed(42)
bmr = benchmark(benchmark_grid(
  tasks = tsks(c("rats", "gbcs", "grace")),
  learners = lrn("surv.coxph"),
  resamplings = rsmp("cv", folds = 3)
))
bmr$score()$surv.cindex 

for (i in 1:3) {
  for (p in bmr$resample_results$resample_result[[i]]$predictions()) {
    print(cindex(pred = p, meth = "A")) # "H" => checking Harrell's C is the same as above (YES)
  }
}

@RaphaelS1
Copy link
Collaborator Author

with the updated code, the use of the distr6 function is correct now, but still we get Antolini C-indexes < 0.5 so there is probably something wrong in the logic of the code itself?

What doesn't make sense to me is that scores coming from S(T) from Antolini's and the crank from Harrell's are "treated" the same way in the code but haven't checked into more details yet...

I did a small benchmark to check the output values:

set.seed(42)
bmr = benchmark(benchmark_grid(
  tasks = tsks(c("rats", "gbcs", "grace")),
  learners = lrn("surv.coxph"),
  resamplings = rsmp("cv", folds = 3)
))
bmr$score()$surv.cindex 

for (i in 1:3) {
  for (p in bmr$resample_results$resample_result[[i]]$predictions()) {
    print(cindex(pred = p, meth = "A")) # "H" => checking Harrell's C is the same as above (YES)
  }
}

Well not treated the same, the inequality is reversed https://github.com/mlr-org/mlr3proba/pull/343/files#diff-033f1cd099ed95e401392d6763a5b44330ec434215f754990b0c0215220a7538R32-R36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Antolini's time-dependent C
2 participants