Repository navigation
Sync headers from barry: fix support-vs-array index confusion in sampling - #9
Merged
Merged
Conversation
…ling Mirrors USCbiostats/barry#25. `keys2support` maps a hash key to a SUPPORT index, but the reuse branch of Model::sample(const Array_Type &, params) treated locator->second as an array index and mapped it through arrays2support a second time; the first time a mid-simulation conditioning state was revisited, the sampler drew from an unrelated support (e.g. the defm "torus" example collapses at row 13). DEFM::simulate had the mirror-image bug on the first process of each individual. Both are keyed on the support index directly; the Array overload now delegates to update_pset_probs, matching the sample(size_t, params) path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thank you for your contribution @gvegayon 🚀! The pkgdown site preview is ready for review 👉 Download here 👈! |
Syncs the follow-up from USCbiostats/barry#25: add a pset_probs.empty() check to the update guard, matching sample(size_t, params). init() adds arrays without sizing pset_probs and a prior likelihood() call sets first_calc_done/params_last without filling it, so the cache guard alone could index into an empty buffer. Header stays byte-identical to barry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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
Header sync of USCbiostats/barry#25 into
inst/include/barry/.keys2supportmaps a hash key to a support index, but the reuse branch ofModel::sample(const Array_Type &, params)treatedlocator->secondas an array index and mapped it througharrays2supporta second time. The first time a conditioning state discovered mid-simulation is revisited, the sampler draws from an unrelated support. In thedefmREADME "Example 2: A fun model" (a 10-state torus walk) the active state advances correctly through row 12 but collapses at row 13.DEFM::simulatehad the mirror-image bug: the first process of each individual was sampled withsample(arrays2support[model_num], par), passing a support index to an overload that expects an array index — giving wrong first-row draws for the 2nd+ individuals in a multi-individual simulation.Fix
Both are keyed on the support index directly; the
Array_Typeoverload now delegates probability computation toupdate_pset_probs, matching the existingsample(size_t, params)path.Verification
inst/include/barry/model-meat.hppandinst/include/barry/models/defm/defm-meat.hppare now byte-identical to the fixed headers in barry#25.109 assertions in 21 test casesagainst these exact headers.defmREADME Example 2 compiled against these headers now walks the torus past row 13.The regression test itself lives in the barry repo (
tests/), which is not vendored here.🤖 Generated with Claude Code