search

flatironinstitute / bayes-kit

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

More efficient HMCDiag.leapfrog #30

Closed wrongu closed 1 year ago

wrongu commented 1 year ago

Inside HMCDiag.leapfrog(), this moves the half momentum steps outside the loop.

In the spirit of TDD, added a test that asserts that the number of calls to model.log_density_gradient is equal to hmc._steps + 1, where before it was 2 * hmc._steps.

codecov-commenter commented 1 year ago

Codecov Report

Merging #30 (ce57477) into main (f845628) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   95.76%   95.78%   +0.01%     
==========================================
  Files           9        9              
  Lines         260      261       +1     
==========================================
+ Hits          249      250       +1     
  Misses         11       11
Impacted Files Coverage Δ
bayes_kit/hmc.py 94.87% <100.00%> (+0.13%) :arrow_up:

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

WardBrian commented 1 year ago

Hi @wrongu - this is great! Do you mind adding a test for the behavior when num_steps=0? Obviously this is an edge case, but checking that we "do nothing" correctly (e.g. not throw an error, not actually perform any steps, etc) would be nice to go along with this

wrongu commented 1 year ago

@WardBrian nice idea. I added a quick "at least it runs" test with steps=0 by parameterizing the test. It doesn't yet assert things like no change in sample value, but like you said this is an edge case.