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

BUGs in random_choice #393

Closed
oyamad opened this issue Mar 8, 2018 · 5 comments
Closed

BUGs in random_choice #393

oyamad opened this issue Mar 8, 2018 · 5 comments

Comments

@oyamad
Copy link
Member

oyamad commented Mar 8, 2018

I tried the recently merged random_choice #391 and found the following:

  1. This doesn't work with Numba v.0.36 with the following error:

    TypeError: Failed at nopython (inline calls to locally defined closures)
    sequence item 0: expected str instance, NoneType found
    

    It works with Numba v.0.37, so this should be a bug in Numba v.0.36. The problem for us is that the latest Anaconda contains Numba v.0.36.2.

  2. Setting a seed with np.random.seed within a library (hidden from the user) is problematic: it seems to affect Numba's global random state.

    Suppose you have the following jit function:

    @numba.jit(nopython=True)
    def my_rand(size):
        return np.random.uniform(0, 1, size)

    Call it after calling random_choice with the same seed (with Numba v.0.37):

    qe.random_choice(np.array([0.1, 0.9]), seed=0)
    my_rand(5)

    Then it always returns the same sequence:

    array([0.71518937, 0.60276338, 0.54488318, 0.4236548 , 0.64589411])

@jstac What is your use case where you need a jitted version of this functionality? What's wrong with generating random numbers outside your jit function and then using np.searchsorted (or qe.searchsorted) inside the jit function?

@jstac
Copy link
Contributor

jstac commented Mar 8, 2018

@oyamad I need to draw one variate from a fixed discrete distribution until a stopping condition is satisfied, in a loop. It needs to be fast. I don't know the size of the sequence ex ante.

@oyamad
Copy link
Member Author

oyamad commented Mar 8, 2018

@jstac I see.

Would it be possible to set a seed in your own function, removing np.random.seed from quantecon?

@jstac
Copy link
Contributor

jstac commented Mar 8, 2018

@oyamad Yes, that's certainly possible.

Regarding the numba version, thanks for picking that up. Version 0.36.2 has many problems.

So, the proposal is

  1. drop the ability to set seeds in these functions (apart from externally, as you suggest)
  2. replace None with 0 as the default argument for cum_sum

@oyamad
Copy link
Member Author

oyamad commented Mar 9, 2018

@jstac Here's my proposal:

>>> cdf = np.cumsum([0.4, 0.6])
>>> qe.random.draw(cdf)
1
>>> qe.random.draw(cdf, 1)
array([1])
>>> qe.random.draw(cdf, 10)
array([1, 0, 1, 1, 0, 1, 1, 1, 1, 1])

Works with both Numba 0.36.2 and 0.37.0.

@jstac
Copy link
Contributor

jstac commented Mar 10, 2018

@oyamad This is beautiful. Thank you. I learned a lot from reading the code.

I fully support this. Would you mind to make a PR?

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

No branches or pull requests

2 participants