Simplified & streamlined inference backend + vignette for xgboost backend#293
Merged
Conversation
Demonstrate that a new PAMM estimation backend can be defined in a few lines on top of existing pammtools infrastructure. The vignette wires up xgboost (objective = "count:poisson") via three small functions (as_xgb_data, xgb_pam, get_hazard.pam_xgb), reusing sim_pexp, as_ped, make_newdata, add_hazard and the hazard/survival geoms unchanged, and compares boosted fits against GAM-PAMMs and known truth. Also add xgboost to Suggests and Config/Needs/website.
Introduce an internal `sim_hazard()` generic that factors out the "draw nsim hazard trajectories" step shared by the simulation-based CI helpers (get_sim_ci / _cumu / _surv). The default method reproduces the existing coef-sampling behaviour (RNG order and type-6 quantiles unchanged), so gam/scam results are identical; a custom backend can now supply its own draws (e.g. a bootstrap ensemble) and obtain simulation-based intervals from the same code. Drop the glm/scam class guards in get_cumu_hazard/get_surv_prob and let resolve_time_var default to "tend" when present, so any model providing get_hazard (+ sim_hazard) works with add_hazard/add_cumu_hazard/ add_surv_prob and the geom_* layers. Rewrite the xgboost-backend vignette accordingly: the backend is a bagged bootstrap ensemble expressed via get_hazard.pam_xgb, sim_hazard.pam_xgb and a sample_coefs.pam_xgb no-op, so all add_* functions yield point estimates and bootstrap CIs for free. Examples now recover a known hazard with uncertainty and a time-varying effect, verified against the truth.
The simulation-CI helpers now draw their hazard trajectories once for the whole (grouped) newdata via sim_hazard() and accumulate cumulative quantities within groups, instead of precomputing a coefficient matrix with sample_coefs() and threading it through a per-group split. This removes the only reason a non-coefficient backend (e.g. the xgboost ensemble) had to supply a sample_coefs() no-op: implementing get_hazard and sim_hazard now suffices. Behaviour for glm/scam models is unchanged: draws are still shared across groups (one draw matrix for the whole newdata) and RNG order/type-6 quantiles match, so the existing simulation-CI tests pass as-is. The xgboost vignette drops the sample_coefs.pam_xgb stub accordingly.
Make the post-processing stack backend-agnostic and DRY. Every derived
quantity is now a transform of hazard trajectories obtained from two
per-model primitives:
* get_hazard(object, newdata) -> point hazard vector
* sim_hazard(object, newdata, nsim) -> matrix of hazard draws
get_hazard is repurposed as the lean point primitive (default =
exp(make_X %*% get_coefs)); its former CI/augmentation duties move into
add_hazard. add_hazard/add_cumu_hazard/add_surv_prob compute the point
via the transform of get_hazard and the simulation CI via sim_hazard;
analytic ("default"/"delta") CIs use the make_X/get_coefs/get_Vp triplet
(extracted into hazard_ci) and fall back to simulation for backends that
don't provide it (resolve_ci_type).
add_cif and add_trans_prob no longer sample coefficients / build design
matrices inline: they obtain cause/transition-specific hazards from the
same primitives (CIF stacks the cause-mutated newdata so one sim_hazard
call gives draws coherent across causes). The CIF point estimate is now
the principled plug-in (transform of the point hazard) rather than a
Monte-Carlo mean of draws.
sample_coefs is now only an implementation detail of sim_hazard.default;
removed the dead get_sim_cumu helper. A backend thus needs just
get_hazard + sim_hazard to unlock hazard, cumulative hazard, survival
(and, with the cause/transition indicator among its features, CIF and
transition probabilities) with point estimates and bootstrap CIs.
The xgboost vignette backend collapses to those two methods.
…ckends; DRY sim-CI helpers - Replace resolve_ci_type()'s silent coercion to "sim" with assert_ci_supported(), which errors with a clear message when ci_type "default"/"delta" is requested for a model without the make_X/get_coefs/get_Vp triplet, pointing to ci_type="sim". - Factor the repeated type-6 row-quantile into row_quantile(); reuse it in get_sim_ci, the cumulative helpers, get_cif and add_trans_ci. - Collapse the near-identical get_sim_ci_cumu/get_sim_ci_surv into thin wrappers over a shared sim_cumulative_ci() parameterised by the per-group accumulator. - Sync the xgboost vignette prose with the implementation: the backend interface is get_hazard + sim_hazard (sample_coefs is no longer part of it; make_X/ get_coefs/get_Vp are only for analytic CIs), and the xgb add_hazard() calls now pass ci_type="sim".
Replace the explicit supports_analytic_ci() seam (and the earlier hardcoded glm/scam list) with a simple inherits(object, "lm") check in assert_ci_supported(). Models that carry coefficients (gam, scam, glm, and any future lm-based backend) inherit "lm" and provide coef()/model.matrix()/vcov(), so they support "default"/"delta" CIs automatically; other backends use ci_type = "sim". No opt-in method for backend authors to discover.
…y rows P1 (scam IC crashed): the IC imputation cache and the pooled CI-draw paths hardcoded predict.gam(..., type="lpmatrix"), so pamm_ic(engine="scam") failed because predict.gam rejects scam objects. Route every lpmatrix evaluation through the backend-aware make_X() generic (gam -> predict.gam, scam -> predict.scam): ic_pred_cache() (impute-ic.R), ic_ci_draws() and ic_cif_fit_group() (pool-ic.R), and smooth_term_qoi() (pamm-ic-pool.R). P2 (missing origin row): the pooled adders returned only the prediction grid, so curves started at the first positive time. Mirror the default adders by dropping any pre-existing t=0 row up front and prepending the boundary row at the end via add_cumulative_boundary(): surv_prob=1 (add_surv_prob.pamm_ic), cumu_hazard=0 (add_cumu_hazard.pamm_ic) and cif=0 (add_cif.pamm_ic), so pooled outputs match the default adders.
get_cif.default() built per-cause hazard lists by indexing split() groups (whose names are already sorted) but then overwrote those names with the raw `causes` (factor-level) order. When cause factor levels were not in alphabetical order, haz_by_cause[[cause_data]] then picked the wrong cause's hazard, silently producing an incorrect CIF. Keep split()'s names so the lookup is always correct (verified: CIF now invariant to cause level order). Also guard the CIF increment against division by zero (total hazard == 0), which can occur for arbitrary non-coefficient backends, so the result is 0 rather than NaN.
The pooled cumulative adders (add_cumu_hazard/add_surv_prob/add_cif for pamm_ic) accumulate within each group of `newdata`, like the default add_* methods. If the user forgets to group_by() across covariate (or cause) combinations, distinct curves are silently pooled into one cumulative sum, giving wrong results. Add warn_if_pooled_undergrouped(): when any group still contains repeated time points, emit a clear warning pointing at group_by(). Verified: fires on ungrouped multi-covariate newdata, silent for properly grouped and single-curve grids; added a regression test.
- brms becomes a first-class backend via sim_hazard.brmsfit/get_hazard.brmsfit, so all three baseline-hazard fits (mgcv, jagam, brms) run through one add_cumu_hazard() call instead of bespoke posterior_epred/cumsum/quantile code - precompute the slow JAGS/Stan fits in vignettes/precompute_bayesian.R and load the small pred_df from bayesian_results.rds; render drops from ~11 min to ~8 s - collapse non-didactic chunks (setup, plotting, tables) behind <details> toggles in both the bayesian and xgboost vignettes - add a footnote from the xgboost vignette to the bayesian one as a second worked example of defining get_hazard/sim_hazard - add a References section with proper citations (JAGS, brms, jagam) to the bayesian vignette and an xgboost (Chen & Guestrin 2016) citation to the xgboost vignette - point the get_hazard/sim_hazard roxygen @Seealso at both backend examples Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
vignettes: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors pammtools’ inference backend around a small S3 “backend primitive” interface (get_hazard() / sim_hazard()), expands support for additional model backends (notably scam and user-defined backends like brms/xgboost), and adds a full multiple-imputation workflow for interval-censored survival data, along with corresponding vignettes/tests and regenerated documentation.
Changes:
- Introduces unified backend primitives (
get_hazard,sim_hazard, plus coefficient helpers) to streamline prediction + CI computation across model types/backends. - Adds interval-censored support via MI (
pamm_ic(),pamm_ic_cr()), pooling methods foradd_*, and a helper to generate IC panel data (add_inspections()). - Adds/updates vignettes (shape-constrained
scam, updated Bayesian article workflow, bibliography) and extends test coverage for new behavior.
Reviewed changes
Copilot reviewed 67 out of 76 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/shape-constraints.Rmd | New vignette demonstrating shape-constrained hazard modeling with scam. |
| vignettes/Remote.bib | Adds citations used by new/updated vignettes. |
| vignettes/precompute_bayesian.R | Script to precompute slow Bayesian fits and persist reduced prediction outputs. |
| vignettes/bayesian.Rmd | Updates Bayesian vignette to use unified backend interface + precomputed results. |
| tests/testthat/test-simulation.R | Adds regression test for sim_pexp() factor handling. |
| tests/testthat/test-scam.R | New test suite for scam backend support across add_*, tidiers, plotting. |
| tests/testthat/test-predict-functions.R | Updates expectations for get_terms() output schema (level column). |
| tests/testthat/test-mgcv-convenience.R | Adds tests for get_terms() / gg_smooth() term selection edge cases. |
| R/warnings.R | Exports/registers S3 methods for warn_about_new_time_points. |
| R/tidiers.R | Adds scam tidiers + updates plotinfo extraction to accept scam. |
| R/sim-pexp.R | Hardens simulation against silent factor arithmetic coercion. |
| R/sim-ic.R | Adds add_inspections() for generating interval-censored panel data. |
| R/pool-ic.R | Adds pooled add_* methods for pamm_ic objects (MI pooling of predictions/CIs). |
| R/pammfit.R | Extends pamm() to support engine = "scam". |
| R/pamm-ic.R | Adds pamm_ic() / pamm_ic_cr() MI fitting workflows for IC data. |
| R/pamm-ic-pool.R | Implements fit stripping and pooled summary objects for MI workflows. |
| R/model-helpers.R | Introduces backend primitives + coefficient/covariance accessors to unify inference. |
| R/impute-ic.R | Implements caching + imputation draws for IC (single & competing risks). |
| R/formula-utils.R | Adds RHS-variable resolution helper and internal PED-var exclusions. |
| R/cumulative-effect.R | Switches cumulative-effect calculations to unified coefficient helpers. |
| R/cumulative-coefficient.R | Adds scam method + CI quantile type fix + boundary handling adjustment. |
| R/convenience-plots.R | Updates gg_smooth() behavior for factor-indexed smooths and term selection. |
| R/as-ped.R | Detects IC Surv(..., type=\"interval2\") and routes to IC preprocessing. |
| R/as-ped-ic.R | New IC parsing + preprocessing building blocks (detect_ic, as_ped_ic, etc.). |
| NEWS.md | Documents new IC feature set, scam support, CI quantile changes, convenience fixes. |
| NAMESPACE | Registers new S3 methods/exports and updates imports. |
| man/tidy_fixed.Rd | Documents tidy_fixed.scam. |
| man/strip_pamm_fit.Rd | New docs for fit stripping used in MI workflow. |
| man/smooth_term_rows.Rd | New internal docs supporting revised get_terms() implementation. |
| man/sim_pexp.Rd | Updates docs for factor/character covariate handling in simulation. |
| man/sim_hazard.Rd | New docs for backend primitive sim_hazard(). |
| man/sample_coefs.Rd | New docs for coefficient-draw helper. |
| man/resolve_terms.Rd | New internal docs for term resolution in get_terms(). |
| man/pool_pamm_fits.Rd | New internal docs for MI pooled summary construction. |
| man/parse_by_level.Rd | New internal docs for parsing factor-by smooth labels. |
| man/pamm.Rd | Updates pamm() docs for engine = \"scam\". |
| man/pamm_ic.Rd | New docs for interval-censored MI fitting + print/summary methods. |
| man/pamm_ic_pooling.Rd | New internal docs describing MI pooling approach. |
| man/pamm_ic_cr.Rd | New docs for competing-risks IC MI fitting. |
| man/interval_censored.Rd | New docs for IC detection/parsing/preprocessing helpers. |
| man/impute_ic_times.Rd | New internal docs for IC imputation draws. |
| man/impute_ic_cr.Rd | New internal docs for IC competing-risks imputation draws. |
| man/ic_pred_cache.Rd | New internal docs for IC prediction-grid caching. |
| man/hazard_ci.Rd | New internal docs for analytic hazard CI helper. |
| man/gg_smooth.Rd | Updates gg_smooth() documentation for term selection + factor-indexed curves. |
| man/get_Vp.Rd | New internal docs for covariance extraction helper. |
| man/get_terms.Rd | Updates docs for new get_terms() semantics and output columns. |
| man/get_term.Rd | Updates internal docs for extracting a single smooth curve. |
| man/get_term_legacy.Rd | New internal docs for non-mgcv smooth-metadata fallback. |
| man/get_surv_prob.Rd | Updates docs for pooled CI arguments. |
| man/get_smooth_terms.Rd | New internal docs for enumerating plottable smooths. |
| man/get_sim_cumu.Rd | Removes obsolete internal doc stub. |
| man/get_sim_ci.Rd | Updates internal docs to reflect draw-sharing via sim_hazard(). |
| man/get_plotinfo.Rd | Updates docs to reflect scam support. |
| man/get_hazard.Rd | Updates docs to describe backend primitive get_hazard(). |
| man/get_cumu_hazard.Rd | Updates docs around simulation CI quantile type + pooled args. |
| man/get_coefs.Rd | New internal docs for coefficient extraction helper. |
| man/get_cif.Rd | Updates docs/signature to match refactor. |
| man/cumulative_coefficient.Rd | Updates docs for scam method and baseline/boundary handling. |
| man/compute_cumu_diff.Rd | Updates docs to match refactor + type-6 quantiles. |
| man/add_trans_prob.Rd | Updates docs re: sim CI quantiles + boundary-row behavior. |
| man/add_term.Rd | Updates docs to note scam support. |
| man/add_surv_prob.Rd | Adds default method alias, boundary arg docs, and pooled-method docs. |
| man/add_inspections.Rd | New docs for interval-censoring inspection generator. |
| man/add_hazard.Rd | Updates docs for sim CI quantiles + boundary arg + pooled methods. |
| man/add_cif.Rd | Adds pooled-method docs + boundary-row behavior documentation. |
| DESCRIPTION | Adds xgboost to Suggests and website needs. |
| .gitignore | Ignores Rplots.pdf. |
| _pkgdown.yml | Adds IC + scam articles and reference grouping to pkgdown site config. |
Files not reviewed (7)
- man/add_cif.Rd: Generated file
- man/add_hazard.Rd: Generated file
- man/add_inspections.Rd: Generated file
- man/add_surv_prob.Rd: Generated file
- man/add_term.Rd: Generated file
- man/add_trans_prob.Rd: Generated file
- man/compute_cumu_diff.Rd: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
pkgdown::build_articles_index() requires every vignette to appear in _pkgdown.yml's articles index; the newly added xgboost-backend article was missing, aborting the site build. Add it to articles: contents: and to the Articles navbar dropdown.
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.
add_*functions and CI-calculations so that any new backend ONLY needs to defineget_hazardandsim_hazardS3 methods to make ~everything else work.Includes #292.