-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Product of distributions #473
Comments
Kind of like the |
Yes... the only problem is: julia> using POMDPTools, Distributions
julia> d = SparseCat([1,2], [0.5, 0.5]);
julia> Product([d, d])
ERROR: MethodError: no method matching Product(::Vector{SparseCat{Vector{Int64}, Vector{Float64}}})
Closest candidates are:
Product(::V) where {S<:ValueSupport, T<:UnivariateDistribution{S}, V<:AbstractVector{T}} at ~/.julia/packages/Distributions/Spcmv/src/multivariate/product.jl:25 So, either we need to fit our distributions into the Distributions.jl type hierarchy or make our own product. This is why I don't like elaborate type hierarchies like the one in Distributions.jl. It scares me every time I see one. (Side note: that is another reason why I am not excited about DomainSets - it has a very elaborate type hierarchy) |
Yep, we can make our own. Or, it seems like we could provide a wrapper around general distributions from Distributions.jl? |
Out of curiosity: what is the problem with fitting it into the Distributions.jl type hierarchy? |
@johannes-fischer thanks for asking! It is very useful for me to have to work out how to explain things like this. Let's say we wanted to put put struct SparseCat{V, P} <: Distribution{F<:VariateForm, S<:ValueSupport}
vals::V
probs::P
end So then we need to choose a I suppose I could do something like struct SparseCat{V, P, F<:VariateForm} <: Distribution{F, Discrete}
vals::V
probs::P
end but I would still have to make a new If Note: POMDPs.jl is also guilty of this: we should just have |
@zsunberg I see, thanks for the detailed explanation! What would be the issue with defining struct SparseCat{V, P} <: Distribution{VariateForm, Discrete}
vals::V
probs::P
end The fact that struct SparseCat{V, P, F<:VariateForm} <: Distribution{F, Discrete}
vals::V
probs::P
end
SparseCat(vals::V, probs::P) where {V,P} = SparseCat{V, P, VariateForm}(vals, probs)
SparseCat(::Type{F}, vals::V, probs::P) where {V, P, F<:VariateForm} = SparseCat{V, P, F}(vals, probs) to allow for |
Yeah, actually I guess some form of what you are suggesting wouldn't be too bad. I think the niceness of integrating with Distributions.jl outweighs the bit of ugliness, and it looks like Distributions.jl would move in the direction of traits if someone has time to work on it in the future. I like the second approach in general better. But I think we should try a little harder to infer the VariateForm in the constructor. |
If anyone wants to take a stab at making the distributions in POMDPTools compatible with Distributions.jl, it would be a nice contribution. |
I saw that Constructing the product distribution works at least when giving the |
We should add a way to make a product of two independent distributions to POMDPTools, i.e.
First we should consult Distributions.jl to see if there is already something like it.
The text was updated successfully, but these errors were encountered: