Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Replace continuous distribution 'draw_' methods with dolo IID processes #611

Closed
sbenthall opened this issue Apr 6, 2020 · 6 comments
Closed

Comments

@sbenthall
Copy link
Contributor

HARK currently has several functions in simulation for drawing from continuous distributions:

  • drawMeanOneLognormal
  • drawLognormal
  • drawNormal
  • drawWeibull
  • drawUniform
  • drawBernoulli

This is custom code that has been implemented in many other libraries.
In particular, we can move towards integration with Dolo by replacing this code with calls to the 'simulate()` method in the Dolo IIDProcess class, e.g.:
https://github.com/EconForge/dolo/blob/master/dolo/numeric/processes_iid.py#L114

There are some complications here:

  • It's possible that the HARK functions and Dolo do not have feature parity. A few observations:

    • HARK draw functions have:
      • an exact_match option -- I frankly don't understand what this does yet
      • the ability to set a seed for the random generator
    • Dolo simulate has:
      • a T argument. What is this?
      • a stochastic argument. What is this?
    • It looks like Dolo does not yet have simulate implemented for the LogNormal distribution, and has no class for the Weibull distribution
  • The automated tests for HARK depend on the current draw_ code, including the random seed setting. Swapping in Dolo will require recalibration of all the tests. This will make it difficult to guarantee that the new code does not introduce any errors.

  • This would introduce a direct dependency between HARK and Dolo. This is the direction it sounds like @llorracc wants to go. However, from an open-source development perspective, Dolo does not currently have any documented contribution guidelines. Does it have a release cycle or stable releases? It's not clear how this dependency will effect HARK's development process.

  • The Dolo IIDProcess classes are for the most part not documented. Unless new documentation is written for Dolo, this change will make HARK less easy to understand. Dolo documentation for IID processes is ticketed here: No documentation in Processes classes EconForge/dolo.py#183

[This is related to #519, which is about discrete distributions. Currently in HARK, continuous distributions are handled entirely separately, in simulation.py, from discrete distributions, which at the time of this writing are in utilities.py (changing with #610). Dolo IIDProcesses support discretization, so this change is instrumental to syncing up these functionalities.]

@sbenthall sbenthall added this to the 1.0.0 milestone Apr 6, 2020
@sbenthall
Copy link
Contributor Author

While doing a full replacement with dolo IIDProcesses would be major change--maybe one more appropriate for 2.0 Dolark integration, a more moderate change would be to the necessary refactoring to start mirroring Dolo.

I.e., to make a ContinuousDistribution class with a draw method, and subclass it for each particular distribution.

This sort of move would move HARK towards equivalence with Dolo, which would become apparent in the DARKolos.

@llorracc
Copy link
Collaborator

llorracc commented Apr 8, 2020

That's an excellent idea. Go for it.

@sbenthall
Copy link
Contributor Author

Interesting: SymPy has stats support: https://docs.sympy.org/latest/modules/stats.html

@sbenthall
Copy link
Contributor Author

@sbenthall sbenthall modified the milestones: 1.0.0, 2.x.y Jun 17, 2020
@sbenthall sbenthall modified the milestones: 2.x.y, 1.0.0 Jun 25, 2020
@sbenthall
Copy link
Contributor Author

Based on @albop 's presentation, it sounds like we want to replace the HARK Distribution classes with dolo Distribution classes

@sbenthall
Copy link
Contributor Author

Now there is even further lack of feature parity between HARK distributions and dolo IID processes, because of the new calcExpectations functionality #625 The closest equivalent in Dolo is 'integrate, but this does not meet all of the requirements @mnwhite put into place with calcExpectations`

https://github.com/EconForge/dolo.py/blob/b9e7018d46e48c05bc3d54bc1dc01bc77f3f6415/dolo/numeric/distribution.py#L78

Given the time available for delivering 1.0, I will move this ticket to a later milestone. Full HARK/Dolo integration is an admirable goal, but it will be much easier to do after HARK is further refactored into elements that are similar to Dolo's.

@sbenthall sbenthall modified the milestones: 1.0.0, 1.x.y Jan 13, 2021
@sbenthall sbenthall modified the milestones: 1.x.y, 1.1.0 Jan 23, 2021
@econ-ark econ-ark locked and limited conversation to collaborators Jun 3, 2024
@sbenthall sbenthall converted this issue into discussion #1443 Jun 3, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

2 participants