Unwind init dependencies #1529
Unwind init dependencies #1529
Conversation
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.
|
Only 8 notebooks have errors, that's pretty good! Will fix on the next commit tomorrow morning, then go back and deal with the cubic and value function issue with the "fast" models. |
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.
|
Most of the example notebook errors went away when I added two one-line methods: a "synonym" method The commit I just put in will pass all tests except the last cell or two of that one notebook. I'm going to try to repair that model, but it's a bit of a mess with its constructed inputs. |
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.
|
It was less of a mess than I feared. The main issue was just a swapped order of inputs on one call. The labeled portfolio model solver also has an unexpected need for I'm going to let the tests run (successfully), and then turn to the cubic and vFunc issue with the "fast" solvers. I suspect I did something wrong when simplifying the terminal solution code. I'll then undo the target value changes in those tests (which changed because cubic interpolation was turned off). |
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.
|
Some ConsAggShock test values have changed slightly because the MaggGrid changed when adding a constructor function. Once tests run, this PR will be ready for review. I'm not sure we want to merge it for the upcoming HARK 0.16.0 release, but instead merge it after 0.16.1 (the packaging-only release) and hold it for later. But it does need to be merged before other significant work can be done, because a lot of the model files were rearranged, even if the code didn't change. I'm going to do the work to remove |
Up to now, each
AgentTypesubclass had bespoke initialization and updating/constructing code that was run when it was called or theupdate()method was run. Because of the inheritance tree, the behavior of these methods often wound through several superclasses and was very confusing.This PR unwinds all of that bespoke code and removes (essentially) all
__init__andupdatemethods from all of our classes. Default parameters and solver (solve_one_period) are now set in the class attributedefault_as a two element dictionary that is read automatically when an AgentType is initialized. I also moved a few more things to constructors that were missed before, but there are still a (very) small number of (extremely minor) exceptions, which I will get to. On top of that, I got rid of all manipulations oftime_invandtime_varyfor solver-dependent attributes. If something should be time varying or time invariant and is used by the solver, the class says so up front.As far as I know, the only exception to the "no
__init__code" mantra is right in PerfForesight, from which everything descends. It calls a method that determines whetherRfreeis time varying, which was put in at some point when I wasn't paying attention. I really wish this change had been implemented as simply makingRfreetime-varying and throwing brackets around one number in a few dictionaries, rather than making a ton of really weird workaround code. I'm going to fix that issue, but on a different PR.As of when I am putting in this PR, all tests pass for me locally except econforge_interp tests, and that's probably because I have an outdated version of the package on my desktop. However, I am quite sure that I broke a bunch of examples with these changes, and now I'm going to deal with that, one notebook at a time.
Some of the convenience methods I removed (like
update_income_process()might get put back in as I work through this.