Fix/refuse rank deficient fit - #76
Merged
Merged
Conversation
A rank-deficient design makes glm() return NA for one or more coefficients, whether from two predictors carrying the same information or a saturated fit with as many coefficients as rows. Those NAs then flowed into the optimizer, every predicted outcome became NA, and the run failed downstream. The package learned to name the aliased terms in that downstream failure, but only after letting the NAs propagate through the whole optimization first. The fit is now refused where it is made, in outcome_model_fitting(), right after the convergence check and before the non-fatal diagnostics, so a model that cannot be estimated is rejected before any intervention is tried rather than after all of them fail. The condition is anyNA(coef(model)), the earliest point the coefficients exist, and the one caller of outcome_model_fitting() is lago_optimization(), so nothing else that wanted such a fit is affected. The message is the existing rank_deficient_outcome_message(), which already named the aliased terms and gave the remedy. Its opening was past tense and described a downstream event, "No outcome could be estimated at any of the interventions tried", which does not fit a refusal made before any intervention is tried. It now opens "No outcome can be estimated from the outcome model", which reads correctly both here and at the two downstream sites that still call it, and neither is weakened: those guards remain for a direct caller of the exported get_confidence_set(), which does not pass through this fit. Nothing changes for a model that is estimable: across a sweep of valid configurations every returned value is identical. The one integration test that drove a rank-deficient fit through lago_optimization() now meets the refusal at the fit, with the same message on both optimizer paths, so its assertions still hold without editing.
…zer uses The refusal added a commit ago fired on any NA coefficient, which is broader than the harm it was guarding against. The optimizer reads the intercept, the intervention components and their interaction terms, the fixed center and time effects, and the center characteristics; it never reads the additional covariates. So an aliased additional covariate has an NA coefficient the optimization never touches, and the recommendation is exactly the one the fit without that covariate produces. Refusing that run rejected an optimization that was valid: on the released version the same call returns a recommendation, byte for byte the drop-the-covariate result. The check now refuses only when an aliased coefficient is one the optimizer reads. The set of those names is built with the same helpers rec_int_processor() uses to read them, so the two cannot drift and a factor covariate's level-named coefficient or a covariate named like a fixed-effect term is classified the same way in both places. An aliased intervention component or a saturated center fit still refuses, since those coefficients are read. An aliased additional covariate now warns rather than refuses: the recommendation is unaffected, but a covariate glm() dropped contributes nothing to the outcome and the caller may not have intended that. This matches the existing model-fit diagnostics, which warn about a questionable fit and let the optimization continue. Valid runs are unchanged, including runs with additional covariates that are not aliased.
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
1 participant
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.
Refuse a rank-deficient outcome model at the fit
A rank-deficient design makes
glm()returnNAfor a coefficient — twopredictors carrying the same information, or a saturated fit. Those
NAs flowedinto the optimizer and every predicted outcome became
NA, so the run faileddownstream after trying every intervention. The package learned to name the
aliased terms in that downstream failure (#73); it now refuses the fit up front,
in
outcome_model_fitting()right after the convergence check, before anyintervention is tried.
Scoped to what the optimizer reads. The refusal fires only when an aliased
coefficient is one the optimization uses: intercept, intervention components and
their interaction terms, fixed centre and time effects, centre characteristics.
An aliased additional covariate does not refuse — the optimizer never reads
it, so the recommendation equals the drop-that-covariate run — it warns instead.
The used-coefficient set is built with the same helpers
rec_int_processor()reads them with, so classification cannot drift and a factor covariate's
level-named coefficient or a covariate named like a fixed effect is handled the
same in both places.
The message is the existing
rank_deficient_outcome_message(), reworded frompast-tense ("could be estimated at any of the interventions tried") to present
("can be estimated from the outcome model") so it reads correctly both at the new
fit-site and the two downstream sites still reachable via the exported
get_confidence_set().