Skip to content

FIX: Fix HyperGeometric.logcdf support bounds and safe indexing - #8493

Open
rushirathod22 wants to merge 1 commit into
pymc-devs:mainfrom
rushirathod22:fix-hypergeometric-logcdf-bounds
Open

rushirathod22 wants to merge 1 commit into
pymc-devs:mainfrom
rushirathod22:fix-hypergeometric-logcdf-bounds

Conversation

@rushirathod22

@rushirathod22 rushirathod22 commented Oct 15, 2026 •

Copy link
Copy Markdown

Closes #8492

Description

This PR fixes mathematical boundary handling and numerical instability in HyperGeometric.logcdf.

Context & Problem

Previously, HyperGeometric.logcdf evaluated bounds using fixed heuristics (value < 0 and value < n):

  • When the lower support limit $\text{lower} = \max(0, n - \text{bad}) &gt; 0$, inputs where $0 \le \text{value} &lt; \text{lower}$ fell through to logsumexp, evaluating betaln on non-positive integers rather than immediately returning -inf.
  • When $\text{upper} = \min(\text{good}, n) &lt; n$, inputs where $\text{value} \ge \text{upper}$ were evaluated in logsumexp rather than immediately returning 0.0.
  • The original code contained a documented TODO regarding this: # TODO: Use lower upper in locgdf for smarter logsumexp?.

Changes Made

  1. Accurate Support Calculation: Dynamically calculates lower = pt.switch(pt.gt(n - bad, 0), n - bad, 0) and upper = pt.switch(pt.lt(good, n), good, n), matching HyperGeometric.logp.
  2. Safe Clamped Indexing: Clamps the summation indexing via safe_value to ensure pt.arange never executes over out-of-support points.
  3. Boundary Return Values: Returns -inf when value < lower and 0.0 when value >= upper.
  4. Testing: Adds dedicated parametrized regression test test_hypergeometric_logcdf_bounds in tests/distributions/test_discrete.py checking lower-tail, upper-tail, and intermediate points against scipy.stats.hypergeom.
  5. Technical Debt: Removes the obsolete # TODO comment.

Checklist

  • Code follows the style guide and conventions of this project
  • Tests added to verify the fix
  • All tests pass locally

- Align lower bound to max(0, n - bad) and upper bound to min(good, n)

- Return -inf when value < lower and 0.0 when value >= upper

- Clamp summation indexing with safe_value to prevent out-of-support evaluations

- Add test_hypergeometric_logcdf_bounds covering boundary edge cases
@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 pymc | 🛠️ Build #35168922 | 📁 Comparing 036eebf against latest (91e6da3)

  🔍 Preview build  

2 files changed
± glossary.html
± _modules/pymc/distributions/discrete.html

@rushirathod22

Copy link
Copy Markdown
Author

Hi @pymc-devs! 👋

This PR is ready for review. It addresses the support bound discrepancies in HyperGeometric.logcdf when $\text{lower} &gt; 0$ or $\text{upper} &lt; n$, aligns the bounds with logp, and adds regression tests against SciPy.

Please let me know if any adjustments, additional tests, or documentation changes are needed. I'm very happy to address any feedback! Thank you for your time and guidance.

This branch has not been deployed

No deployments
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.

BUG: HyperGeometric.logcdf evaluates out-of-support points and fails when lower > 0 or upper < n

1 participant