Skip to content

Factor out intermediate storage #72

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

Merged
merged 1 commit into from
Mar 13, 2025
Merged

Conversation

THargreaves
Copy link
Collaborator

This PR completely removes intermediate storage (solving #37) and makes changes to the callback system to be compatible with this change. The main points are:

  • The ancestors are now stored in the particle distribution. This doesn't change functionality at all as far as I can tell.
  • The callbacks are now parameterised by a subtype of CallbackTrigger e.g. PostPredictCallback. These can be added by the user if they have a special time they want to callback. It's a bit of a pain that these need default implementations that do nothing—this is also quite annoying in that it fails silently if you get the type signature wrong in your callback definition as it just runs the default that does nothing. There's probably a cleaner way to do this but I think this is fine for this PR
  • This change means we can get rid of all deep copies in the filtering code (🎉🥳) and let's us do some nice in-place operations making the predict/update cleaner. This does mean that all callbacks need to deepcopy now but they should have been doing that already. Solves Remove unnecessary deep copies #27.

The only issue this introduces is that it makes the guided filter a bit harder to implement since the old log weights are "forgotten" by the time we get to the update step. There are definitely a few options of how to handle this so not too worried for now—just a case of finding the most elegant.

Let me know if @charlesknipp @FredericWantiez have any thoughts otherwise I will merge soonish so you guys can build on this modified interface.

@THargreaves THargreaves merged commit ba214d6 into main Mar 13, 2025
3 of 4 checks passed
@THargreaves THargreaves deleted the th/intermediate-removal branch March 13, 2025 09:50
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.

1 participant