Remove update_Rfree() #1530
Open
Remove update_Rfree() #1530
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tried to restructure AgentType.__init__ to have universal features, but it seems to have broken immediately. I'm getting a strange message that solve_one_period_ConsPF is getting multiple values for solution_next, which shouldn't be possible.
Problem on prior commit was very small: setting a function as a class attribute behaves a little strangely if you reference it later. Had to put solver functions into a class dictionary.
Also improved usage of constructors. One Krusell-Smith test value changed slightly.
Also caught unexpected issue with construct(): it broke when there were no constructors at all!
The default dictionary for this model might need some work.
Also found some incomplete work in ConsRiskyAssetModel. Note that the risky asset labeled models were *untested* before these changes, and there's no guarantee they work after them. There's some weird conflicts between the function that makes the shock distributions "labeled" and the function that combines income and return shocks into a single object.
Did some reformatting in the unit tests, and couldn't manage to get the non-basic solver to work. Ended up turning off vFuncBool and/or CubicBool to make it work. I suspect it's something I did with the terminal period solution. Will take a look on later commits.
The update_income_process() method has been removed, but it seems like maybe I should bring it back so as not to break a bunch of examples. We'll see.
Added a synonym method for construct so that old calls to update still work. Also added update_income_process as a one liner because it's used so much. Will see how many fewer notebooks break.
I wrote a one line method and still managed to do it wrong.
Only four notebooks needed formatting changes after my initialization overhaul; it was minimal work. One notebook is outstanding, because it provides an example of the RiskyAssetLabeled model, which was untested. Looks like I need to fix that.
The order of inputs on one function was unexpected, and the labeled portfolio model solver for some reason *requires* the existence of RiskyShareFixed even though it couldn't possible be used. Last notebook runs now, tests should pass.
As suspected: I mistakenly threw out important terminal solution code. All tests pass.
Undo some prior changes that are now unnecessary.
If there's anything else that can be converted to a constructor, I don't know about it.
For a while now, there was a bit of code in PerfForesight that checked whether Rfree was a singleton or a list, and then set things up for that. This created a lot of work for something that had a very small solution: just put brackets around Rfree in parameter dictionaries. This commit removes that method and fixes all of the dictionaries in the code. All tests pass, but I assume some notebooks will fail. There might also be some simulation code to simplify.
A lot of notebooks needed very small patches to make them work. I *think* I fixed them all.
|
It looks like all tests and examples will run after the latest commit-- I somehow missed one notebook on my first pass. This PR will be marked as Ready for Review, but it should still NOT BE MERGED until the parent PR is merged. That will make the changes here more readily apparent. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR builds off of #1529 and SHOULD NOT be merged until that branch has been merged. Most of the changes in this branch (relative to master) are from that branch.
At some point a while back, someone wanted
Rfreeto be time-varying in their model. Probably in an attempt to not make other people change their parameter dictionaries, they added some custom code that allowsRfreeto be time-varying or time-invariant, and have that be detected automatically. And then wrote some more code to make sure that's respected in a bunch of places. This was polite, but it also made a lot of unnecessarily complicated code; the alternative solution is just to put brackets aroundRfreewhenever it's passed as a parameter (or make it a constant list).This PR implements that alternative solution. As of this first commit, all tests pass, but some notebooks are probably broken. I'll fix those tomorrow morning.