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:

https://github.com/econ-ark/HARK/blob/78d1981233a6cda398d4e3007be03f558a7bc910/HARK/core.py#L807-L810

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)

Most upvoted comments

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:

I want to flag this non-OO solver function as an example of a different coding pattern around 1-period solvers.

https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsPortfolioModel.py#L228-L234

I see now it’s a bit more complicated than I thought, because there’s more than one pattern for ‘one period solvers’ around.

But I think that the OO-solver functions can be consolidated into a single function. Most of these have a redundant structure. E.g:

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

is similar, except for choice of Solver object, to this:

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

which has some additional logic for the interpolater choice, that could be smoothed over, but which is otherwise almost identical to this:

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

And so on.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/econ-ark/HARK/issues/658#issuecomment-620600607, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKRAFIHQGGC4SKIPYR23RTRO3JOBANCNFSM4MQPH6XQ .