Fix splines and add followup_splines_df parameter#47
Merged
Conversation
…oss bootstraps `cr(followup, df=3)` in patsy computes interior knot positions from the quantiles of the training data. When bootstrapping, each replicate is fit on a resampled dataset that may have a different followup distribution, causing the spline basis to differ between the original model and bootstrap models, and between bootstrap replicates. Fix this by computing knot positions (interior knots, lower bound, upper bound) once from the full unexpanded dataset at the start of the original fit, storing them on the object, and embedding them as literals in the formula string passed to patsy for all subsequent fits (bootstrap replicates included). This mirrors the fix applied to the R package SEQTaRget.
`I(cr(followup, df=3)**2)` was being appended to the formula to represent the non-linear followup effect, but this squares the spline basis matrix element-wise rather than including the spline basis directly. When no treatment-by-followup interaction term is present (e.g. hazard estimation or km_curves=False), this meant the model contained only a squared spline with no main spline effect, completely misrepresenting the followup relationship. Replace the squared term with the spline basis itself. When an interaction term such as `treatment*cr(followup, ...)` is also present, patsy's formula expansion already includes the spline as a main effect and deduplicates it. Update expected test coefficients accordingly.
Add `followup_spline_df` (default 4) to `SEQopts` to allow users to control the number of degrees of freedom for the natural cubic spline fit to followup, matching the `followup.spline.df` parameter introduced in the R package SEQTaRget. The default changes from the previously hardcoded value of 3 to 4, consistent with R. A validation check ensures the value is at least 2, which is the minimum supported by patsy's natural cubic spline implementation. Update expected test coefficients accordingly.
ryan-odea
approved these changes
May 5, 2026
Collaborator
ryan-odea
left a comment
ryan-odea
left a comment
There was a problem hiding this comment.
These look good! Thanks for tackling - got a bit busy last/this week with some finishing up of other projects
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
2 participants
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.
The fixes here for the splines are much smaller.
This,
(Had to modify expected values of a test.)