MarkovChain.simulate API change; random_state option added

[oyamad]

This follows the discussions in #146.
(I hope I did "rebase" in the right way.)

1. I propose to change the simulate method as simulate(ts_length, init=None, num_reps=None, random_state=None); see Update MarkovChain with replicate method and Future Numba Improvements #146 (comment).
   • init now does not accept a distribution; see Update MarkovChain with replicate method and Future Numba Improvements #146 (comment) and Update MarkovChain with replicate method and Future Numba Improvements #146 (comment).
2. random_state option is added to MarkovChain.simulate (not to MarkovChain.__init__).
3. test_mc_sample_path is added with random_state specified.

[oyamad]

Regarding testing methods/functions with randomness (i.e., MarkovChain.simulate and mc_sample_path), I wonder what we want to test. The current test_mc_sample_path verifies that given a certain seed value, mc_sample_path returns a certain sequence of integers (states). Is it what should be tested?

If MarkovChain.simulate and mc_sample_path are properly implemented,

• if the markov chain is irreducible, for a well-behaved seed value (i.e., a seed value that generates uniformly distributed numbers), along a sample path generated by these functions (with long enough length) the frequency should be close to the stationary distribution; and
• if in addition the markov chain is aperiodic, for a large enough period T, having these functions generate many enough sample paths the frequency distribution at T should be close to the stationary distribution.

test_mc_sample_path_lln and test_simulate_ergodicity verify these properties to hold. Any thoughts on this issue?

[oyamad]

In case init is an array and num_reps is supplied, the current implementation ignores num_reps and generates len(init) sample paths:

>>> # If init is an array, num_reps is ignored
... mc.simulate(10, init=[0, 1], num_reps=3)
array([[0, 0, 1, 1, 1, 1, 1, 1, 1, 1],
       [1, 1, 1, 1, 1, 1, 1, 1, 1, 0]])

But would it be better to generate len(init) * num_reps sample paths like the following, not ignoring num_reps?

[[0, ...],
 [1, ...],
 [0, ...],
 [1, ...],
 [0, ...],
 [1, ...]]

[mmcky]

@oyamad I see this PR has some merge conflicts thanks to the change in util references - shall I rebase?

# -Check if Numba is Available- #
<<<<<<< HEAD
from ..util import searchsorted, check_random_state, numba_installed, jit

=======
from ..external import numba_installed, jit

from ..util import searchsorted, check_random_state
>>>>>>> Import statements corrected

But would it be better to generate len(init) * num_reps sample paths like the following, not ignoring num_reps?

This seems like sensible behaviour - rather than ignoring num_reps.

[oyamad]

I did a rebase and made a forced push. (I wonder if this is a good conduct...)
I am going to change the behavior for init + num_reps.

[mmcky]

Great - thanks @oyamad. I will wait for the update in behaviour and then we can merge this PR.
I am always a bit hesitant to force anything in git - but I don't see this being a big problem when working in a branch.

[oyamad]

Now simulate works for array init and int num_reps as:

>>> mc.simulate(10, init=[0, 1], num_reps=3, random_state=1234)
array([[0, 0, 1, 1, 1, 1, 1, 1, 1, 1],
       [1, 1, 1, 1, 1, 1, 1, 1, 1, 0],
       [0, 1, 1, 1, 1, 0, 0, 1, 1, 1],
       [1, 1, 1, 1, 1, 1, 1, 0, 1, 1],
       [0, 0, 1, 1, 1, 0, 0, 0, 1, 1],
       [1, 1, 0, 1, 1, 1, 0, 1, 1, 0]])

[mmcky]

Thanks @oyamad are you happy for this to be merged?

test_mc_sample_path_lln and test_simulate_ergodicity verify these properties to hold. Any thoughts on this issue?

I think these are good additions to the tests suite.

[oyamad]

@mmcky Yes, thanks!