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

CIF and CompetingEventTimes #24

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

CIF and CompetingEventTimes #24

wants to merge 3 commits into from

Conversation

tbeason
Copy link
Contributor

@tbeason tbeason commented Jul 27, 2020

This improves upon #4. I slopped this down pretty quickly but let me say first that I did test it against stcompet from Stata and it gave identical results, so I've got that going for me.

I started with the CompetingEventTime as an extension of EventTime. I made it very general, which could probably also be done to EventTime in the same spirit. There is probably a way to mesh them together much better. I do think it is better to herd users toward these types (as opposed to always allowing them to supply lots of vectors) because it allows for easier handling within the estimators if you do this bit of "pre-organization".

The estimator computes only the CIF for the event of interest, not for all events. It is certainly possible to do all of them in a single pass, but I didn't see a quick easy way to do so and keep it flexible like it is now. The downside is that if you need the CIF for all 5 of your competing events, you need to run this estimator 5 times. The upside is that I made it pretty easy to do so by providing a swapeventofinterest function, so that you can quickly convert your event times and rerun the estimator. I think making it compute all of them at once could be done at a later time.

I initially tried to not add a new _estimator method for CumulativeIncidence, but it requires tracking some additional variables over time so I didn't see a way to do that in the existing framework. So, it sort of looks like the existing method but functionally it is not integrated.

I have not implemented any CI tests and some docstring cleanup is needed. Would appreciate some help polishing this up a bit.

@tbeason
Copy link
Contributor Author

tbeason commented Jul 28, 2020

I did some additional testing to verify correctness. In the absence of competing risks, the overall survival curve and the complement of the CIF should both equal the standard Kaplan-Meier curve, and they do. The standard errors should match as well, and they do. Additionally, the sum of the CIFs for all event types should equal the complement of the overall Kaplan-Meier survival curve, and it does.

As mentioned in the other thread, we need to examine what happens in other software with tied times to verify the approach here. I believe it will be the same.

Note for next commit: the number at risk in the variance calc needs to be wrapped in float or else you will hit overflow with large values.

@tbeason tbeason marked this pull request as ready for review July 28, 2020 03:16
@tbeason
Copy link
Contributor Author

tbeason commented Aug 2, 2020

Ok I tested in the case of tied data, the results are identical to stcompet for Stata.

This just needs unit tests and docs.

@tbeason
Copy link
Contributor Author

tbeason commented Aug 2, 2020

I've started adding some docs.

I did find one design decision that is perhaps questionable. Right now I have it so that CompetingEventTime stores the time, the status, the status that corresponds to the event of interest, and the status the corresponds to censoring. These last 2 are not observation-level things though, they apply equally to all elements, so that there is a significant amount of duplication among the data. For small N, probably not much of an issue, but if you have millions of events I think it is perhaps wasteful with memory. Would it be smarter to extend EventTime to allow more general status types instead and then have a competing event times wrapper like

struct CompetingEventTimes{T,S}
    evt::Vector{EventTime{T,S}}
    eventofinterest::S
    cenoringevent::S
end

What do you think?

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

1 participant