-
Notifications
You must be signed in to change notification settings - Fork 23
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
To do list: porting AcceleratedFailure #5
Comments
Excellent list! I'm not sure when I'll have time to do the refactoring to avoid duplication between Kaplan-Meier, cumulative incidence, and Nelson-Aalen, so a PR for that would be welcome if you're up for it. I took a stab at it a couple weeks ago but was pretty unsatisfied with the result, so I never pushed the branch. The general idea was to have a mutable state object for each estimator that gets updated in the loop, then all that's required for each estimator is defining methods for an updating function. That may still be a viable approach; my implementation of it just left a lot to be desired. It's also unclear to me how best to read and write Gradients and hessians could potentially be handled by a dependency on a package like ForwardDiff, and optimization with Optim and LineSearches. I'd be fine with dependencies on those packages. |
I'm unsure that my implementation will be better than yours... I guess one could have an EDIT: actually it's much easier to refactor to add Nelson Aalen without code duplication, as exactly the same quantities are needed, the cumulative incidence case seems harder As for the gradient and hessian tools, IIRC when I developed my stuff it was not possible to use automatic differentiation as most of the distributions pdfs and cdfs were computed with some external R library. I'm not sure if that's still the case. If not I'd be curious to compare the solution in AcceleratedFailure with say ForwardDiff. |
I don't think R really supports writing and loading this kind of data to/from CSV. Writing a |
I think we would need to first overload Base.parse(T::Type{EventTime{S}}, str::String) where {S} =
endswith(str, '+') ? EventTime(parse(S, chop(str)), false) : EventTime(parse(S, str), true) And then the following should work: CSV.read(filename, types = Dict("event" => EventTime{Int64})) I'm not sure if there's a way to save this information in the file so that the user doesn't have to provide it every time. |
Now that (hopefully) the Cox PR is nearing merging, and following on #3, I wanted to make a checklist of features to port from AcceleratedFailure (the other survival package, I've renamed it to avoid name clashing).
1 and 2 are almost done, 3 and 4 will be next. They will probably require some refactoring as the code is extremely similar for Nelson-Aalen, Kaplan-Meier and cumulative incidence. @ararslan : if you prefer I can wait that you refactor the code for cumulative incidence before adding Nelson-Aalen(in which case I would start by porting the tools for differentiation of cumulatives), otherwise I can try refactoring myself.
The text was updated successfully, but these errors were encountered: