Fix stanr/stanli backend dropping group-level parameter levels - #2
Open
gravesti wants to merge 1 commit into
Open
Fix stanr/stanli backend dropping group-level parameter levels#2gravesti wants to merge 1 commit into
gravesti wants to merge 1 commit into
Conversation
read_stanr_fit_as_stanfit()/.stanfit_from_csfit() computed a correctly filtered 'variables'/'model_pars' list (excluding raw non-centered z_* parameters, keeping lprior/lp__), but never applied that filter to the actual 'samples' data frame before setting fnames_oi <- colnames(samples). This left fnames_oi out of sync with dims_oi/pars_oi, which downstream renaming logic assumes are in sync. Once a grouping factor had more than 2 flattened elements, this silently corrupted parameter names: raw z_1 columns were kept, lprior/lp__ were dropped, and one level of the derived r_<group>__<term> parameter went missing -- all without raising an error, since column counts still matched. Fix: filter 'samples' down to columns matching model_pars/special_vars before computing fnames_oi, mirroring how read_csv_as_stanfit() already pre-filters columns via cmdstanr::read_cmdstan_csv(variables = ...). Adds a regression test (tests/testthat/tests.stanr-backend.R) that fails on the prior code and passes after this fix.
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.
Thanks for putting together this patch for brms. I came across this bug where the wrong variables were saved compared to cmdstan. LLM generated patch below.
read_stanr_fit_as_stanfit()/.stanfit_from_csfit()computed a correctly filtered 'variables'/'model_pars' list (excluding raw non-centered z_* parameters, keeping lprior/lp__), but never applied that filter to the actual 'samples' data frame before settingfnames_oi <- colnames(samples). This leftfnames_oiout of sync withdims_oi/pars_oi, which downstream renaming logic assumes are in sync. Once a grouping factor had more than 2 flattened elements, this silently corrupted parameter names: raw z_1 columns were kept, lprior/lp__ were dropped, and one level of the derived r___ parameter went missing.Fix: filter 'samples' down to columns matching model_pars/special_vars before computing
fnames_oi, mirroring howread_csv_as_stanfit()already pre-filters columns via cmdstanr::read_cmdstan_csv(variables = ...).Adds a regression test (tests/testthat/tests.stanr-backend.R) that fails on the prior code and passes after this fix.
Reprex: