search

jgellar / pcox

Penalized Cox regression models
1 stars 0 forks source link

Allowing "t" in the model formula to specify time-varying effects #38

Closed jgellar closed 7 years ago

jgellar commented 8 years ago

In order to fit a time-varying effect (i.e., \beta(t)*x), right now the user enters the term as p(x, tv=TRUE, linear=T). I was playing around and thought, what would happen if the user entered p(x, t, linear=T)? I debugged the fit, and I was able to get all the way into p without an error. The reason for this is because (fortunately, I guess) t() is an R function. Within p, I can do the following:

dots <- list(...)
which.t <- sapply(dots, identical, t)

and which.t will point right to the "t" entry. We can then proceed by taking that entry out of dots, and flipping tv=TRUE on.

Do you have any thoughts on this? It seems like it would make the user interface a little easier, but perhaps harder to document. We would have some examples to show this functionality.

fabian-s commented 8 years ago

doesn't seem like safe idea to me: what if the user's time variable has a different name than t ?

On Fri, Jul 24, 2015 at 8:15 PM, jgellar notifications@github.com wrote:

In order to fit a time-varying effect (i.e., \beta(t)*x), right now the user enters the term as p(x, tv=TRUE, linear=T). I was playing around and thought, what would happen if the user entered p(x, t, linear=T)? I debugged the fit, and I was able to get all the way into p without an error. The reason for this is because (fortunately, I guess) t() is an R function. Within p, I can do the following:

dots <- list(...) which.t <- sapply(dots, identical, t)

and which.t will point right to the "t" entry. We can then proceed by taking that entry out of dots, and flipping tv=TRUE on.

Do you have any thoughts on this? It seems like it would make the user interface a little easier, but perhaps harder to document. We would have some examples to show this functionality.

— Reply to this email directly or view it on GitHub https://github.com/jgellar/pcox/issues/38.

jgellar commented 8 years ago

For what I'm proposing, t does NOT refer to a specific variable. The user's time variable will be whatever they put in the Surv() part of the formula. However, I'm proposing letting the user enter p(x, t) to indicate a time-varying effect, as an alternative to p(x, tv=TRUE). Again, the t does not refer to any variable, it's just a shortcut - pcox will know what the real time variable is. This is just meant to allow the user to enter the formula in a more "natural" way.

fabian-s commented 8 years ago

oh, ok, sorry, didn't understand that. that does seem like a really nice interface idea.

OTOH: in the long run it's probably better to have a smaller package with well tested and documented features than lots and lots of features that are only partially documented or tested.... invest your time wisely ;)

On Sat, Aug 1, 2015 at 3:10 AM, jgellar notifications@github.com wrote:

For what I'm proposing, t does NOT refer to a specific variable. The user's time variable will be whatever they put in the Surv() part of the formula. However, I'm proposing letting the user enter p(x, t) to indicate a time-varying effect, as an alternative to p(x, tv=TRUE). Again, the t does not refer to any variable, it's just a shortcut - pcox will know what the real time variable is. This is just meant to allow the user to enter the formula in a more "natural" way.

— Reply to this email directly or view it on GitHub https://github.com/jgellar/pcox/issues/38#issuecomment-126843134.

jgellar commented 8 years ago

Yes, I know I have been a little ADD on this. The reason I thought about this was mostly for myself - I was fitting a time-varying model using p(x, tv=TRUE), and just thought it would be easier to type p(x,t). So I was curious if I could make that happen.

I agree that I should be focusing on the main functionality before adding enhancements (which could always be added later). The main things remaining that are necessary are to get something working to extract the baseline hazard function, run some more checks on the predict/cross-validation code, and clean up some documentation.

I've been busy with other things the past couple weeks, but hopefully I can find time this week before JSM.

fabian-s commented 7 years ago

I'm closing this because it doesn't seem like something we'll work on for the foreseeable future.....