search

econ-ark / HARK

Heterogenous Agents Resources & toolKit
Apache License 2.0
315 stars 195 forks source link

generate random seeds for IndexDistribution upon creation of income d… #1380

Closed sbenthall closed 3 months ago

sbenthall commented 3 months ago

fixes #1379 by generating random seeds from the AgentType's RNG when initializing the IndexDistributions for PermSfkDst and other distributions in construct_lognormal_income_process_unemployment

Please ensure your pull request adheres to the following guidelines:

alanlujan91 commented 3 months ago

@Mv77 no need to do anything but you might want to be aware of this and issue #1379 in case it affects anything you're working on

sbenthall commented 3 months ago

@alanlujan91 Could I please ask for an expedited review/merge of this PR, as it is a blocker for deadline-sensitive work?

sbenthall commented 3 months ago

The errors this PR is getting are like this:

 ImportError: cannot import name 'generated_jit' from 'numba' (/opt/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/numba/__init__.py)

Which has to do with numba dependencies which have nothing to do with this PR.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (638c5ca) 71.58% compared to head (834d408) 71.59%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1380 +/- ## ======================================= Coverage 71.58% 71.59% ======================================= Files 84 84 Lines 13929 13933 +4 ======================================= + Hits 9971 9975 +4 Misses 3958 3958 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sbenthall commented 3 months ago

I'm honestly quite confused by the RNG argument. It looks like it gets clobbered when 'reset_rng()' is called on the distribution, with the seed used instead. This is likely a legacy design and I agree it should be revisited in #1381

But the problem in this case was much dumber: it was that even with a seed set for an AgentType, a default seed (0) was being used for income distributions. This is much easier to fix.

llorracc commented 3 months ago

Seems like there should be a way to make the seed globally available. Probably would be a good idea to tack on the seed to the filename (or record it in some other way) for any output files created. And, if it's a date seed, that would make it possible to explore the history of versions of the output files by date.

mnwhite commented 3 months ago

The issue here was simply that the intended RNG structure wasn't being used. Each instance of AgentType has a single seed value, which is used for its RNG attribute (a numpy.random.RandomState instance). That RNG is supposed to be used to generate seeds for all of the distributions within an AgentType instance.

In applications/projects with only one type, the user can set that top level seed manually. For projects with multiple types, they should have a higher level seed generator, and set the seed for that manually. The principle is that for any given project, there is usually one human-controlled seed.

In this case, the AgentType-level seed generator was not setting seeds for (at least) one kind of distribution, so no matter what seed you set at the top level, income shock draws would come out the same. The only reason they were diverging in later periods (I think) is different mortality draws, which were using the intended structure.

On Wed, Feb 7, 2024, 9:25 AM Christopher Llorracc Carroll < @.***> wrote:

Seems like there should be a way to make the seed globally available. Probably would be a good idea to tack on the seed to the filename (or record it in some other way) for any output files created. And, if it's a date seed, that would make it possible to explore the history of versions of the output files by date.

— Reply to this email directly, view it on GitHub https://github.com/econ-ark/HARK/pull/1380#issuecomment-1932158769, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKRAFJ2NGIBWBGJ2JJEP73YSOFFVAVCNFSM6AAAAABC52EGGOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZSGE2TQNZWHE . You are receiving this because your review was requested.Message ID: @.***>