aesara-devs / aehmc

An HMC/NUTS implementation in Aesara
MIT License
33 stars 6 forks source link

Small fixes #14

Closed rlouf closed 3 years ago

rlouf commented 3 years ago

These changes close #10 and #12.

zoj613 commented 3 years ago

Are the failures a result of the changes? Looks like it:

>       assert np.mean(positions[9000:], axis=0) == pytest.approx(3, 1e-1)
E       assert array([3.89144683]) == 3 ± 3.0e-01
E         +array([3.89144683])
E         -3 ± 3.0e-01

tests/test_hmc.py:68: AssertionError
rlouf commented 3 years ago

Tests pass locally, must have something to do with the random seed used? Anyway, this should not happen so I'll investigate.

Edit: this was a version issue.

codecov[bot] commented 3 years ago

Codecov Report

Merging #14 (8ca1543) into main (87b0c4d) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main       #14   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines           97        97           
  Branches         3         3           
=========================================
  Hits            97        97           
Impacted Files Coverage Δ
aehmc/hmc.py 100.00% <100.00%> (ø)
aehmc/metrics.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 87b0c4d...8ca1543. Read the comment docs.

rlouf commented 3 years ago

@zoj613 ready to review, something must have changed in aesara's random number generator since the version on my machine.

rlouf commented 3 years ago

@brandonwillard this needs a review from someone with write access.