Skip to content

Fix InverseGamma.cumulativeProbability/inverseCumulativeProbability returning Gamma quantiles instead of InverseGamma quantiles - #150

Merged
jordandouglas merged 1 commit into
masterfrom
Inversegamma
Aug 17, 2026
Merged

Fix InverseGamma.cumulativeProbability/inverseCumulativeProbability returning Gamma quantiles instead of InverseGamma quantiles #150
jordandouglas merged 1 commit into
masterfrom
Inversegamma

Conversation

@walterxie

Copy link
Copy Markdown
Member

Fix #149

  • InverseGamma (spec) inherited cumulativeProbability/inverseCumulativeProbability from ScalarDistribution, which delegates to getApacheDistribution() — the internal Gamma(alpha, rate=beta) object used for sampling via x = 1/y. That delegation ignored the 1/y transform, so both methods returned the Gamma distribution's CDF/ICDF, not the InverseGamma's. density()/logDensity() were unaffected (they use a hand-written closed-form formula and never call getApacheDistribution()).
  • Fixed by overriding both methods in InverseGamma with the correct change of variables: for Y = 1/X ~ Gamma(alpha, rate=beta), P(X ≤ x) = 1 - GammaCDF(1/x) = GammaSurvival(1/x), and inverting gives invGammaICDF(p) = 1 / GammaICDF(1-p) = 1 / GammaInverseSurvival(p). p ≤ 0 / p ≥ 1 boundaries return 0.0 / +Infinity explicitly.

Test:

  • InverseGammaTest: re-enabled the previously-disabled CDF assertion in testInvGamma() against reference scipy.invgamma values (was commented out with "not implemented yet", stale even after the CDF path started silently working-but-wrong).
  • Added testCumulativeProbabilityMatchesInverseGammaNotGamma: checks cumulativeProbability against reference values and the 1 - GammaCDF(1/x) formula, and asserts it no longer equals the raw Gamma CDF.
  • Added testInverseCumulativeProbabilityMatchesInverseGammaNotGamma: round-trips inverseCumulativeProbability through the reference CDF values and through cumulativeProbability itself, checks the 1/GammaICDF(1-p) formula, asserts it's no longer the raw Gamma ICDF, and checks p=0/p=1 boundary behavior.
  • Removed a dead commented-out Monte Carlo check in testInvGamma() that referenced a nonexistent nextInverseGamma() method (never compiled, superseded by the new deterministic tests).
  • mvn test -pl beast-base -Dtest=InverseGammaTest → 3/3 pass. Full beast-base suite → 479/479 pass, 0 regressions.

@jordandouglas
jordandouglas merged commit 5d30add into master Aug 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InverseGamma (spec) samples/quantiles from the wrong distribution

2 participants