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

Vensim import incorrectly translates RANDOM UNIFORM to numpy.random.rand instead of numpy.random.uniform #105

Closed
keith-citrenbaum opened this issue Oct 7, 2016 · 3 comments

Comments

@keith-citrenbaum
Copy link
Contributor

From the Vensim documentation:

RANDOM UNIFORM(m,x,s) provides a uniform distribution between m and x (exclusive of the endpoints).

RANDOM UNIFORM as of version 0.7.2 is translated to numpy.random.rand which generates as uniform distribution between 0..1 with the parameters defining the shape of the returned values.

RANDOM UNIFORM(m, x, s) should be translated to np.random.uniform(m, x), the s parameter isn't actually used and is just in the VENSIM function to maintain a common interface

@keith-citrenbaum
Copy link
Contributor Author

A quick and dirty workaround is to add a function to functions.py:

def random_uniform(m, x, s):
    return np.random.uniform(m, x)

and change the mapping in vensim2py.py:

"random uniform": "functions.random_uniform"

This seemed to work on the model that was giving me problem

@JamesPHoughton
Copy link
Collaborator

I don't think that's dirty at all. If you want to submit a PR, I'll bring it into master.

Thanks for finding this - the random functions are hard to test for because they don't give a standard output that lets us test the way the rest of the tests are run. We should think about a unit test, perhaps somewhere in here. While it won't help us find other issues with the way the random functions are implemented, it'll make sure we don't make this mistake again...

James

@JamesPHoughton
Copy link
Collaborator

As this particular case is fixed, I'm closing the issue. However, it points to a need for better testing of randomization functions, so I've opened an issue for that specifically, #107.

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