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

Fix macro-elasticity logic so that GDP change in year t depends on tax change in year t-1 #1579

Merged
merged 5 commits into from
Oct 13, 2017
Merged

Conversation

martinholmer
Copy link
Collaborator

@martinholmer martinholmer commented Oct 6, 2017

This pull request fixes the problem identified in the discussion of TaxBrain issue 680. The 680 discussion was very useful because several people contributed important insights: @GoFroggyRun pointed out the only nine rather than ten GDP change values are being saved in the TaxBrain database of past runs; @talumbau pointed out relevant changes in Tax-Calculator in #1314; and @Amy-Xu pointed out that the academic literature (namely the Barro-Redlick paper for which she provided a public link) estimates the change in GDP in year t using the reform-induced change in the average marginal tax rate in year t-1.

This pull request is based on my understanding of the 680 discussion, which is laid out in this comment.

The lag logic introduced in this pull request will cause Tax-Calculator to generate different GDP change estimates.

The newly introduced lag logic also implies a need to revise TaxBrain in several ways:

First, the run_nth_year_gdp_elast_model function needs to be called for all ten budget years using a loop something like this:

for year_n in range(0, 10):
    gdp_change[year_n] = run_nth_year_gdp_elast_model(year_n, start_year, ...)

Second, the numerical values for GDP change need to be shown on the Macro-Elasticity results page for all ten years (that is, the NA needs to be removed).

And third, all ten numerical values of GDP change need to be saved in the TaxBrain run database.

@MattHJensen @feenberg @andersonfrailey @hdoupe @brittainhard

P.S. This pull request consists of only a couple of commits beginning with 8ee99a3. All the commits before that one are part of pull request #1577, which is still open waiting for review and discussion.

@codecov-io
Copy link

codecov-io commented Oct 6, 2017

Codecov Report

Merging #1579 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1579   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          37      37           
  Lines        2734    2743    +9     
======================================
+ Hits         2734    2743    +9
Impacted Files Coverage Δ
taxcalc/macro_elasticity.py 100% <100%> (ø) ⬆️
taxcalc/records.py 100% <100%> (ø) ⬆️
taxcalc/calculate.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27c1b14...64de40c. Read the comment docs.

@martinholmer martinholmer merged commit 8e546f4 into PSLmodels:master Oct 13, 2017
@martinholmer martinholmer deleted the fix-gdp-logic branch October 13, 2017 14:36
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

Successfully merging this pull request may close these issues.

None yet

2 participants