tweag / monad-bayes

A library for probabilistic programming in Haskell.
MIT License
404 stars 62 forks source link

Test "Integrator Variance/variance numerically" is flaky #275

Closed turion closed 1 year ago

turion commented 1 year ago

See https://github.com/tweag/monad-bayes/actions/runs/5001514340/jobs/8960326116#step:6:34973

monad-bayes> Failures:
monad-bayes>   test/Spec.hs:53:5:
monad-bayes>   1) Integrator Variance variance numerically
monad-bayes>        Falsified (after 79 tests and 12 shrinks):
monad-bayes>          -2.0
monad-bayes>          1.0e-5
monad-bayes>   To rerun use: --match "/Integrator Variance/variance numerically/"

This test usually passes, but it failed here without any code changes.

turion commented 1 year ago

The test code in question is:

  describe "Integrator Variance" do
    prop "variance numerically" $
      \mean var ->
        var > 0 ==> property $ TestIntegrator.normalVariance mean (sqrt var) ~== var

I'd suspect floating point rounding issues.

turion commented 1 year ago

@reubenharry any ideas?

reubenharry commented 1 year ago

If there were no changes, I'm slightly confused, because I believe the integrator is deterministic. (Oh but I guess quickcheck isn't). Does the case it identified consistently fail? The negative variance is...odd.

It's not a huge worry, because the integrator is mainly just a toy implementation, and isn't used in any real inference algorithm, but maybe worth double checking the failure case.

turion commented 1 year ago

This might even lead to monad-bayes getting marked broken in nixpkgs, see https://github.com/NixOS/nixpkgs/pull/236431

turion commented 1 year ago

Ah, this might be due to a better QuickCheck generator that revealed test failures that were false negatives:

https://github.com/nick8325/quickcheck/issues/359

turion commented 1 year ago

Or similar to https://github.com/haskell/vector/issues/460 it might be that the new generator produces more values for which we experience overflows.

turion commented 1 year ago

I can't reproduce it with QuickCheck 2.14.3 yet.

reubenharry commented 1 year ago

Worth noting that the Integrator is really just a toy implementation, which is never used in any real inference algorithm in Monad-Bayes. Quickest thing would be just to remove these tests. Or even just remove the Integrator altogether (although it's quite nice for graphing distributions).

turion commented 1 year ago

Still it would be great to understand what's going wrong. If we can't solve a test problem with a toy implementation, the real inference algorithms will be even harder to debug.

turion commented 1 year ago

Maybe var > 0 is the issue, and we should have something like "var is significantly above 0". For example, var > 0 && not (var ~== 0).