search

JuliaStats / Distributions.jl

A Julia package for probability distributions and associated functions.
Other
1.11k stars 416 forks source link

pdf(::Binomial, ::Int) should return rational number. #889

Open newptcai opened 5 years ago

newptcai commented 5 years ago

If the parameters of binomial distribution are given in rational number, then perhaps the pdf function should also return rational number. But now it gives Float64, which can cause some unintended rounding problems.

julia> using Distributions

julia> dist = Binomial(2, 1//2)
Binomial{Rational{Int64}}(n=2, p=1//2)

julia> pdf(dist, 0)
0.24999999999999994

julia> pdf(dist, 1)
0.49999999999999994

julia> pdf(dist, 2)
0.24999999999999994
simonbyrne commented 5 years ago

Would be cool to have, though may not be that practical.

newptcai commented 5 years ago

I just need this for relatively small $n$. I can do write a bit script to do compute the probabilities, but it feels cleaner to extend the pdf function.

matbesancon commented 5 years ago

I think the delegate_statsfuns is also an issue, we cannot enjoy generics with functions called from R. In the case of Binomial, it's a bit sad to have to call R, PR welcome

matbesancon commented 5 years ago

Feel free to check https://github.com/JuliaStats/Distributions.jl/pull/138/files#diff-3696f20958ebe481685c9a355438c8d7

lukasbk commented 3 years ago

I'd be happy to work on this, but I'm mostly new contributing to open source. Why do you say delegate_statsfuns is "also" an issue? To me, it seems like it's the only place in binomial.jl where Rationals are unnecessarily converted into Floats. Wouldn't the solution be to implement the functionality of delegate_statsfuns for the binomial distribution directly in binomial.jl, indeed removing the dependence on R?

devmotion commented 3 years ago

The solution to the R dependency is https://github.com/JuliaStats/StatsFuns.jl/pull/113.