Uniform is not type stable + Bounds type should be independent of distribution type

[aminya]

The current definition of Uniform is not type stable. I tried to make a method in #1035 to make it usable, but it doesn't help.

using Distributions
set = Uniform(Float32(0.0), Float32(2.0))
v = rand(set, 3)

julia> eltype(v)
Float64

The definition should not be dependent on the type of a and b. From the mathematical point of view, a and b bounds may not be part of the distribution and so their type shouldn't matter. Actually their probability is equal to 0!
https://github.com/JuliaStats/Distributions.jl/blob/master/src/univariate/continuous/uniform.jl#L26

Instead, the user should provide the type explicitly. It should use Float64 if no type is provided.

The following definition is my proposed definition:
for array like constructor (Uniform{T}(a, b))

struct Uniform{T<:Real} <: ContinuousUnivariateDistribution
    a::T1 where {T1 <: Real}
    b::T2 where {T2 <: Real}
    Uniform{T}(a, b) where {T <: Real} = new{T}(a, b)
end

for zeros like constructor (Uniform(T, a, b))

function Uniform(::Type{T}, a, b) where {T <: Real}
    return Uniform{T}(a, b)
end

No type specified constructor:

function Uniform(a, b)
    return Uniform{Float64}(a, b)
 end

API examples:

julia> Uniform(1,2)
Uniform{Float64}(a=1, b=2)

julia> Uniform(Float32, 1,2)
Uniform{Float32}(a=1, b=2)

julia> Uniform{Int64}(1,2)
Uniform{Int64}(a=1, b=2)

[andreasnoack]

This isn't really what is usually called type stability. rand(::Uniform) is perfectly type stable according the usual definition. The problem is that we hardcode for Float64. Historically, the package only supported Float64 and we have gradually moved towards generic element support. It looks like Uniform still relies on Rmath which only supports Float64 but the necessary methods should be fairly simple for Uniform so making it fully should be doable without too much effort. However, I don't think the struct definition needs to change to achieve this.

[aminya]

This isn't really what is usually called type stability. rand(::Uniform) is perfectly type stable according the usual definition. The problem is that we hardcode for Float64. Historically, the package only supported Float64 and we have gradually moved towards generic element support. It looks like Uniform still relies on Rmath which only supports Float64 but the necessary methods should be fairly simple for Uniform so making it fully should be doable without too much effort. However, I don't think the struct definition needs to change to achieve this.

The current definition imposes some restrictions on the API that are not necessary. Removing ::T anotation from struct defintion opens up the door for a better API.

In that case, all these redundant defenitions can be removed:
https://github.com/JuliaStats/Distributions.jl/blob/master/src/univariate/continuous/uniform.jl#L37-L38

[andreasnoack]

The current definition imposes some restrictions on the API that are not necessary.

It's usually beneficial to make these kinds of restrictions. It makes it easier when defining methods for the struct since you don't have to consider as many possibilities. I'm also a bit puzzled by your proposal since it doesn't restrict the fields at all, so you could write something like

julia> Uniform2{Float32}("W", :fwe)
Uniform2{Float32}(
a: W
b: fwe
)

[aminya]

You are right, I edited my proposal, so that a and b are Real

[andreasnoack]

Another thing, if the struct fields are abstract, as in you proposal, the compiler can't infer the return types for functions of the struct. This causes a big overhead and is the reason why the distributions initially used to be hard coded for Float64 and now are mostly parametric, i.e. the types of the fields are parameters of the distribution.

I'm not completely sure about the role of the type parameter in your proposal. Is it so allow for different types for the parameters of the distribution and realized values of it? In theory that could make sense but we always have to balance generality with the overhead that more flexible struct impose on the methods defined for the struct. So far, I think it's been mostly sufficient to require the types of the parameters and realizations to be the same.

Btw, in #1041 (comment) I was wrong about the Rmath part. I was fooled

Distributions.jl/src/univariate/continuous/uniform.jl

