Add convolve for DiscreteNonParametric #1523
Conversation
DiscreteNonParametric convolution has a very nice trivial closed form. It was not implemented. This pull request implements it.
src/convolution.jl
Outdated
| @@ -12,13 +12,16 @@ and one of | |||
| * [`NegativeBinomial`](@ref) | |||
| * [`Geometric`](@ref) | |||
| * [`Poisson`](@ref) | |||
| * [`DiscreteNonParametric`](@ref) | |||
| * [`Normal`](@ref) | |||
| * [`Cauchy`](@ref) | |||
| * [`Chisq`](@ref) | |||
| * [`Exponential`](@ref) | |||
| * [`Gamma`](@ref) | |||
| * [`MvNormal`](@ref) | |||
|
|
|||
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Doesn't preserve the type of the Vector, but perhaps this is better .... Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
use functions to access the support and probabilities, and write as one loop. Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Removed check args: We know the convovultion is a proper distribution.
|
Thanks for the suggestions, they were, all on the point. I think I addressed all of it. If I run into any more floating-point tricky cases, I'll add them, but as far as I can tell, it shouldn't cause errors, apart from any unexpected compiler optimization which causes different things to happen in the two loops. Let me know if anything more is needed. |
Codecov Report
@@ Coverage Diff @@
## master #1523 +/- ##
==========================================
+ Coverage 85.45% 85.47% +0.02%
==========================================
Files 128 128
Lines 7818 7829 +11
==========================================
+ Hits 6681 6692 +11
Misses 1137 1137
Continue to review full report at Codecov.
|
| @testset "DiscreteNonParametric" begin | ||
| d1 = DiscreteNonParametric([0,1],[0.5,0.5]) | ||
| d2 = DiscreteNonParametric([1,2],[0.5,0.5]) | ||
| d_eps= DiscreteNonParametric([-1* eps(Float64),0.0,eps(Float64),1.0],[1//4,1//4,1//4,1//4]) |
There was a problem hiding this comment.
Maybe use
| d_eps= DiscreteNonParametric([-1* eps(Float64),0.0,eps(Float64),1.0],[1//4,1//4,1//4,1//4]) | |
| d_eps= DiscreteNonParametric([prevfloat(0.0),0.0,nextfloat(0.0),1.0], fill(1//4, 4)) |
| d1 = DiscreteNonParametric([0,1],[0.5,0.5]) | ||
| d2 = DiscreteNonParametric([1,2],[0.5,0.5]) | ||
| d_eps= DiscreteNonParametric([-1* eps(Float64),0.0,eps(Float64),1.0],[1//4,1//4,1//4,1//4]) | ||
| d10= DiscreteNonParametric(collect(1:10)//10,ones(Int,10)//10) |
There was a problem hiding this comment.
| d10= DiscreteNonParametric(collect(1:10)//10,ones(Int,10)//10) | |
| d10= DiscreteNonParametric(1//10:1//10:1, fill(1//10, 10)) |
| @@ -47,6 +48,20 @@ end | |||
| convolve(d1::Poisson, d2::Poisson) = Poisson(d1.λ + d2.λ) | |||
|
|
|||
|
|
|||
|
|
|||
| @test probs(d_int_simple) == [0.25,0.5,0.25] | ||
|
|
||
| d_rat = convolve(d10, d10) | ||
| @test support(d_rat)[1] == 1//5 |
There was a problem hiding this comment.
I guess this could be made more precise?
| @test support(d_rat)[1] == 1//5 | |
| @test support(d_rat) == 1//5:1//10:2 |
| @test probs(d_rat)[1] == 1//100 | ||
|
|
||
| d_float_supp = convolve(d_eps, d_eps) | ||
| @test support(d_float_supp)[3] == 0.0 |
There was a problem hiding this comment.
I think this will be
| @test support(d_float_supp)[3] == 0.0 | |
| @test support(d_float_supp) == [2 * prevfloat(0.0), prevfloat(0.0), 0.0, nextfloat(0.0), 2 * nextfloat(0.0), 1.0, 2.0] |
since numerically prevfloat(0.0) + 1 = 1 = nextfloat(0.0) + 1.
|
I think this would be a nice PR to merge. I implemented the latest suggestions from @devmotion and PR'd at #1850. Happy to either see this PR fixed up or merge that. Thank you! |
|
Fixed by #1850. |
DiscreteNonParametric convolution has a very nice trivial closed form. It was not implemented.
This pull request implements it.