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

ConsKinkyPrefSolver doesn't need PrefShkPrbs #641

Closed
sbenthall opened this issue Apr 21, 2020 · 3 comments
Closed

ConsKinkyPrefSolver doesn't need PrefShkPrbs #641

sbenthall opened this issue Apr 21, 2020 · 3 comments

Comments

@sbenthall
Copy link
Contributor

ConsKinkyPrefSolver, as a carry-over from a time without DiscreteDistribution, breaks a distribution into its probabilities and values before using them to construct a utility function:

https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsPrefShockModel.py#L566-L567

https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsPrefShockModel.py#L389-L390

This can be made more compact

@sbenthall
Copy link
Contributor Author

I see. This coding style is quite widely used throughout HARK, not just in ConsPrefShockModel.

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

I suppose that what happened was that because Distributions were once quite hard to understand (because they were just lists or arrays), it was helpful to copy their values to a more descriptively named one temporarily.

As we start to encapsulate distributions in objects, this should be less and less necessary.

Note #623 -- we could use a more descriptive name for the values of a distribution than X.

Note also that Dolo uses an xarray, a data structure that allows for named columns and rows:

EconForge/dolo.py#193

@sbenthall
Copy link
Contributor Author

Depends on #730 now

@sbenthall
Copy link
Contributor Author

This is a duplicate of #625 -- this all has to due with the somewhat convoluted expectation-taking code in the solutions, which is slated to be replaced with calcExpectations. See #896 for progress towards this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant