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

Can not sample from Von Mises #579

Closed
mbeltagy opened this issue Mar 7, 2017 · 3 comments · Fixed by #893
Closed

Can not sample from Von Mises #579

mbeltagy opened this issue Mar 7, 2017 · 3 comments · Fixed by #893

Comments

@mbeltagy
Copy link
Contributor

mbeltagy commented Mar 7, 2017

julia> v=VonMises(30,2)
Distributions.VonMises{Float64}=30.0, κ=2.0)

julia> rand(v)
ERROR: MethodError: no method matching quantile(::Distributions.VonMises{Float64}, ::Float64)
Closest candidates are:
  quantile(::Distributions.Distribution{Distributions.Univariate,S<:Distributions.ValueSupport}, ::Real) at /home/elbeltagy/.julia/Distributions/src/univariates.jl:163
  quantile(::Distributions.DiscreteUniform, ::Float64) at /home/elbeltagy/.julia/Distributions/src/univariate/discrete/discreteuniform.jl:114
  quantile(::Distributions.EmpiricalUnivariateDistribution, ::Float64) at /home/elbeltagy/.julia/Distributions/src/empirical.jl:61
  ...
 in rand(::Distributions.VonMises{Float64}) at /home/elbeltagy/.julia/Distributions/src/univariates.jl:64
@matbesancon
Copy link
Member

matbesancon commented Sep 19, 2018

It comes from the fact that the CDF does not have an analytical expression. I wonder if we should not move all such distributions to another package and then offer different routines to find a given quantile.

The other solution is to implement a simple search, we could do that in the mean time

@matbesancon
Copy link
Member

Requires #320 to be merged first

@yha
Copy link
Contributor

yha commented Jan 28, 2019

This is not due to a missing CDF implementation:

julia> v = VonMises(0,1)
VonMises{Float64}(μ=0.0, κ=1.0)

julia> rand(v,1)
1-element Array{Float64,1}:
 1.8643641705208138

julia> rand(v)
ERROR: MethodError: no method matching iterate(::VonMises{Float64})
[...]

The problem is that rand(::UnivariateDistribution,dims...) uses sampler(v), whereas rand(::UnivariateDistribution) bypasses the sampler completely.
This could be fixed by rand(v::VonMises) = rand(sampler(v)), but I'm not sure that's the best fix. Shouldn't the generic rand(d::UnivariateDistribution) refer to the sampler? It could, for example, call _rand(sampler(d)), with the generic _rand implementing the current fallback of using the cdf (currently there _rand! and no _rand).

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

Successfully merging a pull request may close this issue.

3 participants