Open jwallib opened 3 years ago
Hi @jwallib -- this looks great so far! My only concern is that it I feel it needs a test to make sure that this is working correctly. One easy test would be the following (agrees with survival package when lambda = 0):
# start, stop works
start <- rexp(100)
stop <- start + rexp(100)
event <- rbinom(100, 1, 0.8)
X <- matrix(runif(200), 100, 2)
fit.mle <- coxph(Surv(start, stop, event) ~ X)
fit <- grpsurv(X, cbind(start, stop, event), lambda.min=0)
expect_equivalent(coef(fit.mle), coef(fit, lambda=0))
Could also test that it agrees with glmnet in the special case of group lasso where every feature is in a group by itself (and is thus equivalent to the regular lasso). These tests could be in a separate start-stop file (in inst/tinytest
) or at the end of the grpsurv.R
test file.
I tried this (comparison with survival), and they didn't agree, so maybe there are still some issues to be worked out?
Also, at some point we'll need to update the documentation (.Rd
file), but no hurry on that.
Just FYI, I'm planning to submit a new version of grpreg to CRAN tomorrow (3.4). Since there still seem to be some issues with this code not agreeing with survival, I think I'll hold off on merging this PR for now, but I would love to incorporate this into grpreg 3.5.
Sorry -- accidentally hit the close button!
@pbreheny I have modified the code to enable start, stop survival times as mentioned in issue #43. This brings grpsurv in line with the functionality of glmnet.
Work still to be done: include a way of 'clustering' rows so that individuals split across multiple rows due to time varying covariates are treated as repeat measurements rather than completely independent rows. (See cluster term of coxph function here)
Happy to discuss any of the changes that I've made.