HARK: solveOnePeriod is not a method, but sure looks like one
There is a strange coding pattern in the Model classes. I am tempted to try to turn it into something more standard, but want to check if there is a hidden purpose to it.
Typically, a ConsumerType will have an attribute, solveOnePeriod, that is set upon initialization to some stand-alone function in the module. E.g.:
https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L1658
This stand alone method then typically initalizes a Solver object, runs solve() on it, then returns the result (a solution). E.g.:
https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L590-L625
These methods are for the most part, except for the choice of Solver class, identical across the library.
This is not ideal because it makes it difficult to read the code: to follow the solution logic, you have to jump out of one class into the global scope, then into another.
It seems to me that either:
- (A) solveOnePeriod could be a method, which would just look like exactly the function that is currently assigned to it, or
- (B) solveOnePeriod could be a standardized method, which points to a particular Solver class, which is then parameterized in the model code.
The only motivation I can think of for the current implementation is that it’s suggested by this code block that the idea was to make it possible to have time-varying solution functions:
However, this flexibility does not seem to be used anywhere in the HARK repository (I checked), nor does it have any tests associated with it. If this was still a desirable feature, it would be possible to build ‘solveOnePeriod’, as a standardized method, to optionally take a list of class names that could be indexed by time periods.
[Separately, and maybe to be addressed in a different issue: I’m skeptical about the performance of this method of firing up a new Solver object, complete with a lot of initialization, at every step of the backwards inductions. It seems to me there may be a lot of redundant code calls. I’ll revisit this once I’ve got a profiler set up, but I wanted to mention it now to start circulating the thought.]
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 29 (24 by maintainers)
I would like ConsIndShockSolverSetup, ConsIndShockSolverBasic, and ConsIndShockSolver to be collapsed into one class. I lost that fight 4.5 years ago.
On Tue, Apr 28, 2020 at 9:17 AM Sebastian Benthall notifications@github.com wrote: