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

Changes from 1 commit
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
Fai 8000 led to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add parameter check in negative_binomial generator to avoid infinite … 
…loop for large values
  • Loading branch information
@Androp0v
Androp0v committed Feb 17, 2022
commit f3d450de272f491ab143d17957396a83ccacdb90
37 changes: 35 additions & 2 deletions numpy/random/_generator.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3021,9 +3021,42 @@ cdef class Generator:
... print(i, "wells drilled, probability of one success =", probability)

"""

cdef bint is_scalar = True

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_INT64, 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


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

_dp = PyFloat_AsDouble(p)
_in = PyFloat_AsDouble(n)

check_constraint(<double>_in, 'n', CONS_POSITIVE_NOT_NAN)
check_constraint(_dp, '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) / _dp * (_in + 10 * np.sqrt(_in))
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, 'n', CONS_NONE,
p, 'p', CONS_NONE,
0.0, '', CONS_NONE)

def poisson(self, lam=1.0, size=None):
Expand Down
0