fix: correct entropy formulas for Pareto, Geometric, StudentsT, and Dirichlet#383
Open
gaoflow wants to merge 1 commit into
Open
fix: correct entropy formulas for Pareto, Geometric, StudentsT, and Dirichlet#383gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
…irichlet - Pareto: formula was the exact negation of ln(x_m/a) + 1/a + 1 - Geometric: used log base 2, returning bits instead of nats - StudentsT: scale shifted entropy by -ln(scale) instead of +ln(scale) - Dirichlet: sum of ln_gamma(alpha_i) entered with the wrong sign Existing test assertions for Pareto, Geometric, and Dirichlet encoded the buggy outputs; corrected to closed-form values cross-checked against scipy and extended with more parameter points. Added an entropy test for StudentsT, which had none.
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.
Four
entropy()implementations return wrong values. Against the closed forms:Pareto(x_m, α) — differential entropy is
ln(x_m/α) + 1/α + 1. The implementation (and its doccomment) compute
ln(α/x_m) - 1/α - 1, the exact negation of every term.Geometric(p) — the formula shape is right but uses
.log(2.0), so the result is in bits. Everyother entropy in the crate is in nats.
StudentsT(μ, σ, ν) — for
Y = μ + σX,H(Y) = H(X) + ln σ. The code appliesshift = -scale.ln(); the derivation comment above it states the same relation with the signflipped. Values are correct at σ = 1 and wrong for every other scale.
Dirichlet(α) — the method's doc comment has the correct formula
(
ln B(α) + (α₀ - K)ψ(α₀) - Σ(αᵢ-1)ψ(αᵢ)), but the implementation foldsln Γ(αᵢ)into theaccumulator that gets subtracted, so
Σ ln Γ(αᵢ)enters with the wrong sign.To find the extent I ran all 27
entropy()implementations in the crate against scipy 1.17.1(63 parameter sets). These four are the only formula errors:
Pareto::new(1.0, 3.0)Geometric::new(0.5)StudentsT::new(0.0, 3.0, 5.0)Dirichlet::new(vec![0.1, 0.3, 0.5, 0.8])Everything else agrees to 1e-9 relative or better. (The one remaining deviation is Poisson, whose
entropy is a large-λ asymptotic series that drifts for small λ — an approximation-accuracy question
left out of this PR.)
The existing
test_entropycases for Pareto, Geometric, and Dirichlet asserted the buggy outputs —Pareto's five expected values are each the negation of the correct one, and Dirichlet's expected
values match the sign-flipped formula to within 1e-30 — i.e. they were generated from the
implementation rather than derived independently. This PR corrects those assertions to
closed-form values cross-checked against scipy, adds more parameter points, and adds an entropy
test for StudentsT, which had none (including location-invariance and a scale < 1 point).