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

Add functionality to grpsurv for start stop survival times #45

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

Conversation

jwallib
Copy link

@jwallib jwallib commented Jul 14, 2021

@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.

@pbreheny
Copy link
Owner

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.

@pbreheny
Copy link
Owner

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.

@pbreheny pbreheny closed this Jul 21, 2021
@pbreheny pbreheny reopened this Jul 21, 2021
@pbreheny
Copy link
Owner

Sorry -- accidentally hit the close button!

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.

2 participants