fix some bugs related to the Chernoff distribution.

[sl-solution]

Some functionality of the Chernoff() distribution depended on the newton() function which was not in the dependent list. I added the dependency and also polish some codes.

[codecov-io]

Codecov Report

Merging #1283 (c6a817a) into master (0b1ecad) will decrease coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
- Coverage   81.34%   81.31%   -0.04%     
==========================================
  Files         117      115       -2     
  Lines        6563     6541      -22     
==========================================
- Hits         5339     5319      -20     
+ Misses       1224     1222       -2     

Impacted Files	Coverage Δ
src/univariate/continuous/chernoff.jl	41.25% <100.00%> (+9.36%)	:arrow_up:
src/univariates.jl	61.11% <0.00%> (-5.56%)	:arrow_down:
src/common.jl	67.56% <0.00%> (-0.86%)	:arrow_down:
src/multivariates.jl	70.66% <0.00%> (-0.77%)	:arrow_down:
src/matrixvariates.jl	76.81% <0.00%> (-0.66%)	:arrow_down:
src/quantilealgs.jl	80.95% <0.00%> (-0.23%)	:arrow_down:
src/utils.jl	90.47% <0.00%> (-0.23%)	:arrow_down:
src/samplers.jl
src/matrix/wishart.jl	88.67% <0.00%> (+0.82%)	:arrow_up:
src/univariate/continuous/normalinversegaussian.jl	88.46% <0.00%> (+3.27%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0b1ecad...c6a817a. Read the comment docs.

[andreasnoack]

While this is definitely a bug, we'd have to discuss if adding the dependency is the right solution. It might be, but we've previously been a little reluctant to pull in such dependencies.

[sl-solution]

The other way should be to write a new newton function from the scratch. I think the capability of finding zeros of functions is needed for Distributions.jl (e.g. for obtaining the quantiles of a non-standard distributions) and maybe using Roots.jl is a good solution.

[matbesancon]

Rolling our own Newton in Distributions will not help making things tidier, I would be ok with Roots being added, it's a fairly established package

[codecov-commenter]

Codecov Report

Merging #1283 (0c882f9) into master (0b1ecad) will increase coverage by 1.86%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1283      +/-   ##
==========================================
+ Coverage   81.34%   83.21%   +1.86%     
==========================================
  Files         117      117              
  Lines        6563     6752     +189     
==========================================
+ Hits         5339     5619     +280     
+ Misses       1224     1133      -91     

Impacted Files	Coverage Δ
src/univariate/continuous/chernoff.jl	49.27% <100.00%> (+17.39%)	:arrow_up:
src/common.jl	65.71% <0.00%> (-2.71%)	:arrow_down:
src/mixtures/mixturemodel.jl	70.61% <0.00%> (-2.23%)	:arrow_down:
src/multivariate/mvtdist.jl	60.41% <0.00%> (-1.59%)	:arrow_down:
src/univariate/discrete/categorical.jl	75.00% <0.00%> (-1.55%)	:arrow_down:
src/univariate/continuous/generalizedpareto.jl	78.20% <0.00%> (-1.55%)	:arrow_down:
src/univariate/discrete/bernoulli.jl	89.23% <0.00%> (-1.54%)	:arrow_down:
src/multivariates.jl	70.66% <0.00%> (-0.77%)	:arrow_down:
src/univariate/continuous/chi.jl	94.59% <0.00%> (-0.76%)	:arrow_down:
src/matrixvariates.jl	76.81% <0.00%> (-0.66%)	:arrow_down:
... and 50 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0b1ecad...0c882f9. Read the comment docs.

[devmotion]

Personally, I think it would be fine to add Roots.jl as a dependency. It only depends on Printf and CommonSolve which itself has no dependencies and only defines solve and solve! stubs. I imagine that one could even use Roots to simplify some of the quantile algorithms, and possibly even to improve their efficiency.

@sl-solution, could you add an entry for Roots in the [compat] section in Project.toml? And could you add some tests for the bugs that this PR is supposed to fix? It seems that the lines that call newton haven't been tested.

[st--]

what's still needed to get this merged - if it's just the formatting, is the PR editable by maintainers? (also, how about incorporating JuliaFormatter into the workflow to automate that aspect?)

[matbesancon]

yeah we should add JuliaFormatter to this. It would be good to merge as many PRs as possible before because it will create conflicts all over

[devmotion]

I don't understand this PR anymore (even though I approved it :smile:). Wouldn't the "correct" fix be to use the existing quantile_newton etc. methods in https://github.com/JuliaStats/Distributions.jl/blob/ec8f7d82c59348443f799e0fb0394441fab31073/src/quantilealgs.jl#L37? And, if desired, add Roots in a separate PR in which we also make sure that it is used in other places where it could be useful? Also it seems newton would only be needed in the submodule but not at the toplevel in Distributions.