Add G-computation correction for emmeans, contrasts argument, and expanded emp_start#567
Add G-computation correction for emmeans, contrasts argument, and expanded emp_start#567apanagos-lilly wants to merge 10 commits into
Conversation
…o emmmeans_gcomp_subject_data
danielinteractive
left a comment
danielinteractive
left a comment
There was a problem hiding this comment.
Nice start, thanks @apanagos-lilly ! Please see comments below.
For next time, let's please try to have separate PRs for the separate enhancements as much as possible (here it would be 3 as far as I can see). That usually speeds up reviews, resolution and merges.
One additional high level comment:
I wonder if there is a nice way we can print to the user when they use emmeans on an mmrm with g-computation arguments that average treatment effect estimates are calculated. To basically highlight this as message, or hook into the show/print of emmeans results or similar, to make it very visible for the user.
|
|
||
| ### New Features | ||
|
|
||
| - `mmrm()` and `fit_mmrm()` now accept a `contrasts` argument, allowing users to specify contrast matrices or functions for factor variables, matching the interface of `lm()`. When an explicit contrast matrix includes levels not present in the fitting data, those levels are preserved in the model and marked as aliased, enabling prediction on new data containing those levels (#562). |
There was a problem hiding this comment.
| - `mmrm()` and `fit_mmrm()` now accept a `contrasts` argument, allowing users to specify contrast matrices or functions for factor variables, matching the interface of `lm()`. When an explicit contrast matrix includes levels not present in the fitting data, those levels are preserved in the model and marked as aliased, enabling prediction on new data containing those levels (#562). | |
| - `mmrm()` and `fit_mmrm()` now accept a `contrasts` argument, allowing users to specify contrast matrices or functions for factor variables, matching the interface of `lm()`. When an explicit contrast matrix includes levels not present in the fitting data, those levels are preserved in the model and marked as aliased, enabling prediction on new data containing those levels. |
so far we don't link to any GH issues so no need for this
| person("Daniel D.", "Sjoberg", , "sjobergd@gene.com", role = "aut", | ||
| comment = c(ORCID = "0000-0003-0862-2018")), | ||
| person("Nikolas Ivan", "Krieger", , "nikolas.krieger@experis.com", role = "aut", | ||
| comment = c(ORCID = "0000-0002-4581-3545")), |
There was a problem hiding this comment.
please add yourself as author or contributor here
| visit_var = visit_var, | ||
| subject_var = subject_var, | ||
| subject_groups = subject_groups, | ||
| cov_type = "us" |
There was a problem hiding this comment.
should we still check that additional arguments are ignored?
| subject_groups = fit$tmb_data$subject_groups, | ||
| cov_type = fit$formula_parts$cov_type | ||
| ) | ||
| expect_length(result, 2L) |
There was a problem hiding this comment.
can we please keep comparing to the numbers as before? I want to see that this stays stable
| ) | ||
| }) | ||
|
|
||
| # emp_start per covariance type ---- |
There was a problem hiding this comment.
this should be moved upwards such that all emp_start tests are together in one section of this file
| # Start with the model-based covariance. | ||
| V <- component(object, "beta_vcov") | ||
|
|
||
| if (!is.null(object$emmeans_gcomp_vars)) { |
There was a problem hiding this comment.
can we move this into helper function and apply here? otherwise this thing gets too long
| aliased_pos <- which(is.na(beta_hat)) | ||
|
|
||
| # Apply G-computation correction if specified. | ||
| visit_var <- object$formula_parts$visit_var |
There was a problem hiding this comment.
will come back for review once algorithm vignette is there
|
|
||
| contrasts_preserve_vars <- NULL | ||
| if (!is.null(contrasts)) { | ||
| formula_factors <- intersect( |
There was a problem hiding this comment.
ok we need some explanations here. either in comments or in the function documentation. please also refer to how lm() is doing this, is it the same or different and why
| #' `emp_start` supports all non-spatial covariance types. | ||
| #' It uses linear regression to first obtain the coefficients | ||
| #' and uses the residuals to obtain the empirical | ||
| #' variance-covariance matrix. This is then decomposed into |
There was a problem hiding this comment.
can you please include more algorithm details here? as this is the only place where this is documented
| # mmrm contrasts argument ---- | ||
|
|
||
| test_that("mmrm accepts contrasts argument with contrast function", { | ||
| fit <- mmrm( |
There was a problem hiding this comment.
can we also check that the result is as expected? I mean the coefficients for ARMCD
3 participants
Adds emmeans_gcomp_vars to mmrm_control(). When set, emmeans() returns the average treatment effect with standard errors that account for covariate variability across subjects. Default is NULL, when NULL, mmrm runs identically as before. Related to #560, though the counterfactual path does not support variance correction and has 50x runtime overhead only addressable through emmeans changes.
mmrm() and fit_mmrm() now accept a contrasts argument for specifying contrast matrices or functions, matching lm(). When a contrast matrix includes levels not present in the fitting data, those levels are preserved and marked as aliased, enabling prediction on new data.
emp_start() now supports all non-spatial covariance types, not just unstructured (#562).