A few amends and add some real tests in addition to smoke tests#161
Merged
Conversation
Replaces reliance on a smoke test: verifies the truncation is actually applied to the weights used in the outcome fit. Two clamp bands entirely above the weight range collapse all weights to a constant and, by GLM scale-invariance, give identical coefficients, while a genuinely varying-weight fit differs.
Verifies weight.p99 = TRUE is equivalent to explicitly setting weight.lower/weight.upper to the p01/p99 reported in weight.statistics, and that it differs from an untruncated weighted fit.
Verifies each flag adds or drops its follow-up / trial terms (and their squares) in the fitted outcome-model coefficients, leaving the other flag's terms intact.
Verifies followup.class = TRUE encodes follow-up as a factor: the linear followup / followup_sq terms are gone and there is one dummy coefficient per non-reference follow-up level.
Previously untested. Verifies weight.lag_condition = TRUE fits each arm's weight model only on its treatment-lag stratum (per-arm observation counts differ and sum to the full data), while FALSE fits both arms on the full data (equal counts).
Verifies the expanded data (via expand.only) is clamped to the requested [followup.min, followup.max] window, while the unrestricted expansion spans beyond it.
Verifies each arm's weight model is fit only on rows where its weight.eligible_cols indicator == 1, by checking the per-arm denominator observation counts drop versus an unfiltered fit.
They looked up non-existent fields (n0.coef/d0.coef) instead of the coef.numerator/coef.denominator lists actually stored in weight.statistics, so they silently returned NULL for every weighted model. Index the per-arm models correctly and guard the ITT case where no treatment-weight models exist. Add a regression test.
Clarify in the weight.lower/weight.upper/weight.p99 docs that weight.statistics and the returned data report the untruncated weights; truncation affects only the weights used to fit the outcome model.
Routes the weight numerator/denominator/LTFU/visit predictions through the cached formula path, matching the survival and hazard paths. Results are bit-identical; this is a consistency cleanup, not a perf change.
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.
Hi Ryan,
PR from last week's work.
SEQuential()fit, populateweight.statisticsandoutcome.modelwhenhazard = TRUE.numeratoranddenominatorweight models are given identical covariates. In that case the stabilized weights all equal 1 (i.e., no weighting), which is usually a typo in thedenominatorargument.SEQuential()helpfile by adding a per-protocol example.selection.random = TRUEretains all treated trial-starts, subsamples control trial-starts to the requestedselection.probfraction, and is reproducible under a fixed seed; rename the previous smoke test that did not actually exercise the feature.weight.lower/weight.uppertruncation,weight.p99truncation,followup.include/trial.include,followup.class,weight.lag_condition,followup.min/followup.max, andweight.eligible_cols.numerator()anddenominator()returningNULLfor every weighted model; they now return the fitted per-arm numerator/denominator weight models.inline.pred()in the weight modelsSEQOpts()argument ordering.