Uh oh!
There was an error while loading. Please reload this page.
Fix the example in the README - #26
Merged
Merged
Conversation
ryan-odea
requested changes
Dec 9, 2025
ryan-odea
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for noticing!
I was just about to PR some vignettes and the readme update and you beat me to it! - I think load_data() needs to be load_data("SEQdata")
remlapmotforce-pushed
the
fix-example-typo
branch
from
December 10, 2025 07:11
11e4ff6 to
8e84778Compareremlapmot
commented
Dec 10, 2025
ContributorAuthor
Thanks Ryan, I've added that. It was working without it as you have specified that as the default value of the argument in the function definition but I agree it makes it clearer with it. |
Uh oh!
There was an error while loading. Please reload this page.
remlapmot added a commit
that referenced
this pull request
Jun 9, 2026
The hazard ratio is computed by a g-formula Monte-Carlo simulation that needs the fitted outcome model, so running the bootstrap across a process pool requires those models to survive a pickle roundtrip. glum's _GlumFit held a patsy DesignInfo, which raises NotImplementedError on pickle (patsy #26), so parallel=True and offload=True both crashed under glm_package="glum". Three fixes make the process-pool route work: - _GlumFit now records the formula and a small reference frame and rebuilds its DesignInfo in __setstate__, instead of pickling the DesignInfo. Safe here because _cast_categories freezes categorical level order and the models use only stateless transforms (precomputed squares, explicit-knot splines). Roundtrip preserves params, bse, and predictions exactly. - SEQuential.__getstate__ drops the glum-only _patsy_design_cache, which also holds (unpicklable) DesignInfo objects and otherwise rode along when the object crossed a process boundary. - _bootstrap_worker now calls the raw, undecorated fit body via __wrapped__ rather than the @bootstrap_loop-wrapped method. Going through the wrapper re-entered bootstrap_loop and returned [model_dict] (a list) instead of model_dict, crashing the hazard/survival consumers that index outcome_model[i]["outcome"]. This was a pre-existing bug affecting both the statsmodels and glum backends. parallel=True now bit-matches the serial hazard ratio + CI for both backends; offload=True + glum and fit(parallel=True) + glum both work. Tests: - test_glum.py: _GlumFit pickle roundtrip preserves params and predictions. - test_parallel.py: parallel hazard matches serial (statsmodels and glum).
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 freeto 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.
In trying to use this on our short course practical I just noticed that the README example no longer runs.
Hopefully this is the fix.