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

Updatepresolve #323

Merged
merged 2 commits into from
Jun 21, 2019
Merged

Updatepresolve #323

merged 2 commits into from
Jun 21, 2019

Conversation

pkofod
Copy link
Contributor

@pkofod pkofod commented Jun 20, 2019

Fixes #222 at the expense of a few more calculations each time solve is called. Advanced users can easily create a MyIndShkType and overwrite preSolve with a new 3-line method that doesn't update between solve() calls. The cost is probably negligible anyway, compared to solving the model and any post-processing you might do after solving a model.

@mnwhite
Copy link
Contributor

mnwhite commented Jun 20, 2019 via email

@pkofod
Copy link
Contributor Author

pkofod commented Jun 20, 2019

This has been one of the long-demanded items (as a one line fix). Gentle intro notebook should be updated to reflect this change. That's part of my notebook overhaul.

Just verifying: you agree that this should be done and it should be done like this, right?

@mnwhite
Copy link
Contributor

mnwhite commented Jun 20, 2019 via email

@pkofod
Copy link
Contributor Author

pkofod commented Jun 20, 2019

Great. Merge when you're ready :)

@llorracc llorracc added Type: Enhancement Tag: 1.0 About the v1 release of HARK. Tag: Discussion Tag: New Tool Expertise: Prob and Stats Needs ready familiarity with advanced undergraduate multivariate probability and statistics. labels Jun 21, 2019
@llorracc
Copy link
Collaborator

Ultimately, we will want to define a distribution object which "stores" the parameters needed to produce its approximation (like, that it is a lognormal with mean 0.02 and standard deviation of 0.1 produced by the equiprobable method of approximation using seven nodes). That way, the current parameters can be compared to the stored one, and the distribution computed only if it has changed. A quicker but uglier fix would be just to have the agent object store this info (instead of having a separate distribution object for it).

@albop

@llorracc llorracc merged commit f910fac into econ-ark:master Jun 21, 2019
@pkofod
Copy link
Contributor Author

pkofod commented Jun 21, 2019

Ultimately, we will want to define a distribution object which "stores" the parameters needed to produce its approximation (like, that it is a lognormal with mean 0.02 and standard deviation of 0.1 produced by the equiprobable method of approximation using seven nodes). That way, the current parameters can be compared to the stored one, and the distribution computed only if it has changed. A quicker but uglier fix would be just to have the agent object store this info (instead of having a separate distribution object for it).

I agree that it could be done to make this slightly more efficient, but I'm wondering if the extra complexity is really worth it? I have strong doubts that re-calculating these things will be significant in the overall picture. We can try a specific implementation at some point to see if the extra complexity is worth it.

@pkofod pkofod deleted the updatepresolve branch June 21, 2019 19:11
@shaunagm
Copy link
Contributor

@pkofod Can you write a release note? Thanks.

@pkofod
Copy link
Contributor Author

pkofod commented Jun 28, 2019

Uhm yes. Something like.

"Added a call to updateIncomeProcess() in preSolve() to avoid solutions being based on wrong income process specifications if some parameters change between two solve() calls."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Expertise: Prob and Stats Needs ready familiarity with advanced undergraduate multivariate probability and statistics. Tag: Discussion Tag: New Tool Tag: 1.0 About the v1 release of HARK. Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After income process parameters change, warn user to recompute .updateIncomeProcess()
4 participants