From bc09605c7259f44926c8ba11baefa6eb07a90119 Mon Sep 17 00:00:00 2001 From: Ricardo Date: Mon, 21 Dec 2020 19:03:05 +0100 Subject: [PATCH 1/3] - Add bound to HyperGeometric logp - Pass unit tests when scipy logpmf returns nan --- pymc3/distributions/discrete.py | 4 +++- pymc3/tests/test_distributions.py | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pymc3/distributions/discrete.py b/pymc3/distributions/discrete.py index c835e2dc443..ab872de3e36 100644 --- a/pymc3/distributions/discrete.py +++ b/pymc3/distributions/discrete.py @@ -930,7 +930,9 @@ def logp(self, value): - betaln(n - value + 1, bad - n + value + 1) - betaln(tot + 1, 1) ) - return result + lower = tt.max([0, n - N + k]) + upper = tt.min([k, n]) + return bound(result, lower <= value, value <= upper) class DiscreteUniform(Discrete): diff --git a/pymc3/tests/test_distributions.py b/pymc3/tests/test_distributions.py index daffdbe8e16..16ed2734883 100644 --- a/pymc3/tests/test_distributions.py +++ b/pymc3/tests/test_distributions.py @@ -805,11 +805,15 @@ def test_geometric(self): ) def test_hypergeometric(self): + def modified_scipy_hypergeom_logpmf(value, N, k, n): + original_res = sp.hypergeom.logpmf(value, N, k, n) + return original_res if not np.isnan(original_res) else -np.inf + self.pymc3_matches_scipy( HyperGeometric, Nat, {"N": NatSmall, "k": NatSmall, "n": NatSmall}, - lambda value, N, k, n: sp.hypergeom.logpmf(value, N, k, n), + lambda value, N, k, n: modified_scipy_hypergeom_logpmf(value, N, k, n), ) def test_negative_binomial(self): From 64009ecb8449eb30db5869edbc7c9929bb4a9a61 Mon Sep 17 00:00:00 2001 From: Ricardo Date: Mon, 21 Dec 2020 19:22:06 +0100 Subject: [PATCH 2/3] - Add release-note --- RELEASE-NOTES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 47c04ee323c..4cbb2a6c613 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -25,6 +25,7 @@ It also brings some dreadfully awaited fixes, so be sure to go through the chang - The notebook gallery has been moved to https://github.com/pymc-devs/pymc-examples (see [#4348](https://github.com/pymc-devs/pymc3/pull/4348)). - `math.logsumexp` now matches `scipy.special.logsumexp` when arrays contain infinite values (see [#4360](https://github.com/pymc-devs/pymc3/pull/4360)). - Fixed mathematical formulation in `MvStudentT` random method. (see [#4359](https://github.com/pymc-devs/pymc3/pull/4359)) +- Fix issue in `logp` method of `HyperGeometric`. It now returns `-inf` for invalid parameters (see [4367](https://github.com/pymc-devs/pymc3/pull/4367)) ## PyMC3 3.10.0 (7 December 2020) From 9f9df0e909f6bbb27859b15fede71dc0a968fc54 Mon Sep 17 00:00:00 2001 From: Ricardo Date: Mon, 21 Dec 2020 22:02:22 +0100 Subject: [PATCH 3/3] - Replace tt.max and tt.min with tt.switch --- pymc3/distributions/discrete.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pymc3/distributions/discrete.py b/pymc3/distributions/discrete.py index ab872de3e36..e639ba66844 100644 --- a/pymc3/distributions/discrete.py +++ b/pymc3/distributions/discrete.py @@ -930,8 +930,9 @@ def logp(self, value): - betaln(n - value + 1, bad - n + value + 1) - betaln(tot + 1, 1) ) - lower = tt.max([0, n - N + k]) - upper = tt.min([k, n]) + # value in [max(0, n - N + k), min(k, n)] + lower = tt.switch(tt.gt(n - N + k, 0), n - N + k, 0) + upper = tt.switch(tt.lt(k, n), k, n) return bound(result, lower <= value, value <= upper)