fix(summary): handle single fixed-effect row in print.bmmfit summary#369
Open
GidonFrischkorn wants to merge 1 commit into
Open
fix(summary): handle single fixed-effect row in print.bmmfit summary#369GidonFrischkorn wants to merge 1 commit into
GidonFrischkorn wants to merge 1 commit into
Conversation
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
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.
Summary
summary()errors when a fitted model has only one population-level regression coefficient. The "Regression Coefficients" block ofprint.bmmsummaryselects rows withsapply()followed byapply(include, 1, any). With a single fixed-effect row,sapply()simplifies to a vector, soapply()receives a dimensionless object and fails:This replaces that selection with a shape-stable
Reduce("|", lapply(...))(plusdrop = FALSE), moved into a small unit-tested helper. Output is identical for models with several coefficient rows.Cause
select_pars()returns the union of the model parameters and the predicted parameters, e.g.c("dprime", "sdratio"). The printer then marks which rows ofx$fixedmatch any of them:When
x$fixedhas one row, eachgrepl()returns length 1, sosapply()returns a length-n_parsvector instead of ann_rows x n_parsmatrix, andapply(..., 1, any)fails. A single printed parameter collapses the same way.Fix
lapply()always returns per-pattern logical vectors of lengthnrow(fixed);Reduce("|", ...)ORs them into one vector for any row or parameter count;drop = FALSEkeeps a single matched row a data frame.Tests
Two regression tests in
tests/testthat/test-summary.Rthat need no fitted-model fixture (the existing summary test is gated on a real fit, which is why this slipped through): one reproduces the single-row failure, one confirms multi-parameter selection is unchanged.Relation to the SDT PRs
This is a general bug in core
summary.R, so it is a standalone PR offdeveloprather than part of a model PR. The SDT model stack (sdt_binary⊂sdt_mafc⊂sdt_ranking⊂sdt_rating) is what exposed it. These are the first bmm models that produce a summary with a single population-level coefficient, because they fix nuisance parameters and estimate onlydprime. For example,sdt_ranking(dist = "gumbel_min")andsdt_mafcfit asdprime ~ 1 (+ (1 | id)), leaving one estimated coefficient whileselect_pars()still listsdprimeplus the fixedsdratio/mu. Earlier models always had at least two fixed rows, so the flaw never triggered.The fix is in core code, so every model benefits (any intercept-only single-parameter fit), and the SDT branches will pick it up when rebased onto
developor at merge. They do not need it to build or pass their own tests; they need it only forsummary()to print on the gumbel-min ranking and m-AFC examples.