-
Notifications
You must be signed in to change notification settings - Fork 22
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
Adds Annealed Importance Sampling #550
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #550 +/- ##
===========================================
- Coverage 99.08% 98.68% -0.41%
===========================================
Files 52 56 +4
Lines 3605 3941 +336
===========================================
+ Hits 3572 3889 +317
- Misses 33 52 +19 ☔ View full report in Codecov by Sentry. |
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 algorithm doesn't seem correct to me, but I could be miss-reading it. In addition, I would recommend writing a test for the algorithm, using a known distribution. I'd use the same unimodel distribution used in the paper, so you can compare with its results.
# Main sampling loop | ||
for j in range(1, self._num_beta): | ||
# Compute jth transition with current sample | ||
log_density_current[j] = self.transition_distribution(current, j) |
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.
you are doing f_j(x_j) / f_{j-1}(x_j)
, should be f_j(x_j) / f_{j+1}(x_j)
from eqn 11
log_density_current = np.zeros(self._num_beta) | ||
log_density_current[0] = current_f | ||
log_density_previous = np.zeros(self._num_beta) | ||
log_density_previous[0] = current_f |
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.
same here, log_density_previous[0] should be f_{1}(x_0)
from eqn 11
proposed_f = self.transition_distribution(proposed, j) | ||
|
||
# Metropolis acceptance | ||
acceptance_log_prob = proposed_f - current_f |
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.
for the gaussian test case they use in the paper, a much more complicated transition T_j
is used, a sequence of 3 metropolis tests repeated 5-10 times. I'm not sure if all that is neccessary however
Description
Adds an annealed importance sampling class for marginal likelihood computation.
Issue reference
Fixes #549
Review
Before you mark your PR as ready for review, please ensure that you've considered the following:
Type of change
Key checklist:
$ pre-commit run
(or$ nox -s pre-commit
) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ nox -s tests
$ nox -s doctest
You can run integration tests, unit tests, and doctests together at once, using
$ nox -s quick
.Further checks:
Thank you for contributing to our project! Your efforts help us to deliver great software.