Conversation
Codecov Report
@@ Coverage Diff @@
## master #204 +/- ##
==========================================
- Coverage 91.12% 91.01% -0.11%
==========================================
Files 24 24
Lines 1712 1726 +14
==========================================
+ Hits 1560 1571 +11
- Misses 152 155 +3
Continue to review full report at Codecov.
|
|
Just throwing this out there -- would it be better to replace the |
| end | ||
|
|
||
|
|
||
| function MVDiscreteRV(q::TV1) where TV1<:AbstractArray |
There was a problem hiding this comment.
I think it makes sense to have the only inner constructor take just q. We wouldn't have to do the length(Q) == prod(dims) check
We don't really want people to create these in any other way
|
|
||
| - `out::Vector{NTuple{Int}}`: `k` draws from `d` | ||
| """ | ||
| Base.rand(d::MVDiscreteRV{T1,T2,K,TI}, k::V) where {T1,T2,K,TI,V} = |
There was a problem hiding this comment.
What is k supposed to be? I think it should have k::Integer instead of a free type param
| Base.rand(d::MVDiscreteRV{T1,T2,K,TI}, k::V) where {T1,T2,K,TI,V} = | ||
| NTuple{K,TI}[rand(d) for i in 1:k] | ||
|
|
||
| function Base.rand!(out::AbstractArray{NTuple{K,TI}}, d::MVDiscreteRV) where {K,TI} |
There was a problem hiding this comment.
Will you please add a tests for this method?
This code is very similar to the code that is in the QE.jl library for
DiscreteRV.We could simply treat multivariate discrete random variables as a univariate discrete random variable (and just make everything long vectors), but that requires working with conversions between linear and cartesian indexing. It also simplifies the interpretation of joint distributions.
This mostly mimics the code from
DiscreteRVand adds a call toind2subso that it does the conversions for you.