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

Sample simultaneously #35

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Sample simultaneously #35

wants to merge 16 commits into from

Conversation

richfitz
Copy link
Member

@richfitz richfitz commented Apr 29, 2024

Merge after #34, contains those commits

This PR adds support for simultaneously sampling from multiple chains at once. This is triggered by having pars be a matrix rather than a vector when passed into a sampler/model. Models will need to be able to support this and we don't assume that they do; the property allow_multiple_parameters can be used to signal this.

This functionality has two purposes:

  • to support a generic implementation of parallel tempering, where we will embed a sampler (e.g, random walk, hmc or adaptive walk) within a parallel tempering scheme, requiring us to have each sampler step multiple chains at a single time
  • to allow more efficient sampling from simple models (e.g., those that come from the DSL would be easy to generate to support this).

There are lots of changes here, but I think it does all need to come though at once or it loses context. Let's chat through the ideas this week.

Things to note:

  • I need to tidy up how details is collected at the end of a run; I've not done this even though it's only a few lines
  • I do not like allow_multiple_parameters as a name, but can't think of a better one
  • support for stochastic models is a little hard, and also causes some issues with the wrapper we currently have for dust models; I've punted on that from now and disallow it

Copy link

codecov bot commented Apr 29, 2024

Codecov Report

Attention: Patch coverage is 98.87640% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 99.89%. Comparing base (b588969) to head (77cb30c).
Report is 74 commits behind head on main.

❗ Current head 77cb30c differs from pull request most recent head f83d225. Consider uploading reports for the commit f83d225 to get more accurate results

Files Patch % Lines
R/sampler-hmc.R 96.29% 2 Missing ⚠️
R/sample.R 97.22% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   99.44%   99.89%   +0.44%     
==========================================
  Files          45       53       +8     
  Lines        2163     2859     +696     
==========================================
+ Hits         2151     2856     +705     
+ Misses         12        3       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@richfitz richfitz marked this pull request as ready for review April 30, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant