Distributions.jl icon indicating copy to clipboard operation
Distributions.jl copied to clipboard

Quantile: Newton: introduce oscillation dampening

Open LebedevRI opened this issue 1 year ago • 4 comments
trafficstars

It is well known that Newton's method has an oscillation problem. In fact, it can reasonably happen on real-world, non-synthetic, distributions, when computing quantiles.

This situation appears easy to detect - just check that we've converged with the root we had two steps ago - i may be mistaken, but i'm not sure this can happen in any situation other than during oscillation.

Likewise, it appears to be easily circumventable, by picking a new x0 in-between the oscillation bounds, by using just half of the delta-x we'd apply otherwise.

NOTE: i do not have a proof that this solves all the issues, does not introduce new ones, nor do i know that the choice of, specifically, half of the delta-x is optimal.

Fixes https://github.com/JuliaStats/Distributions.jl/issues/1571 Fixes https://github.com/JuliaStats/Distributions.jl/issues/1833 Fixes https://github.com/JuliaStats/Distributions.jl/issues/1898

LebedevRI avatar Sep 12 '24 06:09 LebedevRI

Codecov Report

Attention: Patch coverage is 94.44444% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.94%. Comparing base (b219803) to head (b0bc737).

Files with missing lines Patch % Lines
src/quantilealgs.jl 94.44% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1899      +/-   ##
==========================================
- Coverage   85.99%   85.94%   -0.05%     
==========================================
  Files         144      144              
  Lines        8666     8668       +2     
==========================================
- Hits         7452     7450       -2     
- Misses       1214     1218       +4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Sep 12 '24 06:09 codecov-commenter

Thank you for the PR!

My general feeling though is that we should not re-invent and re-implement numerical algorithms that already exist and are better maintained in other packages such as Roots.jl. This also has the additional benefit that all other dependencies and users can benefit from any improvements or modifications possibly needed for Distributions.

devmotion avatar Sep 12 '24 07:09 devmotion

Sure, let me see what i can do...

LebedevRI avatar Sep 12 '24 23:09 LebedevRI

@devmotion https://github.com/JuliaStats/Distributions.jl/pull/1900

LebedevRI avatar Sep 13 '24 01:09 LebedevRI