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

Overhaul state/particle/group subset from history #103

Merged
merged 12 commits into from
Oct 21, 2024
Merged

Overhaul state/particle/group subset from history #103

merged 12 commits into from
Oct 21, 2024

Conversation

richfitz
Copy link
Member

@richfitz richfitz commented Oct 16, 2024

How this is 700 lines I don't know. It was fiddly though.

This PR looks at extracting state from the filters with dust_likelihood_last_history and dust_likelihood_last_state. There are two related problems.

  • Previously these accepted index_group to subset by group. But this was problematic because it depended on the index_group passed into dust_likelihood_run()! I have removed the index_group argument from these and they return the groups that we ran with originally which is pretty much always what is wanted, and more advanced use will be rare enough that we can either implement it when asked or people can subset manually.
  • I've moved where index_state goes from filter creation to filter running. This requires a small dance so that we can ensure that the filter is created before we validate the index but that in turn tidies up my least favourite part of the monty initialisation. We might export the little "ensure initialised" function I have written there later. This keeps the two index arguments together which is nice.

The PR is large because:

  • this affects a bunch of generated code
  • we use the history saving mechanism for simulate
  • there's some similar-but-different code for the filter/unfilter and the discrete/continuous splits

@richfitz richfitz marked this pull request as ready for review October 17, 2024 09:26
@richfitz richfitz requested a review from weshinsley October 17, 2024 09:32
Copy link
Contributor

@weshinsley weshinsley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - I think I sort of understand this one, but would probably be good to talk through. The monty bits will probably need tweaking after that other PR - not sure what you'd like to do first so I won't merge

@richfitz richfitz merged commit afba104 into main Oct 21, 2024
7 checks passed
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.

2 participants