8000 BUG: Add parameter check to negative_binomial by Androp0v · Pull Request #21005 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: Add parameter check to negative_binomial #21005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 13, 2022
Merged

BUG: Add parameter check to negative_binomial #21005

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3d450d
Add parameter check in negative_binomial generator to avoid infinite …
Androp0v Feb 17, 2022
c1a95ff
Simplify check and add test for invalid combinations of p and n in ne…
Androp0v Feb 17, 2022
cc2ddfc
Performance improvements for negative_binomial checks
Androp0v Feb 18, 2022
2686f44
Improved negative_binomial checks documentation
Androp0v Feb 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions numpy/random/_generator.pyx
View file
Open in desktop
10000
Original file line number Diff line number Diff line change
Expand Up @@ -3021,9 +3021,40 @@ cdef class Generator:
... print(i, "wells drilled, probability of one success =", probability)

"""

cdef bint is_scalar = True
cdef double *_dn
cdef double *_dp

p_arr = <np.ndarray>np.PyArray_FROM_OTF(p, np.NPY_DOUBLE, np.NPY_ALIGNED)
is_scalar = is_scalar and np.PyArray_NDIM(p_arr) == 0
n_arr = <np.ndarray>np.PyArray_FROM_OTF(n, np.NPY_DOUBLE, np.NPY_ALIGNED)
is_scalar = is_scalar and np.PyArray_NDIM(n_arr) == 0

if not is_scalar:
check_array_constraint(n_arr, 'n', CONS_POSITIVE_NOT_NAN)
check_array_constraint(p_arr, 'p', CONS_BOUNDED_GT_0_1)
# Check that the choice of negative_binomial parameters won't result in a
# call to the poisson distribution function with a value of lam too large.
max_lam_arr = (1 - p_arr) / p_arr * (n_arr + 10 * np.sqrt(n_arr))
if np.any(np.greater(max_lam_arr, POISSON_LAM_MAX)):
raise ValueError("n too large or p too small")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a bit to the Notes section of the docstring explaining the cross-parameter restriction?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally should reference the note in the exception so that users know precisely what has happened


else:
_dn = <double*>np.PyArray_DATA(n_arr)
_dp = <double*>np.PyArray_DATA(p_arr)

check_constraint(_dn[0], 'n', CONS_POSITIVE_NOT_NAN)
check_constraint(_dp[0], 'p', CONS_BOUNDED_GT_0_1)
# Check that the choice of negative_binomial parameters won't result in a
# call to the poisson distribution function with a value of lam too large.
_dmax_lam = (1 - _dp[0]) / _dp[0] * (_dn[0] + 10 * np.sqrt(_dn[0]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cdef double _dmax_lam above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use libc.math.sqrt

from libc.math cimport sqrt

if _dmax_lam > POISSON_LAM_MAX:
raise ValueError("n too large or p too small")

return disc(&random_negative_binomial, &self._bitgen, size, self.lock, 2, 0,
n, 'n', CONS_POSITIVE_NOT_NAN,
p, 'p', CONS_BOUNDED_GT_0_1,
n_arr, 'n', CONS_NONE,
p_arr, 'p', CONS_NONE,
0.0, '', CONS_NONE)

def poisson(self, lam=1.0, size=None):
Expand Down
7 changes: 7 additions & 0 deletions numpy/random/tests/test_generator_mt19937.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,6 +1485,13 @@ def test_negative_binomial_p0_exception(self):
with assert_raises(ValueError):
x = random.negative_binomial(1, 0)

def test_negative_binomial_invalid_p_n_combination(self):
# Verify that values of p and n that would result in an overflow
# or infinite loop raise an exception.
with np.errstate(invalid='ignore'):
assert_raises(ValueError, random.negative_binomial, 2**62, 0.1)
assert_raises(ValueError, random.negative_binomial, [2**62], [0.1])

def test_noncentral_chisquare(self):
random = Generator(MT19937(self.seed))
actual = random.noncentral_chisquare(df=5, nonc=5, size=(3, 2))
Expand Down
0