Skip to content
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.

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

BUG: Fix overflow in special.binom. Issue #8346 #8347

Merged
merged 3 commits into from Feb 3, 2018

Conversation

@pvanmulbregt
Copy link
Contributor

commented Jan 30, 2018

Changed the order of division to avoid overflow for large n, k.

1/beta(1 + n - k, 1 + k) /(n+1) -> 1/(n + 1)/beta(1 + n - k, 1 + k)

BUG: Fix overflow in special.binom.
Changed the order of division to avoid overflow for large n, k.
@ev-br

This comment has been minimized.

Copy link
Member

commented Jan 31, 2018

Can you add a test? Some values from gh-8346 could work, ideally checked against mpmath or something else if possible

@pvanmulbregt

This comment has been minimized.

Copy link
Contributor Author

commented Jan 31, 2018

Test added. Values generated with SageMath.

@ev-br
Copy link
Member

left a comment

Looks good modulo trivial style comments.

dataset = np.asarray(dataset)
FuncData(cephes.binom, dataset, (0, 1), 2, rtol=1e-12).check()


This comment has been minimized.

Copy link
@ev-br

ev-br Feb 1, 2018

Member

Pep8 checker complains about two empty lines where it should have been one. Can you fix this please. Also while you're at it, what's the selfself argument, can change it back to a usual `self'.

This comment has been minimized.

Copy link
@person142

person142 Feb 1, 2018

Member

(Also use a list of tuples over a list of lists. It looks prettier.)

@ev-br ev-br merged commit 5531da3 into scipy:master Feb 3, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/project 88.08% (target 1%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ev-br

This comment has been minimized.

Copy link
Member

commented Feb 3, 2018

Thanks @pvanmulbregt

@ev-br ev-br added this to the 1.1.0 milestone Feb 3, 2018

@pvanmulbregt pvanmulbregt deleted the pvanmulbregt:lbeta_overflow branch Apr 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.