Skip to content

model_parameters() unrolls all pairwise correlations for AR(1) structures#1213

Merged
strengejacke merged 16 commits intomainfrom
strengejacke/issue1202
May 6, 2026
Merged

model_parameters() unrolls all pairwise correlations for AR(1) structures#1213
strengejacke merged 16 commits intomainfrom
strengejacke/issue1202

Conversation

@strengejacke
Copy link
Copy Markdown
Member

Fixes #1202

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the package version to 0.28.3.15 and introduces logic to handle special covariance structures, such as AR1 and compound symmetry, within the random variance extraction process. It also includes significant code reformatting for better readability and style consistency. Feedback highlights a critical indexing bug in the covariance structure loop and redundant logic in a switch statement.

Comment thread R/extract_random_variances.R Outdated
Comment thread R/extract_random_variances.R Outdated
Comment on lines +226 to +227
toep = ,
cs =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

These lines in the switch statement are redundant or incomplete. The cs case is already handled above (lines 222-225). Additionally, an empty case like cs = at the end of a switch can lead to errors or unexpected NULL returns in R. If toep was intended to fall through to the cs logic, it should be placed immediately before the first cs block. If not, these lines should be removed.

@strengejacke strengejacke merged commit 38a97e8 into main May 6, 2026
9 of 21 checks passed
@strengejacke strengejacke deleted the strengejacke/issue1202 branch May 6, 2026 14:29
@strengejacke strengejacke requested a review from Copilot May 6, 2026 14:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses #1202 by revising how model_parameters() (via random-effects variance extraction) reports structured covariance parameters from glmmTMB models, aiming to avoid expanding AR(1)/OU/etc. structures into all implied pairwise correlations and instead returning the actual estimated structure parameters. It also updates documentation/examples gating and adds regression tests for several covariance structures.

Changes:

  • Collapses structured glmmTMB covariance-structure outputs (e.g., AR1/OU/CS/Toeplitz) to more concise parameter rows aligned with VarCorr().
  • Adds a new test suite covering AR1, OU, CS, and Toeplitz covariance structures for glmmTMB random effects.
  • Minor documentation/vignette maintenance (examplesIf checks via insight::check_if_installed(), vignette theme initialization, NEWS + version bump).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
R/extract_random_variances.R Adds logic to detect glmmTMB covariance structures and reduce redundant random-effect rows/labels accordingly.
tests/testthat/test-random_effects_covstruct.R New tests validating the revised random-effects output for several glmmTMB covariance structures.
vignettes/demean.Rmd Only sets ggplot theme when required packages are available.
R/4_standard_error.R Updates @examplesIf condition and applies formatting-only refactors.
R/1_model_parameters.R Updates @examplesIf condition for examples.
man/standard_error.Rd Syncs rendered examples condition with roxygen change.
man/model_parameters.default.Rd Syncs rendered examples condition with roxygen change.
man/model_parameters.brmsfit.Rd Documentation update expanding diagnostic option descriptions (ESS tail/bulk).
NEWS.md Notes glmmTMB cov-structure output revision and BSDA htest support.
DESCRIPTION Bumps package version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

re_cor_slope[to_select] <- .update_indicator_rows(re_cor_slope[to_select])
},
toep = {
re_cor_slope[to_select][duplicated(re_data$var2[to_select])] <- FALSE
Comment on lines +243 to +248
new_label <- paste(new_label, seq_along(which(re_cor_slope[to_select])))
}
)
# fix label for cov_structure correlation
re_data[which(re_cor_slope[to_select]), "var1"] <- new_label
re_data[which(re_cor_slope[to_select]), "var2"] <- ""
# keep all random effects parameters, because some are redundant
.update_indicator_rows <- function(indicator_rows) {
if (any(indicator_rows)) {
# we have multiple rows to select for unstructed ("us") covariance structure
Comment on lines +241 to +254
out <- model_parameters(m_toep, effects = "random", verbose = FALSE)
expect_identical(
out$Parameter,
c(
"SD (time1)",
"SD (time2)",
"SD (time3)",
"SD (time4)",
"Cor (Toeplitz Lag 1)",
"Cor (Toeplitz Lag 2)",
"Cor (Toeplitz Lag 3)",
"SD (Observations)"
)
)
Comment on lines +255 to +259
# Validated against output from VarCorr()
expect_equal(
out$Coefficient,
c(0, 1.0112, 0.9844, 1.0525, 1.0346, 0.3893, 0.2383, 0.1979, 0.0041),
tolerance = 1e-3
"SD (time4)",
"Cor (Toeplitz Lag 1)",
"Cor (Toeplitz Lag 2)",
"Cor (Toeplitz Lag 3)",
Comment on lines +280 to +284
# Validated against output from VarCorr()
expect_equal(
out$Coefficient,
c(0.131, 0.5664, 0.5578, 0.5678, 0.6301, 0.8596, -1, 0.0017, -0.0104, -0.0096, 1e-04),
tolerance = 1e-3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

model_parameters() unrolls all pairwise correlations for AR(1) structures

2 participants