Fix #894 Normal distribution and its type parameter

[halleysfifthinc]

I didn't see any similar tests so I'm not sure if this needs any tests. I don't imagine that fixing the eltype would be particularly breaking (although it is changing the output type of rand(n::Normal, dims)).

However, the fix for the single rand return type could change an incorrect return type into an error, if the eltype of the Normal dist. doesn't support randn. I noticed this when using DoubleFloats (prior to v0.8.1 of DoubleFloats.jl).

# Before PR
julia> n = Normal(Double64(1), Double64(0.1))
Normal{DoubleFloat{Float64}}(μ=1.0, σ=0.1)

julia> x = rand(n)
0.9972028245261662

julia> typeof(x)
DoubleFloat{Float64}

julia> n32 = Normal(1f0, 0.1f0)
Normal{Float32}(μ=r1.0f0, σ=0.1f0)

julia> x32 = rand(n32)
0.9335312783262796

julia> typeof(x32)
Float64

# After PR (with DoubleFloats.jl v0.7.24 as an example of a type that doesn't support `randn`)
julia> n = Normal(Double64(1), Double64(0.1))
Normal{DoubleFloat{Float64}}(μ=1.0, σ=0.1)

julia> x = rand(n)
ERROR: MethodError: no method matching randn(::Random.MersenneTwister, ::Type{DoubleFloat{Float64}})

julia> n32 = Normal(1f0, 0.1f0)
Normal{Float32}(μ=1.0f0, σ=0.1f0)

julia> x32 = rand(n32)
0.9599364f0

julia> typeof(x32)
Float32

So the previous behavior was both inconsistent, and not accurate in the case of the DoubleFloats. The return type was correct due to the type promotion, but the random number wasn't being generated with the precision of the DoubleFloat. randn support for DoubleFloats was added in v0.8.1.

[matbesancon]

Not sure if we shouldn't watch out here, since randn does not support all types

[halleysfifthinc]

Precisely the concern I mentioned above; although on balance, I think this is an improvement. The prior behavior wasn't technically correct, either due to the wrong output type, or the right output type but the code not actually doing what one would expect (as shown in the examples I gave above).

[matbesancon]

@halleysfifthinc thanks a lot for that. Could you add a simple test with for example the eltype, for instance with a Dual from ForwardDiff or Float32

[halleysfifthinc]

Yeah I can add some tests. Where would be appropriate? I didn't see any for the Normal distribution. I'm also not familiar with Dual/ForwardDiff, but I can definitely add some for Float32/Float64.

[matbesancon]

If you don't see any testset / test file where it fits you can just create one

[matbesancon]

ping @halleysfifthinc

[codecov-io]

Codecov Report

Merging #896 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #896      +/-   ##
==========================================
+ Coverage   73.78%   73.79%   +<.01%     
==========================================
  Files         107      107              
  Lines        5200     5201       +1     
==========================================
+ Hits         3837     3838       +1     
  Misses       1363     1363

Impacted Files	Coverage Δ
src/univariate/continuous/normal.jl	94.38% <100%> (+0.06%)	⬆️

---

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 ba2b28a...ecd7143. Read the comment docs.

[matbesancon]

Could you merge master into your branch? This should put cover back

…

On Fri, May 24, 2019 at 7:45 PM Codecov ***@***.***> wrote: Codecov <https://codecov.io/gh/JuliaStats/Distributions.jl/pull/896?src=pr&el=h1> Report ❗️ No coverage uploaded for pull request base ***@***.***). Click here to learn what that means <https://docs.codecov.io/docs/error-reference#section-missing-base-commit> . The diff coverage is 100%. [image: Impacted file tree graph] <https://codecov.io/gh/JuliaStats/Distributions.jl/pull/896?src=pr&el=tree> @@ Coverage Diff @@ ## master #896 +/- ## ========================================= Coverage ? 73.79% ========================================= Files ? 107 Lines ? 5201 Branches ? 0 ========================================= Hits ? 3838 Misses ? 1363 Partials ? 0 Impacted Files <https://codecov.io/gh/JuliaStats/Distributions.jl/pull/896?src=pr&el=tree> Coverage Δ src/univariate/continuous/normal.jl <https://codecov.io/gh/JuliaStats/Distributions.jl/pull/896/diff?src=pr&el=tree#diff-c3JjL3VuaXZhcmlhdGUvY29udGludW91cy9ub3JtYWwuamw=> 94.38% <100%> (ø) ------------------------------ Continue to review full report at Codecov <https://codecov.io/gh/JuliaStats/Distributions.jl/pull/896?src=pr&el=continue> . *Legend* - Click here to learn more <https://docs.codecov.io/docs/codecov-delta> Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov <https://codecov.io/gh/JuliaStats/Distributions.jl/pull/896?src=pr&el=footer>. Last update db09a0a...273960a <https://codecov.io/gh/JuliaStats/Distributions.jl/pull/896?src=pr&el=lastupdated>. Read the comment docs <https://docs.codecov.io/docs/pull-request-comments> . — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#896?email_source=notifications&email_token=AB2FDMRNAKG72GKGZVBJPBTPXASTXA5CNFSM4HM2FW6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWGDHTA#issuecomment-495727564>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2FDMUOCTSZTF4OU7X22NLPXASTXANCNFSM4HM2FW6A> .

-- Mathieu Besançon

[matbesancon]

thanks @halleysfifthinc !