Line 41 in 532feac

@distr_support Uniform d.a d.b

which I read as the macro we use for defining methods based on Rmath. I think that maybe the only thing that needs to change here is

Distributions.jl/src/univariate/continuous/uniform.jl

Line 111 in 532feac

rand(rng::AbstractRNG, d::Uniform) = d.a + (d.b - d.a) * rand(rng)

where rand(rng) causes promotion. Something like

 rand(rng::AbstractRNG, d::Uniform{T}) where T = d.a + (d.b - d.a) * convert(T, rand(rng))

might be sufficient to fix the issue here.

[aminya]

I'm not completely sure about the role of the type parameter in your proposal. Is it so allow for different types for the parameters of the distribution and realized values of it? In theory that could make sense but we always have to balance generality with the overhead that more flexible struct impose on the methods defined for the struct. So far, I think it's been mostly sufficient to require the types of the parameters and realizations to be the same.

From the mathematical point of view, a and b bounds may not be part of the distribution and so their type shouldn't matter. Actually their probability is equal to 0!
https://en.wikipedia.org/wiki/Uniform_distribution_(continuous)

My problem was that for example, for creating a simple Float32, we need to write this giant thing:

Uniform(Float32(1), Float32(2))

while we can just simply write

Uniform{Float32}(1,2)

The latter seems more natural to me.

[SyxP]

As mentioned on the discourse, having probability $0$ doesn't mean it can never occur as an output.

The other points you mentioned are more to do with improving the syntax for construction and overloading rand(Uniform{T}) to return an object of type T.

[aminya]

If you think that we should keep the type of a and b the same internally, this definition also works (compared to my current PR #1045):

UniformOverhaul2

struct Uniform{T} <: ContinuousUnivariateDistribution
    a::T
    b::T

    # Uniform{T}(a::T, b::T) constructor
    function Uniform{T}(a::T, b::T; check_args=true) where {T <: Real}
        check_args && @check_args(Uniform, a < b)
        return new{T}(a, b)
    end

    # Abstract a,b - Uniform{T}(a, b) constructor
    function Uniform{T}(a::Real, b::Real; check_args=true) where {T <: Real}
        check_args && @check_args(Uniform, a < b)
        return new{T}(T(a), T(b))
    end

    # Uniform{T}(a::T, b::T) constructor
    function Uniform{T}(a::T, b::T) where {T <:Complex}
        new{T}(a, b)
    end

    # Abstract a,b - Uniform{T}(a, b) constructor
    function Uniform{T}(a, b) where {T <: Complex}
        return new{T}(T(a), T(b))
    end
end
# Real

# zeros() like constructor (Uniform(T, a::T, b::T))
function Uniform(::Type{T}, a::T, b::T; check_args=true) where {T <: Real}
    return Uniform{T}(a, b, check_args = check_args)
end

# Abstract a,b - zeros() like constructor (Uniform(T, a, b))
function Uniform(::Type{T}, a, b; check_args=true) where {T <: Real}
    return Uniform{T}(T(a), T(b), check_args = check_args)
end

# No type specified constructor:
function Uniform(a::Float64, b::Float64; check_args=true)
    return Uniform{Float64}(a, b, check_args = check_args)
end

# Abstract a,b - no type specified constructor:
function Uniform(a, b; check_args=true)
    return Uniform{Float64}(Float64(a), Float64(b), check_args = check_args)
end

# Complex

# zeros() like constructor (Uniform(T, a::T, b::T))
function Uniform(::Type{T}, a::T, b::T) where {T <: Complex}
    return Uniform{T}(a, b)
end

# Abstract a,b - zeros() like constructor (Uniform(T, a, b))
function Uniform(::Type{T}, a, b) where {T <: Complex}
    return Uniform{T}(T(a), T(b))
end

# No type specified constructor:
function Uniform(a::ComplexF64, b::ComplexF64)
    return Uniform{ComplexF64}(a, b)
end