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

Split out model execution into examples/ from HARK package code #440

Closed
sbenthall opened this issue Nov 26, 2019 · 9 comments
Closed

Split out model execution into examples/ from HARK package code #440

sbenthall opened this issue Nov 26, 2019 · 9 comments
Assignees

Comments

@sbenthall
Copy link
Contributor

Related to #415

HARK is a Python package that is imported by other packages. It is in most respects structured like a typical Python package.

One way it departs from the normal way Python packages are structured is how it uses subpackages. Its subpackages, like cstwMPC, BayerLuettecke, and FashionVictim, SolvingMicroDSOPs, are currently crafted as reproductions of particular research results. Several have directories for Figures and do_all.py scripts as if they are REMARKs. Most have long __main__ methods that do display of results (in print lines or plots) in the same file as the model is built and configured.

This is not standard software architecture. There is a missing distinction here between:

  • a library, which is robustly tested reproducible code for wide use, and
  • an application, a particular piece of functionality not intended for reuse.
  • a demonstrations of the software functionality designed to guide new users.

I gather that Econ-Ark has some idiosyncratic needs. But from what I understand so far, these categories fit Econ-Ark's use cases. Roughly speaking, research publication uses of HARK are like applications. Pedagogical uses of HARK are like demonstrations. The core library code, HARK, is neither of these.

If this tracks, then it would make sense to following the conventions of other scientific software packages in separating core 'library' package code and other code that demonstrates it. A good example of this is matplotlib. That project has a top level examples directory which contains its tutorials. This is adjacent to the directory that contains its library code. If implemented and seen this pattern in other scientific software repositories I've worked with. (e.g. networkx). Dolo does this as well.

There are many benefits to this architecture. Perhaps the greatest is that it makes software developers make decisions about what functionality is supported as reusable code. Code that's in a library can be held to a rigorous testing standard and held to be independent of configuration parameters set in the examples.

@sbenthall sbenthall changed the title Split out model execution in examples/ from HARK package code Split out model execution into examples/ from HARK package code Nov 26, 2019
@sbenthall
Copy link
Contributor Author

sbenthall commented Dec 5, 2019

Application examples that need to be fixed up:

Based on this note, I will check for each example to make sure there isn't a dependency on them.

If I do find a dependency on one of these modules/examples in a different repository, I will note on that repository that its HARK dependency needs to be pegged to the last release, 0.10.2, until it can be fixed.

@sbenthall
Copy link
Contributor Author

sbenthall commented Dec 5, 2019

@sbenthall
Copy link
Contributor Author

One significant source of code dependency for the SolvingMicroDSOPs is the EstimationParameters file, which is mainly about picking exogenous calibration/parameters.

This relates to #446 -- if these parameters were available as a configuration file, as they are in Dolo, there would not be the same need for a code dependency, which is complicating the repository structure and leading to code duplication.

@sbenthall
Copy link
Contributor Author

I see now that even the HARK.ConsumptionSaving module has a lot of model execution code in it. It's in the main() methods.

I believe this is intend for the purpose of demonstrating (and testing) the code in each model. I'll move these over to examples/ as part of this task.

There are parameters for many specific models here:
https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsumerParameters.py

This module is imported by some DemARKs and REMARKs. Confusingingly, sometimes this module gets imported, but never used--for example the KrusellSmith DemARK imports it, but then ignores it, defining its parameters locally. (See #446 )

@llorracc
Copy link
Collaborator

llorracc commented Dec 8, 2019

The general pattern that we should abide by for DemARKs and parameters is to import some standard parameter configurations and then explicitly set only the parameters that either

  1. We want to highlight for expositional purposes; or
  2. We want to set to values different from their defaults.

I thought that was what had been done with KrusellSmith. If we are setting the values of ALL the parameters inside the notebook, then could you go through the notebook and remove the lines that "reset" the values of parameters to the same values that they should already have obtained by the importation of a default dictionary?

@sbenthall
Copy link
Contributor Author

I thought that was what had been done with KrusellSmith.

I see. I'll check in on that.

@sbenthall
Copy link
Contributor Author

As per this comment, the PR for this issue will remove SolvingMicroDSOPs from HARK. It will be preserved as a REMARK.

@sbenthall sbenthall self-assigned this Dec 9, 2019
@sbenthall
Copy link
Contributor Author

LifecycleModelExample should be moved from DemARK to HARK/examples

@sbenthall
Copy link
Contributor Author

Looking for dependencies on HARK.cstwMPC in other repositories:

There's one in the Krussel-Smith REMARK--but this is parameters and data information only.
https://github.com/econ-ark/REMARK/blob/4b38a288ba13069630915dc01c157e288a2200d1/REMARKs/KrusellSmith/KrusellSmith.py#L431

There are some DemARKs that reference the same file:
https://github.com/econ-ark/DemARK/blob/99948acb7b59cc9a6fb7de758972266fa4b03a06/notebooks/Micro-and-Macro-Implications-of-Very-Impatient-HHs.py#L241

https://github.com/econ-ark/DemARK/blob/7370ae005898fdd554222bcfcd927aa4850c1315/notebooks/Structural-Estimates-From-Empirical-MPCs-Fagereng-et-al.py#L50

But more significantly, the llorracc/cstwMPC REMARK imports code from HARK.cstwMPC. This is a dependency that's difficult to ignore or work around.
https://github.com/llorracc/cstwMPC/blob/54fe46264d125dfa9a36156669f077dd1751f248/Code/cstwMPC_MAIN.py#L51

Based on this comment:
#415 (comment)

...it sounds like splitting the generalizable library code from cstwMPC into HARK, and moving the paper specific work elsewhere, is a known task.

I'll consider this out of scope for this ticket. In the PR for this ticket, cstwMPC will remain as is in HARK. It can be treated separately in another issue.

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

No branches or pull requests

2 participants