Generic truncated normal & exponential

[cossio]

Fixes #1089.

Since the sampler for truncated normal takes samples from Exponential, I also modified the Exponential sampler to return values following the parameter type.

[codecov-io]

Codecov Report

Merging #1090 into master will increase coverage by 0.26%.
The diff coverage is 69.56%.

@@            Coverage Diff             @@
##           master    #1090      +/-   ##
==========================================
+ Coverage   79.36%   79.62%   +0.26%     
==========================================
  Files         112      113       +1     
  Lines        5500     5443      -57     
==========================================
- Hits         4365     4334      -31     
+ Misses       1135     1109      -26

Impacted Files	Coverage Δ
src/univariate/continuous/exponential.jl	92.3% <100%> (ø)	⬆️
src/truncated/normal.jl	84.21% <68.18%> (+0.13%)	⬆️
src/common.jl	30.43% <0%> (-21.74%)	⬇️
src/genericrand.jl	75% <0%> (-12.5%)	⬇️
src/univariate/continuous/locationscale.jl	79.41% <0%> (-11.77%)	⬇️
src/univariate/continuous/erlang.jl	67.85% <0%> (-7.15%)	⬇️
src/univariate/continuous/cauchy.jl	75% <0%> (-5.56%)	⬇️
src/utils.jl	80% <0%> (-5%)	⬇️
src/quantilealgs.jl	75.75% <0%> (-4.25%)	⬇️
src/univariate/continuous/biweight.jl	55.55% <0%> (-3.71%)	⬇️
... and 51 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 be19a9e...e50c2a7. Read the comment docs.

[matbesancon]

This looks good to me, but I don't remember what was the decision on return types for sampling so I'll let @andreasnoack give his opinion before moving this forward

[matbesancon]

ping @andreasnoack

[devmotion]

I'm wondering why this is needed and the generic implementation in

Distributions.jl/src/truncate.jl

Line 76 in 3f4856c

Base.eltype(::Type{Truncated{D, S, T} } ) where {D, S, T} = T

is not sufficient.

[burmecia]

I agree, that seems not necessary for me.

[rfarnham]

I'm coming from #1089
I've also run into the same problem. Any blockers for merging this?

[burmecia]

I'm also coming from #1089. Any reasons why this PR is still pending?

[devmotion]

I still wonder why/if the definition of eltype is needed (https://github.com/JuliaStats/Distributions.jl/pull/1090/files#r467471768).

In general, as far as I remember, the general policy is that there are no guarantees that eltype(d) === typeof(rand(d)), i.e., that the type of the samples is the same as of the parameters. There are many issues and discussions regarding this topic, e.g., #1252 and #1071.

[cossio]

In general the parameter type doesn't have to match the type of samples. E.g. a Poisson, has a float parameter but the samples are integers.

The decision that eltype(dist) should mean the type of parameters seems strange to me. The parameters can also be of different types.

[devmotion]

The decision that eltype(dist) should mean the type of parameters seems strange to me. The parameters can also be of different types.

I am very unsure if this is the right approach but it is definitely what is done currently (and also what you implement in this PR when you define eltype). In many distributions parameters are promoted to a common type and hence there is no problem with parameters of different types.

[devmotion]

BTW I just checked and already without this PR, as expected, we have

julia> eltype(truncated(Normal(0f0,1f0), 0f0, Inf32))
Float32

so the eltype definition above is not needed. In fact, I think it is wrong since it does not respect the types of the truncated distribution: with this PR also eltype(truncated(Normal(0f0,1f0), 0, Inf)) === Float32 even though all calculations of the truncated distribution are performed with Float64 (since logcdf(Normal(0f0, 1f0), Inf) isa Float64).

[cossio]

I am very unsure if this is the right approach but it is definitely what is done currently (and also what you implement in this PR when you define eltype). In many distributions parameters are promoted to a common type and hence there is no problem with parameters of different types.

Can we be more flexible about this? Seems like an unnecessary restriction.