Fix Dirichlet return type by adding Dirichlet(d::Integer, alpha::Integer) constructor.

[ztangent]

After #1243 was merged, it looks like the old constructor for Dirichlet(d::Integer, alpha::Integer) was removed, and the new constructor introduced creates a situation where the eltype of Dirichlet ends up being Int64 if alpha is given as an Int64 (see ccebbd7#diff-65f8aff731d04673339bb6e204537e9f99b6ac88797bd476a816ceaab046077aL52-R41 ).

This was causing issues in some downstream code I've been using, which converts the return value of rand(dist::Dirichlet{T}, ...) to type T. When T was Int64, this created errors, because the conversion from a float wasn't possible.

This PR should address the issue by re-introducing a constructor that handles the case where alpha is an Integer, and converts alpha to Float64.

[devmotion]

The integer-valued parameters and are intentional: #1262 Instead of promoting the parameters, we just ensure that the samples are floating point numbers.

Hence I don't think anything is missing here and suggest that you modify your implementation of rand as done in #1262.

[ztangent]

Hmm, I agree that the parameters can be kept as integer-valued, but then it seems like there needs to be a way for eltype to correctly return Float64 on Dirichlet distributions. The current version has the following behavior:

julia> eltype(Dirichlet(5, 2))
Int64

This doesn't seem to meet the documented behavior for eltype:

"""    eltype(::Type{Sampleable})

The default element type of a sample. This is the type of elements of the samples generated
by the `rand` method. However, one can provide an array of different element types to
store the samples using `rand!`."""

This affects downstream code I have, which determines the expected type of the samples based on eltype. Perhaps the best way to address this is by augmenting the definition eltype for Dirichlet? I.e. the general case will handle all floats (Float32, Float64, etc.):

Base.eltype(::Type{<:Dirichlet{T}}) where {T} = T

but this special case will handle the case when there are Integer parameters:

Base.eltype(::Type{<:Dirichlet{<:Integer}}) = Float64

[ztangent]

It also looks like the default implementation of eltype already handles this issue, but it's over-ridden for specific distributions for reasons that are unclear to me (is it so that we can sample Float32s instead of Float64?)

Distributions.jl/src/common.jl

Lines 58 to 66 in b35f091

	"""
	eltype(::Type{Sampleable})
	The default element type of a sample. This is the type of elements of the samples generated
	by the `rand` method. However, one can provide an array of different element types to
	store the samples using `rand!`.
	"""
	Base.eltype(::Type{<:Sampleable{F,Discrete}}) where {F} = Int
	Base.eltype(::Type{<:Sampleable{F,Continuous}}) where {F} = Float64

[devmotion]

This doesn't seem to meet the documented behavior for eltype:

In my experience, eltype is mostly a heuristic but not a reliable and correct way to determine the type of samples. I ran into these problems as well and am not satisfied with the current situation either. However, Distributions has grown and been extended over multiple years and some design decisions taken early on were probably the right ones back then but are limiting or inappropriate nowadays. E.g., another problem is that eltype is supposed to return the element-type of the samples for multi- and matrixvariate distributions but often it would be important to know the type of the vector or matrix as well (you might not always want to sample Vector or Matrix but e.g. also static vectors or Cholesky decompositions).

It also looks like the default implementation of eltype already handles this issue, but it's over-ridden for specific distributions for reasons that are unclear to me (is it so that we can sample Float32s instead of Float64?)

Yes, it should be redefined for most distributions since e.g. otherwise usually automatic differentiation is broken. There were (and probably are) multiple bugs caused by missing eltype implementations.

[devmotion]

And I am still convinced that one should not convert the parameters only to "fix" a then still broken eltype situation: all other computations such as pdf, logpdf etc. work just fine with integer-valued parameters. In previous versions even integer-valued vectors were converted to floating point numbers as a "fix" which causes unnecessary allocations and, in my opinion, seems not like a proper solution.

[ztangent]

Yup, I'm fine with keeping type parameters as integers, but will the following suggested fix somehow break AD? It seems like eltype ought to meet its defined spec, or otherwise there should a decision about its proper behavior:

Base.eltype(::Type{<:Dirichlet{<:Integer}}) = Float64

I'm happy to create a separate issue / PR about this. This is important for some work I'm doing in https://github.com/probcomp/Genify.jl.

[devmotion]

This definition would be not consistent with the current behaviour but would have to be

Base.eltype(::Type{<:Dirichlet{T}}) where {T<:Integer} = float(T)

This was actually (for general T) my initially proposed fix in the PR linked above (ba1eb85 (#1262)). However, it was argued that this is not the right approach, e.g., in #1262 (comment):

In any case, I don't think this PR is the right fix since it "lies" about the element type. The element type usually refers to the type of the parameters and not the type of the sample space.

[devmotion]

The same argument has been brought forward multiple times, e.g., in #1071 (comment):

The element type of a distribution specifies the type of the parameters, not the type of the variates produced by rand

So I think the documentation should be fixed.

[devmotion]

In my experience, the only reliable way to determine the type of the samples is to draw a sample with rand.

[ztangent]

Thanks for sharing this context! It's bizarre to me that eltype now refers to the type of the parameters instead of the type of the elements returned by rand. o_O I would have been in favor of your initially proposed fix. But maybe I'll continue this on #1071.