survival icon indicating copy to clipboard operation
survival copied to clipboard

concordance with start/stop data and timewt='S' crashes R

Open Shaunson26 opened this issue 11 months ago • 3 comments

concordance() with start/stop data and timewt = S crashes R.

I've narrowed it done to the C script Cfastkm2.

Here is a reprex

library(survival)

fit1 <- 
  coxph(Surv(tstart,tstop, status) ~ inherit, data = cgd)

# concordance(fit1, timewt='S') # crash
# concordance() -> concordancefit() -> docount() -> Cfastkm2

y = fit1$y
wts = rep(1, length(y))
risk = fit1$linear.predictors
sort.stop <- order(-y[, 2], y[, 3], risk) - 1L
sort.start <- order(-y[, 1]) - 1L

.Call(survival:::Cfastkm2, y, wts, sort.stop, sort.start)

Shaunson26 avatar Mar 14 '24 15:03 Shaunson26

I tried this out of pure curiosity and can confirm that this indeed crashes R-Studio

RobinDenz1 avatar Mar 16 '24 07:03 RobinDenz1

I also tried in R, and outside Rstudio, and it still crashes. There was once an issue with another package that crashed Rstudio but not R.

Shaunson26 avatar Mar 16 '24 07:03 Shaunson26

This is serious stuff. What is surprising is that I can make it happen with R sometimes, but not when I turn on valgrind, the tool that would help me find it. Working on it....

therneau avatar Mar 18 '24 03:03 therneau

  1. You gave a great example 2. I think time weights (Uno paper) is a tempest in a teacup so don't pay much attention to it, and thus never created an example in my test suite -- big mistake. 3. There were two serious flaws, one in init.c that caused the crash (fastkm1 appeared twice), and another that gives wrong results (sort1 before sort2 in the C code, sort2 before sort1 in the call to the C code).
    Those are fixed, now working on an addition to the test suite.

Now resolved; the code has been updated

therneau avatar Apr 25 '24 21:04 therneau