Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AalenJohansenFitter.fit(..., timeline=...)currently supplies the reporting grid to the intermediate Kaplan–Meier fit, then aligns those sampled survival values to the full event table. Missing event-time values become NaNs, and their incidence increments are skipped. The raw curve contains NaNs andpredict()can return finite, severely understated estimates. Separately, the first event-table row is always overwritten with CIF=0; this drops an event at the initial time.Calculate survival, incidence and variance on the complete event grid, then forward-sample the estimates and confidence intervals onto the requested sorted timeline. Initialise lagged survival to 1 and retain any first-time event contribution. The event table stays complete and existing jitter behaviour is preserved.
A six-observation fixture
[1,2,3,4,5,6], events[0,1,1,2,2,0], has an exact cause-2 incidence of 0.4 at time 6. Changing only the reporting grid to[0,2,4,6]currently loses increments. A separate first-time fixture checks the incidence against weighted event fractions and an equivalent time-shifted dataset.On the public 35-patient BMT example from Scrucca et al. (https://luca-scr.github.io/R/bmt.csv), using the same fixed jitter seed (12), the original six-month reporting grid returns 0 for both risks at 60 months. The corrected fit returns 0.27533760 for transplant-related mortality and 0.48234443 for relapse, matching a separate scalar risk-set calculation on the identical jittered event times. The original default relapse estimate is 0.45377300; the 1/35 difference is the separately dropped initial event. These are controlled software checks on released example data, not a claim that the original paper used this implementation or reached an incorrect conclusion.
Validation on upstream base
7a8fc34a013ecd79fa405017b89e1697c1cc6e17: 26 added cases, 20 fail and 6 pass before; the complete Aalen–Johansen test class with the additions gives 35 passed after. Coverage includes two causes, full/coarse/off-event/unsorted reporting grids, variance on/off, fixed-seed jitter, case weights, confidence intervals and first-time events. Released lifelines 0.30.3 also reproduces both errors. The full package suite was not run.Prepared with AI assistance. Maintainer acceptance and independent scientific review are not claimed. The Cheerful Duck report provides the reproduction scripts, patch, dataset checksum and before/after execution evidence in a downloadable archive.