Return 0 from Gamma CDF for non-positive inputs - #33
Open
arpitjain099 wants to merge 1 commit into
Open
Conversation
The Gamma distribution is supported on [0, infinity), so gammainc(k, x) is 0 for x <= 0. The substitution t = x*u used by GammaMarginal._gammainc only holds for x > 0, so a non-positive sample produced a spurious value: around 0.50 at x=-1 and 134 at x=-5 for shape=2, a negative value for shape=1, and NaN for shape<1 (where the CDF was also NaN at x=0). That breaks monotonicity and puts the CDF outside [0, 1]. Evaluate the quadrature on the positive part only and mask the rest to 0. Masked entries are substituted with ones before the quadrature so they stay finite and cannot poison the torch autograd graph. Positive-x results are unchanged. Signed-off-by: Arpit Jain <arpitjain099@gmail.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
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.
The Gamma distribution is supported on [0, inf), so its CDF should be 0 for every x <= 0. Right now
GammaMarginal.cdfreturns nonsense there, because_gammaincuses the substitution t = x*u, which only holds for x > 0.With scale=1.5, on current
dev/main:cdf(-1.0)is about 0.50 andcdf(-5.0)is about 134, so the CDF is not monotonic and leaves [0, 1] entirely.cdf(-1.0)is negative.cdf(x)is NaN for x <= 0, including at x=0, since0 ** (shape - 1)blows up.scipy.stats.gamma(...).cdfreturns 0.0 for all of those.The fix evaluates the quadrature on the positive part only and masks the rest back to 0. Masked entries get ones substituted before the quadrature so nothing goes non-finite, which keeps the torch autograd graph clean (I checked that d/dx cdf at a positive point still equals the pdf). Positive-x results are untouched.
Tests added for x <= 0 at shape 2.0, for the shape < 1 case where the integrand is singular at the origin, and for monotonicity across a grid spanning zero. All three fail before the change on both backends. After it,
packages/pyapprox/tests/probability/is 1610 passed, 28 skipped, andruff check packages/pyapprox/src/pyapprox/is clean.I based this on
dev/mainsince that is where development seems to happen. Happy to retarget, or to move the guard up intocdf()if you would rather keep_gammainca raw kernel.I work on open-source supply-chain security and spend a fair amount of time reading numerical code in national-lab projects, which is how I ran into this one.