Skip to content
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

IndShockConsumerType.getShocks() samples the shocks twice #494

Closed
sbenthall opened this issue Feb 8, 2020 · 5 comments
Closed

IndShockConsumerType.getShocks() samples the shocks twice #494

sbenthall opened this issue Feb 8, 2020 · 5 comments

Comments

@sbenthall
Copy link
Contributor

sbenthall commented Feb 8, 2020

It looks like the IndShockConsumerType.getShocks() methods samples the exogenous shocks for newborn agents twice:

  1. first using one method, then throwing out the result
  2. then again with a different, approximate method

https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L2094-L2119

Doing unnecessary and unused computation is bad for performance. I.e., this slows down the simulation and solution runtime.

@llorracc
Copy link
Collaborator

llorracc commented Feb 8, 2020 via email

@sbenthall
Copy link
Contributor Author

I've linked to the code; I'm just reading "approximate" from the comments.
I defer to @mnwhite on the interpretation.

@mnwhite
Copy link
Contributor

mnwhite commented Feb 8, 2020 via email

@sbenthall
Copy link
Contributor Author

Oh, I see. I misunderstood the code. My original issue is incorrect.

In any case, thanks for explaining this to me. I wouldn't have figured it out without you going into it.

Calculating the shocks for newborns twice, while a much more minor issue, is still less than ideal.
I think a little refactoring here could both improve performance slightly, as well as make the code a little more readable.

If you don't mind, I'd like to keep this ticket open to flag this minor issue. I hope to get to it once I have a clearer idea of the simulation logic.

@sbenthall
Copy link
Contributor Author

#499

@MridulS MridulS closed this as completed Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants