Skip to content

Fix term matching in check_collinearity() for clm scale and nominal effects, more vglm families, and backticked multinomial names - #992

Merged
strengejacke merged 9 commits into
easystats:mainfrom
jmgirard:vif-term-matching
Oct 19, 2026
Merged

strengejacke merged 9 commits into
easystats:mainfrom
jmgirard:vif-term-matching

Conversation

@jmgirard

Copy link
Copy Markdown
Contributor

check_collinearity() matched coefficients to the wrong terms, or to no terms, for three kinds of models.

What changes

  • ordinal::clm() with scale effects. The variance-covariance matrix has the thresholds first, then the location slopes, then the scale effects. A scale effect can have the same name as a location slope. An example is x1 in clm(y ~ x1 + x2 + x3, scale = ~ x1). The slopes were kept by name, so the VIF of x1 also used the scale column. clm slopes are now kept by position. For clm fits with scale or nominal effects, a message names the effects that the VIFs leave out. A location term that is also a nominal effect, which clm drops, is no longer reported as rank deficient.
  • VGAM::vglm() with the sratio(), cratio(), acat(), and poissonff() families. These fits took the default branch with the assign list of the model matrix. When a term had more than one column, they failed with "'list' object cannot be coerced to type 'integer'". Otherwise they gave VIFs for the wrong terms. They now use the branch that the multinomial and cumulative families use. The Details section of ?check_collinearity names the new families.
  • Multinomial models from nnet, mclogit, and brglm2 with backticked names, such as y ~ `my x` + x2. insight::get_varcov() drops the backticks from the column names, while coef() and the model matrix keep them. These fits returned NULL with "Could not extract model terms". Backticks are now removed on all sides before matching.
  • The rank-deficiency block can get a matrix with the rank_deficient attribute, no na_columns_index, and no NA. For that matrix, it used na_cols before it was set. na_cols now starts as NULL.

Tests

  • clm with scale = ~ x1 and with nominal = ~ x3, against the generalized VIF of the location slope columns of vcov().
  • vglm fits with the five families (and sratio(parallel = TRUE)), against the generalized VIF over the columns that the assign list gives each term. The poissonff fit also matches glm(family = poisson).
  • nnet, brmultinom, bracl, parallel bracl, and mblogit fits with a backticked name. They must match the same fits with the column renamed my_x.
  • A mocked insight::get_varcov() with the rank_deficient attribute and no NA columns.

These tests fail on main and pass with this change. The one failing test in devtools::test(), test_performance - lavaan in test-test_performance.R, fails the same way on main.

#990 and #991 also change R/check_collinearity.R and NEWS.md.

Copilot AI left a comment

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.

🔵 Needs a closer look

Split the optional mclogit test dependency and document its backticked-name fix in NEWS.

No open issues 🎉

1 file not reviewed
  • man/check_collinearity.Rd: Generated file
What changed in this PR

Fixes term-to-coefficient matching in check_collinearity() across CLM, VGAM, and multinomial models.

Changes:

  • Corrects CLM, VGAM, and backticked-name matching.
  • Improves rank-deficiency handling.
  • Adds regression tests and updates documentation, NEWS, and version metadata.
File Summary
tests/​testthat/​test-check_collinearity.R Adds regression coverage for affected model types.
R/​check_collinearity.R Updates term matching and rank-deficiency logic.
NEWS.md Documents user-visible fixes.
man/​check_collinearity.Rd Updates function documentation.
DESCRIPTION Increments the development version.

Review effort: Lite

@jmgirard

Copy link
Copy Markdown
Contributor Author

Copilot summary points addressed in bf3b7cd: the mblogit case of the backticked-names test is now its own test_that() block, so a missing mclogit no longer skips the nnet and brglm2 cases. NEWS now lists mclogit, because mblogit() models with backticked names also returned NULL on main.

@strengejacke
strengejacke merged commit 2315bfb into easystats:main Oct 19, 2026
9 of 20 checks passed
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.

3 participants