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

MarkovChain simulate: Match with python version #77

Merged
merged 4 commits into from
Jan 12, 2016
Merged

Conversation

oyamad
Copy link
Member

@oyamad oyamad commented Nov 5, 2015

This is to propose matching the simulate method with that in the Python version (see QuantEcon/QuantEcon.py#146 (comment)). It works as follows:

julia> using QuantEcon

julia> mc = MarkovChain([0.4 0.6; 0.2 0.8])
Discrete Markov Chain
stochastic matrix:
2x2 Array{Float64,2}:
 0.4  0.6
 0.2  0.8


julia> simulate(mc, 5)
5-element Array{Int64,1}:
 2
 2
 1
 1
 2

julia> simulate(mc, 5, [1, 2])
5x2 Array{Int64,2}:
 1  2
 1  1
 2  1
 2  1
 2  2

julia> simulate(mc, 5, num_reps=3)
5x3 Array{Int64,2}:
 1  2  2
 2  1  1
 2  2  1
 1  2  2
 1  1  1

julia> simulate(mc, 5, 1, num_reps=3)
5x3 Array{Int64,2}:
 1  1  1
 1  1  2
 2  2  2
 2  2  2
 1  2  2

julia> simulate(mc, 5, [1, 2], num_reps=3)
5x6 Array{Int64,2}:
 1  2  1  2  1  2
 2  2  1  1  2  1
 2  1  2  2  1  2
 2  2  1  2  1  2
 2  2  2  2  2  2

As discussed in #52 (comment), it uses searchsortedfirst (via DiscreteRV). The method using searchsortedfirst (simulate) seems faster than the previous one using Categorical (mc_sample_path) for medium- and large-size Markov chains; see mc_simulate.jl.ipynb.

In passing, Numba seems to do a good job; see mc_simulate.py.ipynb.

I am not sure if I properly exploited the multiple dispatch system of Julia. Any suggestion/correction etc will be welcome!

function simulate(mc::MarkovChain,
ts_length::Int,
init::Vector{Int};
num_reps::Union{Int, Void}=nothing)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the Union and nothing here. Just set a default argument of num_reps=1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted.

@sglyon
Copy link
Member

sglyon commented Nov 5, 2015

Looks great overall, thanks for adding this.

I have another request: can we document how the columns are ordered when num_reps > 1?

@oyamad
Copy link
Member Author

oyamad commented Nov 6, 2015

"If init is a vector, the sample path from the j-th repetition of the simulation with initial state init[i] is stored in the i*j-th (j-1)*num_reps+i-th column of the matrix X."

Is this what you mean?

@sglyon
Copy link
Member

sglyon commented Jan 11, 2016

Sorry this has sat for so long.

@oyamad what's the status on this, do you recall?

@oyamad
Copy link
Member Author

oyamad commented Jan 12, 2016

@spencerlyon2 Sorry I had forgotten about this PR.

I now remembered why I did num_reps::Union{Int, Void}=nothing in the first simulate method; it is called from the other methods with num_reps=nothing. There may be a good way to avoid it, but I left it as is. Anyway, the issue is whether it is a good implementation that if num_reps is not supplied, the output is a 1-dimensitonal array, while if it is supplied, the output is 2-dimensional even when num_reps=1.

@sglyon
Copy link
Member

sglyon commented Jan 12, 2016

Ahh that's right thanks @oymad.

The behavior you described will be type stable, so I think it is a good choice. I'm happy to merge as is.

Thanks

@sglyon sglyon merged commit 1a36aaa into master Jan 12, 2016
@oyamad
Copy link
Member Author

oyamad commented Jan 12, 2016

@spencerlyon2 Thanks for taking care of the tests.

@oyamad oyamad deleted the mc_simulate branch January 12, 2016 07:06
@sglyon
Copy link
Member

sglyon commented Jan 12, 2016

You're welcome, thanks again for the quality work!

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 this pull request may close these issues.

2 participants