Repository navigation
Allow different numbers of draws in loo_model_weights() - #409
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #409 +/- ##
=======================================
Coverage 92.48% 92.49%
=======================================
Files 31 31
Lines 3139 3143 +4
=======================================
+ Hits 2903 2907 +4
Misses 236 236 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if f6df6b2 is merged into master:
|
There was a problem hiding this comment.
This looks good. Claude found an older problem nearby (not caused by this PR, not blocking) that maybe we should consider fixing with this PR. Or should we do it separately? Here it is, what do you think?
- With subsampled LOO objects (from loo_subsample()), loo_model_weights() can give wrong weights without any warning. These objects keep pointwise values only for the subsampled observations, and both the old and new checks compare only how many there are.
- When I ran two such objects with 50 observations each, only 7 of the observation indices matched, but I still got weights of 0.645 and 0.355. That is stacking across different observations. Master behaves the same way.
- The PR widens this slightly, because subsampled objects with different numbers of draws now pass too. A follow-up could reject subsampled objects in validate_psis_loo_list(), since loo_compare() already handles them separately.
|
Thanks, good catch. I included the fix in this PR.
Not in this PR: Subsampled objects that use the same observations ( |
Fixes #389.
This PR was assisted by Claude
Summary
loo_model_weights()stopped with "Each object in the list must have the same dimensions." when two models had the same observations but different numbers of posterior draws. A list of log-likelihood matrices hit the same check. The weights depend only on the pointwiseelpd_loovalues, one N-vector per model, so the draws do not have to match.loo_compare()already compares only the number of observations.Change
validate_psis_loo_list()comparesnrow(x$pointwise)instead ofdim(x).validate_log_lik_list()compares only the number of columns. The matrix path runsloo()on each matrix separately, and ther_eff_listcheck uses only N.xsays that S can differ between models.Tests
I added
loo_model_weights accepts models with different numbers of draws. One of three models has 250 draws instead of 500. For stacking and pseudo-BMA, the matrix list and thepsis_loolist give the same weights, and the weights are non-negative and sum to 1.I removed the test that expected an error for a matrix with one draw less. The
psis_loomismatch test changed only thedimsattribute; it now passes an object with 49 observations instead of 50, so it exercises the check that remains. The other mismatch tests match the new message.With the master
R/loo_model_weights.R, the new test and the changed error test fail. With this PR,devtools::test()withNOT_CRAN=trueruns 1270 tests with no failures and no skips (R 4.6.1, testthat 3.3.2, Linux x86_64). The example in #389 returns weights for both methods.