Skip to content
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

Sampling from MvNormal with integer arrays fails #1004

Closed
baggepinnen opened this issue Oct 30, 2019 · 7 comments · Fixed by #1262
Closed

Sampling from MvNormal with integer arrays fails #1004

baggepinnen opened this issue Oct 30, 2019 · 7 comments · Fixed by #1262
Labels

Comments

@baggepinnen
Copy link

When an MvNormal is created using integer arrays, sampling from the distribution fails.

julia> using Distributions

julia> rand(MvNormal([1,1],[1,1]), 10)
ERROR: MethodError: no method matching randn(::Random._GLOBAL_RNG, ::Type{Int64})
Closest candidates are:
  randn(::AbstractRNG, ::Type{T}, ::Tuple{Vararg{Int64,N}} where N) where T at /home/fredrikb/julia-1.3.0-rc4/share/julia/stdlib/v1.3/Random/src/normal.jl:181
  randn(::AbstractRNG, ::Type{T}, ::Integer, ::Integer...) where T at /home/fredrikb/julia-1.3.0-rc4/share/julia/stdlib/v1.3/Random/src/normal.jl:184
  randn(::AbstractRNG) at /home/fredrikb/julia-1.3.0-rc4/share/julia/stdlib/v1.3/Random/src/normal.jl:39
  ...
Stacktrace:
 [1] randn!(::Random._GLOBAL_RNG, ::Array{Int64,2}) at /home/fredrikb/julia-1.3.0-rc4/share/julia/stdlib/v1.3/Random/src/normal.jl:173
 [2] _rand! at /home/fredrikb/.julia/packages/Distributions/2i7UF/src/multivariate/mvnormal.jl:269 [inlined]
 [3] rand at /home/fredrikb/.julia/packages/Distributions/2i7UF/src/multivariates.jl:77 [inlined]
 [4] rand(::MvNormal{Int64,PDMats.PDiagMat{Int64,Array{Int64,1}},Array{Int64,1}}, ::Int64) at /home/fredrikb/.julia/packages/Distributions/2i7UF/src/multivariates.jl:76
 [5] top-level scope at REPL[10]:1

(v1.3) pkg> st Distributions
    Status `~/.julia/environments/v1.3/Project.toml`
  [31c24e10] Distributions v0.21.5
  [1fd47b50] QuadGK v2.0.3
  [2913bbd2] StatsBase v0.32.0

julia> VERSION
v"1.3.0-rc4.1"
@matbesancon
Copy link
Member

we could add a conversion method, PR welcome :)

@ben-schulz
Copy link

it looks like something similar was marked resolved way back in issue #498 -- but it would appear that change only addressed a type error on invocation of the constructor.

the code above fails because because:

dist = MvNormal([1,1],[1,1])
dist::MvNormal{Int32,PDMats.PDiagMat{Int32,Array{Int32,1}},Array{Int32,1}}

i.e., MvNormal successfully instantiates with element type <: Integer, where as an invocation to rand expects a float of some kind.

should the MvNormal constructor always promote the underlying element type to be usable with rand? that might make sense (seeing as it is a distribution), but it would seem to entail parameterizing MvNormal{T} by a type more restrictive than the current T<:Real

@JobJob
Copy link

JobJob commented May 13, 2020

Related - MvNormal with Array{Int} covariance also gives an unintuitive error:

julia> MvNormal([0, 0.0], [1 0; 0 1])
ERROR: MethodError: no method matching PDMats.PDMat{Int64,Array{Int64,2}}(::Int64, ::Array{Int64,2}, ::LinearAlgebra.Cholesky{Float64,Array{Float64,2}})

@mossr
Copy link

mossr commented Sep 20, 2020

Note that the fix for this (which originates in PDMats.jl) was merged and is now part of PDMats v0.10.1

@matbesancon
Copy link
Member

so should this be closed?

@mossr
Copy link

mossr commented Sep 20, 2020

The fix is part of a PDMats patch release (v0.10.1), but a fresh install of Distributions grabs v0.10 (I just tested this with Julia v1.5)—so it's up to you how to proceed. I doubt putting an explicit [compat] requirement for PDMats v0.10.1 is what you'd want.

@mossr
Copy link

mossr commented Sep 20, 2020

But my suggestion would be to—in fact—add the [compat] for "0.10.1" before closing (which would capture [0.10.1, 0.11.0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants