flatironinstitute / bayes-kit

Bayesian inference and posterior analysis for Python
MIT License
43 stars 3 forks source link

Test HMC and MALA against each other #18

Closed WardBrian closed 1 year ago

WardBrian commented 1 year ago

This adds a test which uses the statement "MALA is equivalent to HMC with 1 step" to test HMC and MALA against one another.

It's a neat test, but I also think this is cool for serving the pedagogical goals of the package as well. It's cool to see that they are literally equivalent up to some scaling on the step size.

This also fixes an issue @jsoules noticed with the BetaBinom test model returning a gradient which was off by a constant multiple from the real gradient.

codecov-commenter commented 1 year ago

Codecov Report

Merging #18 (08c6545) into main (22a3e9f) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #18      +/-   ##
==========================================
+ Coverage   95.68%   95.71%   +0.03%     
==========================================
  Files           9        9              
  Lines         255      257       +2     
==========================================
+ Hits          244      246       +2     
  Misses         11       11              
Impacted Files Coverage Δ
bayes_kit/__init__.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

jsoules commented 1 year ago

Looks good to me if we can fix whatever's going on with the HMC beta binomial test.

WardBrian commented 1 year ago

Fixing the gradient calculation seems to have made the mala test less stable as well. I’m sure we can find another sort of island of stability by tuning step sizes, I’ll try some more tomorrow (annoyingly I seem to have quite good luck on my local machine, so even the current step size refuses to fail!)

WardBrian commented 1 year ago

I seem to have found some slightly better settings without it taking much longer than it did before

WardBrian commented 1 year ago

Merging now, will follow on with a PR to unconstrain the binomial model