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

The DCEGM machinery is susceptible of improvements #1062

Closed
Mv77 opened this issue Sep 4, 2021 · 7 comments
Closed

The DCEGM machinery is susceptible of improvements #1062

Mv77 opened this issue Sep 4, 2021 · 7 comments
Milestone

Comments

@Mv77
Copy link
Contributor

Mv77 commented Sep 4, 2021

I have been working with the DCEGM tools for a project of mine. They are usable but I believe that they are susceptible of substantial improvements. I will list some here for my own reference and for enthusiasts looking to contribute:

1. The current tools for taking upper envelopes rely on re-interpolating functions to an exogenous "common m grid"
See: https://github.com/econ-ark/HARK/blame/master/HARK/dcegm.py#L246
Suppose I have two pairs of (m, V(m)) [(m1, v1), (m2, v2)] of which I want to take the upper envelope. What the DCEGM paper suggests that I do is to pick the pairs from these two existing grids that belong in the upper envelope. What HARK currently does is interpolate both groups of points on an exogenously given commonM grid and then take commonV = max(v1(commonM), v2(commonM)).
This is okay for a first pass, and would work well for a fine enough commonM grid. However, it is not optimal, since it requires the user to specify a new grid and discards whatever convenient properties are in the m1 and m2 grids. This is important if these come from EGM, which automatically makes gridpoints denser in places where there is more action. Under the commonM approach, this information is lost.

@project-bot project-bot bot added this to Needs Triage in Issues & PRs Sep 4, 2021
@llorracc
Copy link
Collaborator

llorracc commented Sep 4, 2021

This is probably a pretty important point for computational efficiency, for the density-of-grids-where-you-need-it reasons.

@Mv77
Copy link
Contributor Author

Mv77 commented Sep 4, 2021

2.The code makes assumptions about the properties of the commonM grid, without clearly telling the user
See, for instance: https://github.com/econ-ark/HARK/blame/master/HARK/dcegm.py#L353

Those lines seem to assume that the lowest point of the commonM grid is 0, and therefore assign consumption[0] = 0 and value[0] = -Infinity. This is assumes:

  • The user gave us a grid with commonM[0] = 0.
  • Market resources of 0 imply a consumption of 0. Therefore there is no borrowing or consumption floors.
  • A consumption of 0 gives infinitely negative utility.

But the user is not told these assumptions are being made. These are often right for the type of problem we usually deal with, but I believe that it would be convenient to have a tool for taking upper envelopes that assumes as little as possible (give me a list of (x,y) grids and I'll give you their upper envelope) and then let the user (or particular model's solvers) do with those envelopes what he wants.

@Mv77 Mv77 changed the title The DCEGM machinery is succeptible of improvements The DCEGM machinery is susceptible of improvements Sep 4, 2021
@Mv77
Copy link
Contributor Author

Mv77 commented Sep 4, 2021

3. The upper envelope tool extrapolates
See: https://github.com/econ-ark/HARK/blame/master/HARK/dcegm.py#L359

Since there is no guarantee about the degree of overlap between the set of original grids and the commonM grid that the user has to specify, extrapolation can occur.

@Mv77
Copy link
Contributor Author

Mv77 commented Sep 18, 2021

2.The code makes assumptions about the properties of the commonM grid, without clearly telling the user

Here is another instance, in calc_segments, the function that is supposed to take a line and split it into its increasing segments.

HARK/HARK/dcegm.py

Lines 150 to 152 in f59ffdd

# NOTE: assumes that the first segment is in fact increasing (forced in EGM
# by augmentation with the constrained segment).
# elements in common grid g

I guess the note IS telling me what is being assumed, but the behavior is unexpected. The code assumes that the (x,y) pairs its being fed come from applying egm and implementing a very specific way of dealing with the liquidity constrained segment. Turns out I was dealing with it in a different way and after some painful debugging time, landed here.

@Mv77
Copy link
Contributor Author

Mv77 commented Nov 12, 2021

I have a private (too messy to be published) version with improvements.

I'm planning to tidy it up at some point between now and the end of the year and make a PR with it. Just a heads up that there is some progress on this.

@Mv77 Mv77 mentioned this issue Jan 9, 2022
@Mv77
Copy link
Contributor Author

Mv77 commented Feb 9, 2022

Fixed by #1100

@Mv77 Mv77 closed this as completed Feb 9, 2022
Issues & PRs automation moved this from Needs Triage to Done Feb 9, 2022
@llorracc
Copy link
Collaborator

llorracc commented Feb 9, 2022

I'm delighted to see this is now merged -- just in time, since I believe @AMonninger will need to use it for the replication exercise he wants to do for the HA-macro course this semester.

@sbenthall sbenthall added this to the 0.13.0 milestone Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Issues & PRs
  
Done
Development

No branches or pull requests

3 participants