fix: natural_logpdf clamps a genuine -inf to -1.8e308 - #1534
Merged
Conversation
`AbstractMessage.natural_logpdf` reduced through
xp.nan_to_num(log_base + eta_t - log_partition, nan=-xp.inf)
The `nan=-xp.inf` is deliberate: an out-of-support NaN (log of a negative value
under a transformed message) is zero density. But `posinf`/`neginf` were left at
their DEFAULTS, and `nan_to_num` replaces a genuine `-inf` with negative float
max. So the call did the opposite of its intent for the inputs that already had
the right answer:
value reaching the reduction | intended | actual
NaN | -inf | -inf
-inf | -inf | -1.7976931348623157e+308
Measured on main @ 6e2d8c8, LogGaussianPrior(0.4, 1.3):
message.logpdf(-1.0) = -inf (log(-1) is NaN)
message.logpdf( 0.0) = -1.7976931348623157e+308 (log(0) is -inf)
That asymmetry is the proof of mechanism, and the second value is exactly
-sys.float_info.max.
It matters because -1.8e308 is FINITE, and `isfinite` is what
`optax.apply_if_finite` and `autofit.non_linear.clipper` branch on to detect a
lane leaving the prior support -- clipper.py's entire premise is that leaving
the box makes the objective non-finite. Two such terms summed overflow to -inf
while one does not, so the behaviour depended on how many parameters were out
of support.
Fix: pass `neginf`/`posinf` through, so only NaN is replaced.
Tests: new test_autofit/messages/test_natural_logpdf_neginf.py (13), covering
both halves of the reduction, in-support values unchanged, and the general
property that no prior family reports a finite density off its support.
Verified by inversion: reverting the fix fails 2 of the 13.
Full suite: 2203 passed, 36 skipped (baseline on 6e2d8c8: 2190 / 36, measured).
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AbstractMessage.natural_logpdfreduced throughxp.nan_to_num(..., nan=-xp.inf)withposinf/neginfleft at their defaults.nan_to_numreplaces a genuine-infwith negative float max, so the call did the opposite of its intent for the inputs that already had the right answer:NaN-inf-inf✅-inf✅-inf-inf-1.7976931348623157e+308❌-inf✅Measured on
main@6e2d8c8,LogGaussianPrior(0.4, 1.3):The asymmetry is the proof of mechanism, and the clamped value is exactly
-sys.float_info.max.Closes #1533.
Why it matters
-1.8e308is finite, andisfiniteis whatoptax.apply_if_finiteandautofit/non_linear/clipper.pybranch on to detect a lane leaving the prior support — that module's entire premise is that leaving the box makes the objective non-finite. Two such terms summed overflow to-infwhile one does not, so the behaviour depended on how many parameters were out of support.API Changes
No API changes. One behaviour change, and it is the fix:
message.logpdfnow returns-infrather than-1.8e308at points whose density is genuinely zero and whose expression reaches-infrather thanNaN. In practice that isLogGaussianPriorat exactly0.0;UniformandLogUniformalready returned-inf.log_prior_from_value— the objective-facing path — is untouched and was already correct throughout.Test Plan
NUMBA_CACHE_DIR=… MPLCONFIGDIR=… python3 -m pytest -x -q test_autofit/— 2203 passed, 36 skipped. Baseline measured on this branch point (6e2d8c8, changes stashed): 2190 / 36. +13, no regressions.test_autofit/messages/test_natural_logpdf_neginf.py— 13 tests: the-infhalf, the NaN half (pinned so fixing one cannot break the other), in-support values still finite across 8 points, and the general property overUniform/LogUniform/LogGaussianthat no family reports a finite density off its support.pytest -x, GREEN on a clean tree.Deliberately out of scope
TruncatedGaussianPrior's message returns finitelogpdfwell outside its limits (-8.20at-1.0for a(0, 3)support). Separate looseness inTruncatedNormalMessage, not this clamp — the prior-levellog_prior_from_valueis correct there, which is why the P6 property tests pass. The new general-property test excludes it explicitly, with a comment saying why, rather than quietly asserting something weaker across all families.Generated by Claude Code