Repository navigation
Add modelling the baseline hazard in the weight models as a flexible function of time - #61
Merged
Merged
Conversation
…ight models Add opt-in weight_spline and weight_spline_df to SEQopts, replacing the time quadratics in the default weight models with a natural cubic spline basis. Generalise spline knot baking to every cr(x, df=N) term in the outcome and weight model formulas, so the basis is constant across bootstrap resamples and survives the glum backend's formula re-parse on unpickle. Validate formula argument types in SEQopts, and read column names out of function-call terms in _col_string.
…="center" Neither warning the weight-spline tests emit marks a bad fit, so filter them at module level with the reasoning. "separation detected" is a false positive of _check_separation's |coef| > 25 rule: the last cr() basis column has a small numeric scale, so its coefficient is legitimately large (~62 with a standard error of ~35, i.e. z ~ 1.8) and every weight model converges. "failed to converge" comes only from the tests that hand-write an unconstrained cr(x, df=N), whose basis spans the constant function and is therefore collinear with the model intercept by one dimension; statsmodels reports the redundancy and pinv resolves it. Since the terms weight_spline generates are centred and avoid this entirely, tell users writing their own cr() terms to pass constraints="center" too - statsmodels only limps through without it and the glum backend cannot fit at all. Also correct the interior knot count in the vignette, which the centring changed from 2 to 3.
ryan-odea
approved these changes
Sep 22, 2026
ryan-odea
left a comment
ryan-odea
left a comment
Collaborator
There was a problem hiding this comment.
Overall looks good! I shortened the SEQopts comments a bit - hoping its less for an end-user to chew through.
Contributor
Author
|
Thanks Ryan |
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.
This adds the last PR which went into SEQTaRget - modelling the baseline hazard in the weight model as a flexible function of time by adding an opt-in
weight_splineoption toSEQopts().When enabled, the default weight models use natural cubic splines of the time terms in place of the quadratics -
followup/followup_sqandtrial/trial_sqwhenweight_preexpansion=False, the time column and its square whenweight_preexpansion=True- with knots fixed from the data the models are fit on so the basis is identical across bootstrap resamples. This covers the treatment weight models and, where specified, thecenseandvisitmodels. The default is unchanged (weight_spline=False), and the option is the weight-model counterpart offollowup_spline.Also,
cr(x, df=N)term in the outcome and weight model formulas rather than just thefollowup_splineoneSEQopts()