Allow simultaneous sampling and boundaries for nested random walk sampler #49
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 61 61
Lines 4050 4142 +92
=========================================
+ Hits 4050 4142 +92 ☔ View full report in Codecov by Sentry. |
| ## If e.g. density is provided by a particle filter, would this bit | ||
| ## mean rerunning it? Increases time cost if so, and would result in | ||
| ## new value of density (different to that which was accepted) |
There was a problem hiding this comment.
I think it does really, which leaves us in a bit of a pickle. Options here:
- forbid this when the model is stochastic
- allow a model to advertise itself as additive (though some subtleties with the prior here)
- add a new method that nested models must provide that allows them to replace new groups (in effect moving this part into the model)
I think the latter is the best option, and something we can discuss when we're both back?
There was a problem hiding this comment.
I've put this down as mrc-5699
| }) | ||
|
|
||
|
|
||
| test_that("can run nested random walk sampler with rejecting boundaries |
There was a problem hiding this comment.
Probably we need tests of some of the lower level bits in the sampler too here, particularly around the details of rejecting just some samples?
| i_group <- model$parameter_groups %in% which(!i) | ||
| pars_next[i_group] <- state$pars[i_group] | ||
| } | ||
| density_next <- model$density(pars_next, by_group = TRUE) |
There was a problem hiding this comment.
You can now use index_group to run a subset here
There was a problem hiding this comment.
actually this needs support from dust too (mrc-5701, mrc-5700)
No description provided.