Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loo moment_match fails for brms fits created with combine_models #1603

Closed
avehtari opened this issue Feb 27, 2024 · 3 comments
Closed

loo moment_match fails for brms fits created with combine_models #1603

avehtari opened this issue Feb 27, 2024 · 3 comments
Labels
Milestone

Comments

@avehtari
Copy link
Contributor

avehtari commented Feb 27, 2024

The problem and reproducible example provided in Stan discourse https://discourse.mc-stan.org/t/loo-moment-match-crashes-on-brms-fits-after-combine-models/34273

I looked at this, and the reason is that .update_pars in loo_moment_match.R tries to access x$fit@sim$fnames_oi_old which is not available in brms object created by combine_models()

The relevant part in loo_moment_match.R
https://github.com/paul-buerkner/brms/blob/b92ed99e627b7ba0894803143aa6552a0d7a1131/R/loo_moment_match.R#L128C3-L128C74

new_draws <- named_list(x$fit@sim$fnames_oi_old, list(numeric(ndraws)))

EDIT: all the missing slots in @sim in brms object created by combine_models()

  .. .. ..$ pars_oi_old  : chr [1:8] "b" "Intercept" "sd_1" "z_1" ...
  .. .. ..$ dims_oi_old  :List of 8
  .. .. ..$ fnames_oi_old: chr [1:127] "b[1]" "b[2]" "b[3]" "b[4]" ...
@paul-buerkner paul-buerkner added this to the 2.21.0 milestone Feb 27, 2024
@paul-buerkner
Copy link
Owner

Thanks! I don't fully understand your edit. Are you saying combine_models() already stores or does not store the relevant info already?

@avehtari
Copy link
Contributor Author

avehtari commented Feb 27, 2024

Those slots are missing in the combined object. See the discourse post for the failing example. moment_match works if you run the following lines to copy the missing info from one one model to the combined model

brm_fit$fit@sim$pars_oi_old <- brm_fit_c1$fit@sim$pars_oi_old
brm_fit$fit@sim$dims_oi_old <- brm_fit_c1$fit@sim$dims_oi_old
brm_fit$fit@sim$fnames_oi_old <- brm_fit_c1$fit@sim$fnames_oi_old

paul-buerkner added a commit that referenced this issue Feb 27, 2024
@paul-buerkner
Copy link
Owner

Thank you! Should be fixed now. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants