-
-
Notifications
You must be signed in to change notification settings - Fork 199
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
functional simulation tests to enable external distribution classes #1105
Comments
Though it's not entirely clear from the documentation, I've tested it and this https://docs.scipy.org/doc/scipy/reference/generated/scipy.stats.rv_discrete.html |
I've spent a little time in the last couple of days looking into this and learned several things that are relevant for our purposes. A key one that illustrates the reason I want to outsource our construction of distributions as much as possible is the recent announcement of this toolkit which provides tools for constructing distributions that are adapted to super fast simulation with TensorFlow Markov Chain Monte Carlo tools. It would be totally unsurprising if someone developed a corresponding tool for PyTorch or other generic tools for doing these kinds of simulations. Ideally, we could construct an architecture that would make it reasonably straightforward for people to choose Another interesting new development I've just come across is a new toolkit that is laser-focused on describing bellman problems, which we should take a very close look at as we think about our future direction. |
An example of the kind of test that needs to be changed: And example of the kind of test it should be changed into: |
I have no objection to ADDITIONAL tests like https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/tests/test_ConsPortfolioFrameModel.py#L153-L156 And if the reason you think the first test should be eliminated is that it tests the outcome for a specific agent (at index 1), I'm on board. Instead, what we should be testing is things like whether the average level of assets in simulations matches some raw number like 0.184 (or whatever the correct answer is). Any time a change in the HARK code causes our simulations to produce significantly different quantitative answers, we need to be alerted to that because we know that our current code produces the right answers and any substantial change in those answers means the HARK change introduced a conceptual error somewhere (even if accounting identities remain satisfied). |
Yes, the simulation tests I'm referring to are those that test a specific value for a specific agent. We also have tests for the solution objects, which should not break with a change in how distributions are sampled. I.e. that the consumption function, for some amount of market resources, is 0.184. So these can go untouched. I don't recall if we have any tests for the approximate equality of a simulated population mean to a fixed number. I would be wary of such tests, because the sensitivity of the equality approximation would need to be calibrated to the confidence interval of the result somehow to be effective if the only change was to distribution sampling. |
Any such tests should be low precision ones. If an aggregate variable changed from 3.14 to 3.15 or 3.13 we would not worry. But if it changed to 6.8 or something, that would be a clear sign of a bug. |
As of the meeting today, we decided:
|
I'm taking responsibility for this issue, but am a little stuck. One thing we could do is remove any tests for particular values that come out of a simulation's results. In many cases, this would leave simulations as entirely untested except to show that they executed without error. I wonder if there's anything else that comes to mind for what would be a good simulation test. |
Continuing from this discussion:
#1104 (comment)
I believe that @llorracc is keen on replacing as much of the distribution modeling functionality in HARK with an external library as soon as possible.
A major obstacle to doing this is that the tests for model simulations currently mostly check for near equality with specific numerical values, as opposed to testing for a functional relationship between a model's state value and prior relevant state values.
Fixing this is just a matter of labor time.
This would make it much easier to work on #611 #949
The text was updated successfully, but these errors were encountered: