facebookresearch / beanmachine

A library that allows for inference on probabilistic models
https://beanmachine.org/
MIT License
264 stars 49 forks source link

Fix assertion in BMG's log1mexp #1790

Closed ericlippert closed 1 year ago

ericlippert commented 1 year ago

Summary: In any code path where we end up with a log1mexp of NaN, we were asserting. Remember, NaN <= anything is always false, including NaN <= NaN. All comparisons on NaN are false.

In the particular tests which are re-enabled, the NaN arises from an unfortunately multiplication of 0 by -infinity which is NaN in the IEEE double system, but this problem is more general than just the model under test; lots of things can result in NaN.

We now explicitly check for NaN and return NaN before doing the assertion.

There still might be a code path on which a truly negative number can be passed to this function, but I will attempt to produce that scenario and address it in a later diff.

Differential Revision: D40808256

facebook-github-bot commented 1 year ago

This pull request was exported from Phabricator. Differential Revision: D40808256

facebook-github-bot commented 1 year ago

This pull request was exported from Phabricator. Differential Revision: D40808256