Skip to content

Commit 89916ad

Browse files
authored
Increase logcdf coverage for invalid parameter values (#4421)
* Test logcdf methods outside of finite edges * Fix failing logcdf methods * Disable failing tests temporarily * Revert removed xfail * Update docstring * Add more informative comment
1 parent a3c2060 commit 89916ad

File tree

4 files changed

+227
-75
lines changed

4 files changed

+227
-75
lines changed

RELEASE-NOTES.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ It also brings some dreadfully awaited fixes, so be sure to go through the chang
3232
- Fixed mathematical formulation in `MvStudentT` random method. (see [#4359](https://github.com/pymc-devs/pymc3/pull/4359))
3333
- Fix issue in `logp` method of `HyperGeometric`. It now returns `-inf` for invalid parameters (see [4367](https://github.com/pymc-devs/pymc3/pull/4367))
3434
- Fixed `MatrixNormal` random method to work with parameters as random variables. (see [#4368](https://github.com/pymc-devs/pymc3/pull/4368))
35-
- Update the `logcdf` method of several continuous distributions to return -inf for invalid parameters and values, and raise an informative error when multiple values cannot be evaluated in a single call. (see [4393](https://github.com/pymc-devs/pymc3/pull/4393))
35+
- Update the `logcdf` method of several continuous distributions to return -inf for invalid parameters and values, and raise an informative error when multiple values cannot be evaluated in a single call. (see [4393](https://github.com/pymc-devs/pymc3/pull/4393) and [#4421](https://github.com/pymc-devs/pymc3/pull/4421))
3636
- Improve numerical stability in `logp` and `logcdf` methods of `ExGaussian` (see [#4407](https://github.com/pymc-devs/pymc3/pull/4407))
3737
- Issue UserWarning when doing prior or posterior predictive sampling with models containing Potential factors (see [#4419](https://github.com/pymc-devs/pymc3/pull/4419))
3838
- Dirichlet distribution's `random` method is now optimized and gives outputs in correct shape (see [#4416](https://github.com/pymc-devs/pymc3/pull/4407))

pymc3/distributions/continuous.py

Lines changed: 101 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,12 @@ def logcdf(self, value):
553553
-------
554554
TensorVariable
555555
"""
556-
return normal_lcdf(self.mu, self.sigma, value)
556+
mu = self.mu
557+
sigma = self.sigma
558+
return bound(
559+
normal_lcdf(mu, sigma, value),
560+
0 < sigma,
561+
)
557562

558563

559564
class TruncatedNormal(BoundedContinuous):
@@ -1144,10 +1149,16 @@ def logcdf(self, value):
11441149
tt.eq(value, 0) & tt.eq(lam, np.inf)
11451150
)
11461151

1147-
return tt.switch(
1148-
left_limit,
1149-
-np.inf,
1150-
tt.switch((right_limit | degenerate_dist), 0, a + tt.log1p(tt.exp(b - a))),
1152+
return bound(
1153+
tt.switch(
1154+
~(right_limit | degenerate_dist),
1155+
a + tt.log1p(tt.exp(b - a)),
1156+
0,
1157+
),
1158+
~left_limit,
1159+
0 < mu,
1160+
0 < lam,
1161+
0 <= alpha,
11511162
)
11521163

11531164

@@ -1539,10 +1550,10 @@ def logcdf(self, value):
15391550
value = floatX(tt.as_tensor(value))
15401551
lam = self.lam
15411552
a = lam * value
1542-
return tt.switch(
1543-
tt.le(value, 0.0) | tt.le(lam, 0),
1544-
-np.inf,
1553+
return bound(
15451554
log1mexp(a),
1555+
0 <= value,
1556+
0 <= lam,
15461557
)
15471558

15481559

@@ -1654,10 +1665,17 @@ def logcdf(self, value):
16541665
a = self.mu
16551666
b = self.b
16561667
y = (value - a) / b
1657-
return tt.switch(
1658-
tt.le(value, a),
1659-
tt.log(0.5) + y,
1660-
tt.switch(tt.gt(y, 1), tt.log1p(-0.5 * tt.exp(-y)), tt.log(1 - 0.5 * tt.exp(-y))),
1668+
return bound(
1669+
tt.switch(
1670+
tt.le(value, a),
1671+
tt.log(0.5) + y,
1672+
tt.switch(
1673+
tt.gt(y, 1),
1674+
tt.log1p(-0.5 * tt.exp(-y)),
1675+
tt.log(1 - 0.5 * tt.exp(-y)),
1676+
),
1677+
),
1678+
0 < b,
16611679
)
16621680

16631681

@@ -1909,16 +1927,12 @@ def logcdf(self, value):
19091927
"""
19101928
mu = self.mu
19111929
sigma = self.sigma
1912-
z = zvalue(tt.log(value), mu=mu, sigma=sigma)
1930+
tau = self.tau
19131931

1914-
return tt.switch(
1915-
tt.le(value, 0),
1916-
-np.inf,
1917-
tt.switch(
1918-
tt.lt(z, -1.0),
1919-
tt.log(tt.erfcx(-z / tt.sqrt(2.0)) / 2.0) - tt.sqr(z) / 2,
1920-
tt.log1p(-tt.erfc(z / tt.sqrt(2.0)) / 2.0),
1921-
),
1932+
return bound(
1933+
normal_lcdf(mu, sigma, tt.log(value)),
1934+
0 < value,
1935+
0 < tau,
19221936
)
19231937

19241938

@@ -2220,8 +2234,15 @@ def logcdf(self, value):
22202234
m = self.m
22212235
alpha = self.alpha
22222236
arg = (m / value) ** alpha
2223-
return tt.switch(
2224-
tt.lt(value, m), -np.inf, tt.switch(tt.le(arg, 1e-5), tt.log1p(-arg), tt.log(1 - arg))
2237+
return bound(
2238+
tt.switch(
2239+
tt.le(arg, 1e-5),
2240+
tt.log1p(-arg),
2241+
tt.log(1 - arg),
2242+
),
2243+
m <= value,
2244+
0 < alpha,
2245+
0 < m,
22252246
)
22262247

22272248

@@ -2336,7 +2357,12 @@ def logcdf(self, value):
23362357
-------
23372358
TensorVariable
23382359
"""
2339-
return tt.log(0.5 + tt.arctan((value - self.alpha) / self.beta) / np.pi)
2360+
alpha = self.alpha
2361+
beta = self.beta
2362+
return bound(
2363+
tt.log(0.5 + tt.arctan((value - alpha) / beta) / np.pi),
2364+
0 < beta,
2365+
)
23402366

23412367

23422368
class HalfCauchy(PositiveContinuous):
@@ -2444,7 +2470,12 @@ def logcdf(self, value):
24442470
-------
24452471
TensorVariable
24462472
"""
2447-
return tt.switch(tt.le(value, 0), -np.inf, tt.log(2 * tt.arctan(value / self.beta) / np.pi))
2473+
beta = self.beta
2474+
return bound(
2475+
tt.log(2 * tt.arctan(value / beta) / np.pi),
2476+
0 <= value,
2477+
0 < beta,
2478+
)
24482479

24492480

24502481
class Gamma(PositiveContinuous):
@@ -2953,10 +2984,11 @@ def logcdf(self, value):
29532984
alpha = self.alpha
29542985
beta = self.beta
29552986
a = (value / beta) ** alpha
2956-
return tt.switch(
2957-
tt.le(value, 0.0),
2958-
-np.inf,
2987+
return bound(
29592988
log1mexp(a),
2989+
0 <= value,
2990+
0 < alpha,
2991+
0 < beta,
29602992
)
29612993

29622994

@@ -3255,17 +3287,21 @@ def logcdf(self, value):
32553287
nu = self.nu
32563288

32573289
# Alogithm is adapted from pexGAUS.R from gamlss
3258-
return tt.switch(
3259-
tt.gt(nu, 0.05 * sigma),
3260-
logdiffexp(
3261-
normal_lcdf(mu, sigma, value),
3262-
(
3263-
(mu - value) / nu
3264-
+ 0.5 * (sigma / nu) ** 2
3265-
+ normal_lcdf(mu + (sigma ** 2) / nu, sigma, value)
3290+
return bound(
3291+
tt.switch(
3292+
tt.gt(nu, 0.05 * sigma),
3293+
logdiffexp(
3294+
normal_lcdf(mu, sigma, value),
3295+
(
3296+
(mu - value) / nu
3297+
+ 0.5 * (sigma / nu) ** 2
3298+
+ normal_lcdf(mu + (sigma ** 2) / nu, sigma, value)
3299+
),
32663300
),
3301+
normal_lcdf(mu, sigma, value),
32673302
),
3268-
normal_lcdf(mu, sigma, value),
3303+
0 < sigma,
3304+
0 < nu,
32693305
)
32703306

32713307
def _distr_parameters_for_repr(self):
@@ -3753,8 +3789,13 @@ def logp(self, value):
37533789
-------
37543790
TensorVariable
37553791
"""
3756-
scaled = (value - self.mu) / self.beta
3757-
return bound(-scaled - tt.exp(-scaled) - tt.log(self.beta), self.beta > 0)
3792+
mu = self.mu
3793+
beta = self.beta
3794+
scaled = (value - mu) / beta
3795+
return bound(
3796+
-scaled - tt.exp(-scaled) - tt.log(self.beta),
3797+
0 < beta,
3798+
)
37583799

37593800
def logcdf(self, value):
37603801
"""
@@ -3774,7 +3815,10 @@ def logcdf(self, value):
37743815
beta = self.beta
37753816
mu = self.mu
37763817

3777-
return -tt.exp(-(value - mu) / beta)
3818+
return bound(
3819+
-tt.exp(-(value - mu) / beta),
3820+
0 < beta,
3821+
)
37783822

37793823

37803824
class Rice(PositiveContinuous):
@@ -4052,7 +4096,10 @@ def logcdf(self, value):
40524096
"""
40534097
mu = self.mu
40544098
s = self.s
4055-
return -log1pexp(-(value - mu) / s)
4099+
return bound(
4100+
-log1pexp(-(value - mu) / s),
4101+
0 < s,
4102+
)
40564103

40574104

40584105
class LogitNormal(UnitContinuous):
@@ -4360,14 +4407,12 @@ def logp(self, value):
43604407
-------
43614408
TensorVariable
43624409
"""
4363-
scaled = (value - self.mu) / self.sigma
4410+
mu = self.mu
4411+
sigma = self.sigma
4412+
scaled = (value - mu) / sigma
43644413
return bound(
4365-
(
4366-
-(1 / 2) * (scaled + tt.exp(-scaled))
4367-
- tt.log(self.sigma)
4368-
- (1 / 2) * tt.log(2 * np.pi)
4369-
),
4370-
self.sigma > 0,
4414+
(-(1 / 2) * (scaled + tt.exp(-scaled)) - tt.log(sigma) - (1 / 2) * tt.log(2 * np.pi)),
4415+
0 < sigma,
43714416
)
43724417

43734418
def logcdf(self, value):
@@ -4385,5 +4430,11 @@ def logcdf(self, value):
43854430
-------
43864431
TensorVariable
43874432
"""
4388-
scaled = (value - self.mu) / self.sigma
4389-
return tt.log(tt.erfc(tt.exp(-scaled / 2) * (2 ** -0.5)))
4433+
mu = self.mu
4434+
sigma = self.sigma
4435+
4436+
scaled = (value - mu) / sigma
4437+
return bound(
4438+
tt.log(tt.erfc(tt.exp(-scaled / 2) * (2 ** -0.5))),
4439+
0 < sigma,
4440+
)

pymc3/distributions/discrete.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ def logcdf(self, value):
349349
0,
350350
),
351351
0 <= value,
352+
0 <= n,
352353
0 < alpha,
353354
0 < beta,
354355
)

0 commit comments

Comments
 (0)