-
-
Notifications
You must be signed in to change notification settings - Fork 983
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
Improve sequential_gaussian_filter_sample() #3146
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes make sense to me. Looks great overall! Thanks, Fritz.
if noise is None: | ||
noise = torch.randn(shape, dtype=loc.dtype, device=loc.device) | ||
else: | ||
noise = noise.reshape(shape) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to broadcast noise
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, it is better to avoid broadcasting the noise.
Thanks for reviewing @fehiepsi! |
This makes a number of improvements to
sequential_gaussian_filter_sample()
:cat([noise, -noise])
.sequential_gaussian_filter_sample()
more useful outside of the context ofGaussianHMM
, e.g. it will be easier to use in a fully-observed Markov model. This attempts to make up for my admittedly bad design choice of making the hidden Z series one step longer than the observed X series inGaussianHMM
😬torch.empty()
, rather thantorch.nn.functional.pad
andtorch.stack
. This is admittedly less functional, but does reduce memory usage and IMHO reads cleaner.This also exposes the helper
matrix_and_gaussian_to_gaussian()
which I'm finding useful.Tested