-
-
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
Pass the RNG stored in AgentType to portfolio agents' methods for dra… #433
Conversation
…wing risky returns rates.
I will prioritize looking at this. Ok, I just did look at this. You
should have it grab a seed from self.RNG, as with the other shock
generators in AgentType. I think the method is to do something like
int(np.floor(2**31*self.RNG.rand())) as the seed. It's clunky, but it
makes sure that the same seed is passed to all of the RNGs on each run of
the code, as long as self.reset() is called (that's why it appears in
AgentType.initializeSim()).
Man, this sure would be a lot easier on Patrick if I had written a
documentation notebook about our simulation structure. I mean, what's up
with that??
…On Sat, Nov 9, 2019 at 4:12 PM Patrick Kofod Mogensen < ***@***.***> wrote:
…wing risky returns rates.
Fixes #430 <#430>
We should of course verify that a) this works with the different examples
I provided in the original PR and make it clear that b) you now have to
define your function to accept 1 positional argument that represents a RNG
state.
@llorracc <https://github.com/llorracc> @mnwhite
<https://github.com/mnwhite>
------------------------------
You can view, comment on, or merge this pull request online at:
#433
Commit Summary
- Pass the RNG stored in AgentType to portfolio agents' methods for
drawing risky returns rates.
File Changes
- *M* HARK/ConsumptionSaving/ConsPortfolioModel.py
<https://github.com/econ-ark/HARK/pull/433/files#diff-0> (4)
Patch Links:
- https://github.com/econ-ark/HARK/pull/433.patch
- https://github.com/econ-ark/HARK/pull/433.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#433?email_source=notifications&email_token=ADKRAFI6MUPO6UFJYHSYO3TQS4RURA5CNFSM4JLJS4EKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HYGNP6A>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKRAFJCRCO4GLB73ESMTW3QS4RURANCNFSM4JLJS4EA>
.
|
Oh, of course. So RNG is the actual generator, and the idea is to use that to generate a seed for each simulation. So if the RNG is reset, you get the same sequence next time around. Can you have a look again? Thanks! |
Is this ready for merging now? |
Yes. I haven't run this branch, but if it works for Patrick after making these changes, it's ready for merge. His RNG is now formatted the same as our other shock-drawers. |
Can someone write a release note for this? |
Release note: Fixed bug in ConsPortfolioModel in which the same risky rate of return would be drawn over and over |
…wing risky returns rates.
Fixes #430
We should of course verify that a) this works with the different examples I provided in the original PR and make it clear that b) you now have to define your function to accept 1 positional argument that represents a RNG state.
@llorracc @mnwhite