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

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

Merged
merged 5 commits into from
Feb 18, 2017
Merged

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

merged 5 commits into from
Feb 18, 2017

Conversation

jrdnmdhl
Copy link
Contributor

  • Added functions for mean/restricted mean for all distributions
  • Renamed all existing mean functions so that roxygen2 didn't confuse them as S3 methods
  • q/mean/rmst functions automatically included in dfns for all distributions
  • implemented type="median" for summary.flexsurv
  • Added missing exports
  • Added extra tests

-Made use of analytical means/rmst where available
-q/mean/rmst functions automatically added to distributions
-added type="median" to summary.flexsurvreg
-Removed unncessary "t" column from summary when type="mean" or "median"
-Added additional tests
-Added missing functions to namespace
-Renamed mean/rmst functions to avoid getting confused as S3 methods
@chjackson
Copy link
Owner

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
Copy link
Contributor Author

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.

-Added missing space after comma
@jrdnmdhl
Copy link
Contributor Author

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 chjackson merged commit 2fa3929 into chjackson:master Feb 18, 2017
@chjackson
Copy link
Owner

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 mean_survspline (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
Copy link
Contributor Author

jrdnmdhl commented Feb 18, 2017 via email

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.

None yet

2 participants