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

fix infinite loop of Newton's method #666

Merged
merged 2 commits into from
Sep 20, 2017

Conversation

bicycle1885
Copy link
Contributor

quantile_newton and cquantile_newton find a quantile of a distribution
using Newton's method. However, their convergence will never be
satisfied when the initial point and the mode are the same. For example,
quantile_newton(Normal(0, 1), 0.5) will result in an infinite loop. This
patch fixes the problem by relaxing the convergence criterion.

This must be merged before #665.

quantile_newton and cquantile_newton find a quantile of a distribution
using Newton's method. However, their convergence will never be
satisfied when the initial point and the mode are the same. For example,
quantile_newton(Normal(0, 1), 0.5) will result in an infinite loop. This
patch fixes the problem by relaxing the convergence criterion.
Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

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

Ouch, good catch!

@andreasnoack andreasnoack merged commit 61ae3ca into JuliaStats:master Sep 20, 2017
LebedevRI added a commit to LebedevRI/Distributions.jl that referenced this pull request Sep 12, 2024
This is what the `quantile_newton()`/`cquantile_newton()` does,
because otherwise they were able to end up in an endless loops,
when the initial point and the mode are the same, see JuliaStats#666.

I'm not sure this is needed here, but the next change
is going to refactor them to use general `newton()`,
which would make this change anyway,
so unless we need the current behaviour,
let's do this change explicitly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants