Fix entropy_AIC_weights overflow when a model fits far better than baseline - #10
Merged
Merged
Conversation
…seline Discovered while running compute_fits.py on real MIBI-TOF data (large N, clean signal), a case entirely plausible in normal use. relative_likelihood_over_baseline is referenced against baseline_fit.AIC, so it overflows to +inf when a model's AIC is far below baseline's (a real, meaningful result on its own, not a bug). But _set_entropy_weights fed these same values into calculate_AIC_weight_entropy, so an inf relative likelihood corrupted the weight normalization (inf/inf or x/inf), producing NaN entropy. Entropy is shift-invariant, so _set_entropy_weights now references against the minimum AIC among the compared fits instead of baseline_fit's -- mathematically identical result when no overflow occurs, but every exponent stays <= 0 so it never overflows. relative_likelihood_over_baseline itself is unchanged (still legitimately inf in this case); its overflow warning is now silenced at the source since it's an expected outcome, not an error. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merged
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
1 participant
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
Found while doing a smoke test for the figure3 SLURM recompute on real MIBI-TOF data:
entropy_AIC_weightscan come backNaNwhen a model fits far better than theConstantFitbaseline (large N + clean signal -- an entirely normal case, not a contrived edge case).Root cause:
relative_likelihood_over_baselineis referenced againstbaseline_fit.AICspecifically, so it correctly overflows to+infwhen a model's AIC is far below baseline's (a legitimate result -- "infinitely more likely than the null model"). But_set_entropy_weightsfed these same (possiblyinf) values straight intocalculate_AIC_weight_entropy's normalization, whereinf/inforx/infcorrupts the result intoNaN.Fix: since entropy is shift-invariant,
_set_entropy_weightsnow references relative likelihood against the minimum AIC among the compared fits rather thanbaseline_fit.AIC-- mathematically identical result whenever no overflow occurs, but every exponent stays<= 0, so it can never overflow.relative_likelihood_over_baselineitself (the reported per-fit value) is unchanged -- still legitimatelyinfin this case -- but its overflow warning is now silenced at the source (np.errstate) since it's an expected, not erroneous, outcome.Test plan
entropy_AIC_weightswasNaNbefore,~0(correctly confident) afterpytest tests/-- 118 passed, 1 skipped (1 new regression test:test_entropy_finite_when_far_better_than_baseline)🤖 Generated with Claude Code