`DiscreteNonParametric` and `Categorical` Construction Issue
Construction of a Categorical distribution seems to make a copy of the p vector. I see this through profiling, @btime and the fact that I can't see changes in the original vector after I create the Categorical. There are three issues I see:
-
Categoricaldocstring includes the following: "Note: The input vectorpis directly used as a field of the constructed distribution, without being copied." which seems incorrect. - Performance issues in critical sections of code where this allocation can really add up
- Bugs such as the following:
using Distributions
x = rand(3,5)
x = x ./ sum(x, dims=1) # each column is a valid probability vector
c = Categorical.(eachcol(x))
julia> c = Categorical.(eachcol(x)) ERROR: MethodError: Cannot
convertan object of type Vector{Float64} to an object of type SubArray{Float64, 1, Matrix{Float64}, Tuple{Base.Slice{Base.OneTo{Int64}}, Int64}, true}
I believe the underlying issue is that the DiscreteNonParametric inner constructor tries to sort and reorder everything, which creates a copy and then the constructor doesn't update the type.
Just an anecdote, I'm hitting the performance implication of this issue. I'm running discrete event simulations that construct Categorical distributions from several statistical models 100s of billions of times. The ability to reuse p would help greatly here. Alternatively, supplying a tuple instead of a vector would work too.
Did you compare it with https://github.com/JuliaStats/Distributions.jl/pull/1908?
Yes PR 1908 looks like it fixes this issue, thanks. Is there anything blocking it being merged?
It hasn't been approved yet.