-
Notifications
You must be signed in to change notification settings - Fork 90
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
Test Random Functions #107
Comments
Some tests have been added for the supported random functions. We may need to add tests for the different moments and ranges of the distributions to ensure they are being generated correctly. |
Could you add exponential function? My colleagues use it often for lead time and informed me to plead for adding this function :) ![]() Following
I tried
|
Hi @hyunjimoon We were using I could add it, but it will be used at your own risk if you don't mind. We are missing random function tests (that was the purpose of this issue). Ideally, we should have a bunch of data generated by different configurations of the random functions in Vensim. Then, compare with our implementation different order moments (like mean, variance, skewness, and kurtosis) and minimum, maximum, and some percentiles to check that the distribution behaves the same. I currently have no time nor access to Vensim to develop that, so I cannot ensure that it will behave the same. If you want to test this approach, this is what it should be added in pysd.builders.python.python_functions, note that the first value is ignored as it has no implementation for this function: "random_exponential": (
"stats.truncexpon.rvs(%(1)s, loc=%(2)s, scale=%(3)s, size=%(size)s)",
("scipy", "stats")), |
Sounds great - although my collaborator (he told me to pay gratitude to you and @JamesPHoughton for this amazing library) will be the one who'll test, I'll make sure to share whether it works, and if not the error log in this thread. |
Hello everyone? It's amazing that such great work is being done here. I'm the one who asked to add that function. Thank you for responding quickly. Since I am a modeler, I do not have the ability to modify Python or Vensim functions. But RANDOM EXPONENTIAL(m,x,h,r,s) in the vensim app provides an exponential distribution starting at 0 with a mean of 1 before being stretched, shifted and truncated. So loc=h and scale=r would be used. The problem is the value of 'b', and this value should be set to 5.3, which is 99.5% of the exponential distribution with a mean of 1. Nevertheless, it is difficult to say that the two functions are exactly the same. The following picture is the result of comparing the random numbers generated by vensim by parameter. I hope this helps you convert the function. And I hope this function will be included in the next released version. |
Hi @parksehoon1971, I could try to add try to add the function. However, I will be doing it in my free time, so I will need a little collaboration from you. I can develop some tests to compare random distributions., so we can also test the existing ones and potentially add others. I will also add a small toolkit to plot the results so we can visually compare them if something is not working as expected. The deal is that I will implement the testing tool, and I will ask you for data. We may need to generate a very big bunch of data to test it, and I do not have access to Vensim anymore. If you agree, I will send you some configurations to run (I need to check before which number of points is reasonable so we have strong tests, but they don't take too long). I will ask you to generate data in the same way you did in the example, a TAB file with the output per column from Vensim may work, but will be nice to have in the first row the function that was called (e.g. |
I started working on the test at https://github.com/SDXorg/pysd/tree/random_functions |
Thank you your efforts! |
Hi, @parksehoon1971 I have been doing some other tests. I guess the better approach is to compare both with Vensim but using a bigger tolerance and then with expected values calculated analytically so we can test with less error. In this process, I have discovered that the current implementation of I will ask you to produce the following files with the first column uniform_vensim.tab
normal_vensim.tab
exponential_vensim.tab
If you have doubts maybe is easier to talk via slack, you can join the workspace we have sdtoolsandmet-slj3251.slack.com Files should be something like:
|
I'm sorry for the delay in communication due to the time difference. I have finished what you asked me to do. Please test using the attached file. thanks! |
Hi, @parksehoon1971, as soon as the tests pass, I will create the new release. Hope it helps. Just remember that if you publish anything using PySD you can always cite our paper |
@enekomartinmartinez, Thank you so much for updating the library so quickly. This update removed one obstacle in implementing the web app we were trying to develop. I hope you are always happy. |
@enekomartinmartinez, |
try using numpy.random.seed before running the model, this should set the seed for all the calls to random functions. |
Yes, thank you for helping me solve the random number problem. |
While running the model with PySD, something strange occurred. There was a difference between the data executed by Random Normal() in Vensim and the data executed in PySD. In particular, there is a large difference between the upper and lower limits. mean daily demand = 100 So I created only one variable and generated a random number, and there was no problem. I've attached the file with the problem, so if you have the time, I hope you can solve this problem. Thank you for always! |
It would be good to do an integration test of all of the random functions we support:
The challenge is that our testing infrastructure matches values at each timestep, and that won't work with random functions.
Lack of this sort of testing leads to issues like #105.
The text was updated successfully, but these errors were encountered: