search

chjackson / flexsurv

The flexsurv R package for flexible parametric survival and multi-state modelling
http://chjackson.github.io/flexsurv/
55 stars 27 forks source link

Incorporated mean, rmst, q into dfns, Added missing exports #27

Closed jrdnmdhl closed 7 years ago

jrdnmdhl commented 7 years ago
chjackson commented 7 years ago

Thanks. Looks good, just testing it out.

I think the default t in summary(..., type="rmst") should be a scalar, rather than the observation times. It's nicer in the situation where the calculation is slow - also I can't imagine someone wanting to know the "trajectory" of the RMST in the same way as the hazard or survival. What do you think of the default being the maximum follow up time in the data?

Also found a bug in mean_survspline, should have start=0 instead of start=start.

jrdnmdhl commented 7 years ago

Thanks Chris. Believe it or not, generating rmst over a sequence of observation times is a thing! I do a ton of curve fitting for economic evaluations. As part of this process, I often produce figures of RMST (and delta RMST) for time because they are quite good at showing the impact of extending/shortening the timeframe of a projection.

You are right though that the calculation is quite slow when CI=T and when t is a long vector. Sadly, I don't see a good way around this that wouldn't potentially compromise precision in certain cases. I defer to you on what to do with this.

I do like the idea of making the default maximum the maximum time in the data. Can implement that along w/ fixing the bug in mean_survspline.

jrdnmdhl commented 7 years ago

For using the max time as a default, I need to get a much better sense of what obj$data$Y looks like under the different ways that the Surv object can be defined. Not immediately obvious to me if I can safely take the max of one of the columns in order to cover all types of censoring/survival times.

chjackson commented 7 years ago

Merged now. max(obj$data$Y[,"time1"]) is fine I think - this is the event or right-censor time. "time2" is the left-censor time, so that interval censored events happen after time1 and before time2. So I think time1 is a good enough definition of the "follow-up time", for the purpose of this function.

Also I think meansurvspline (or any of the mean functions) shouldn't have a "start" argument - I just meant that start=0 should be set in the call to rmst_generic. I've made these changes now.

jrdnmdhl commented 7 years ago

Apologies, you are right.  Mean should not have a start parameter!  My initial implementation allowed for this, because it was always based on the numeric integral.  Here that gets more complicated/confusing since you couldn’t use the analytical means to calculate mean conditioned on start !=0.

Must have gotten myself mixed up along the way...

From: chjackson notifications@github.com Reply-To: chjackson/flexsurv-dev reply@reply.github.com Date: Saturday, February 18, 2017 at 5:44 PM To: chjackson/flexsurv-dev flexsurv-dev@noreply.github.com Cc: Jordan Amdahl jrdnmdhl@gmail.com, Author author@noreply.github.com Subject: Re: [chjackson/flexsurv-dev] Incorporated mean, rmst, q into dfns, Added missing exports (#27)

Merged now. max(obj$data$Y[,"time1"]) is fine I think - this is the event or right-censor time. "time2" is the left-censor time, so that interval censored events happen after time1 and before time2. So I think time1 is a good enough definition of the "follow-up time", for the purpose of this function.

Also I think meansurvspline (or any of the mean functions) shouldn't have a "start" argument - I just meant that start=0 should be set in the call to rmst_generic. I've made these changes now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.