Use a python class for discrete probability distributions

[sbenthall]

This PR addressed issue #519

This is only a refactor of the current HARK code. It does not yet pull in any new dolo dependencies.

Rather, it refactors the discrete distribution-related code into a new module, distribution, containing a class, DiscreteDistribution, with attributes pmf and X.

This should be familiar enough to the current users of HARK.
The principle benefit of this is that it:

• begins to compartmentalize this functionality
• makes the code a little easier to understand because one type is used unambiguously for distributions, instead of a generic type (list of arrays).

Tests are passing locally for me.

I'd like to ask to expedite this PR because my understanding is that @llorracc is doing high-priority work on seeding models with ergodic distributions.
This change, though it touches a lot of code, is meant to be a minimal restructure to make it easier to:

• build a mixture of Guassian representation
• eventually swap in Dolo, which would allow smoother generation of discrete distributions from continuous ones.

[MridulS]

Any ideas why is this here?
(I know this isn't this PR change)

[sbenthall]

Are you referring to the @memoize decorator?
I assumed it was for performance reasons.

[mnwhite]

Yeah. Memoize is there so that in a problem with a large T_cycle but identical permanent or transitory shock distributions, the function doesn't get re-computed dozens of hundreds of times.

…

-- mnw

On Sun, Apr 5, 2020, 7:25 PM Sebastian Benthall ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In HARK/distribution.py <#610 (comment)>: > + for i in range(K): + zBot = cutoffs[i] + zTop = cutoffs[i+1] + tempBot = (mu+sigma**2-np.log(zBot))/(np.sqrt(2)*sigma) + tempTop = (mu+sigma**2-np.log(zTop))/(np.sqrt(2)*sigma) + if tempBot <= 4: + X[i] = -0.5*np.exp(mu+(sigma**2)*0.5)*(erf(tempTop) - erf(tempBot))/pmf[i] + else: + X[i] = -0.5*np.exp(mu+(sigma**2)*0.5)*(erfc(tempBot) - erfc(tempTop))/pmf[i] + + else: + pmf = np.ones(N)/N + X = np.exp(mu)*np.ones(N) + return DiscreteDistribution(pmf, X) + ***@***.*** Are you referring to the @memoize <https://github.com/memoize> decorator? I assumed it was for performance reasons. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#610 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADKRAFOJ4IKZGTWOXMZ2OKDRLEHQNANCNFSM4L7ZRRKA> .

[sbenthall]

Please merge #616 before this PR
It is expected that the new test there will fail, since ConsRepAgentModel has not yet been included in this refactor

[sbenthall]

I believe this is now ready to merge

[llorracc]

Aside from my one minor bug (the junk file that needs to be deleted), this looks great to me!

One point that should be addressed separately is that the IncomeDstn variable should really be renamed IncShkDsnt -- it is NOT the distribution of income, it is the distribution of income SHOCKS. I think there's an issue about this somewhere long ago (if not, there should be).

I do NOT think that change should be made as part of this refactor. Instead, it should be its own self-contained PR. We should at the same time construct corresponding PR's for DemARK and REMARK repos if there are any notebooks therein that directly reference IncomeDstn.

[sbenthall]

junk is removed in latest commit.

The issue you refer to is #227 I think

[llorracc]

Yes, #227 is right