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

Replace StageIFactors.csv with growfactors.csv and simplify logic #1178

Merged
merged 6 commits into from
Feb 7, 2017
Merged

Replace StageIFactors.csv with growfactors.csv and simplify logic #1178

merged 6 commits into from
Feb 7, 2017

Conversation

martinholmer
Copy link
Collaborator

@martinholmer martinholmer commented Feb 3, 2017

This pull request simplifies Tax-Calculator logic by moving data-preparation work from Tax-Calculator code (where is executes every time Tax-Calculator is run) to the taxdata code (where it is executed just once). The forthcoming change in taxdata logic is described in taxdata issue 60. The desirability of moving this data-preparation code into the taxdata repo (where it logically belongs) was identified in issue #1175.

These changes cause minor differences in some tax results (even with the new grow_factors.csv file containing rates that are expressed with six decimal places of precision). For example, in 2022 income taxes are now $2033.2 billion, which is up from $2032.7 billion, a rise of $0.5 billion, which is an increase of about one-fourth of one-tenth of a percent. There is no change in the payroll tax total for any year in the 2013-2022 period.

@MattHJensen @feenberg @Amy-Xu @andersonfrailey @GoFroggyRun @codykallen

@codecov-io
Copy link

codecov-io commented Feb 3, 2017

Codecov Report

Merging #1178 into master will not impact coverage by -0.02%.

@@            Coverage Diff             @@
##           master    #1178      +/-   ##
==========================================
- Coverage   98.88%   98.87%   -0.02%     
==========================================
  Files          38       38              
  Lines        3063     3023      -40     
==========================================
- Hits         3029     2989      -40     
  Misses         34       34
Impacted Files Coverage Δ
taxcalc/records.py 95.39% <100%> (-0.72%)

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 f021633...2ab749f. Read the comment docs.

@MattHJensen
Copy link
Contributor

MattHJensen commented Feb 6, 2017

These changes cause minor differences in some tax results (even with the new grow_factors.csv file containing rates that are expressed with six decimal places of precision). For example, in 2022 income taxes are now $2033.2 billion, which is up from $2032.7 billion, a rise of $0.5 billion, which is an increase of about one-fourth of one-tenth of a percent. There is no change in the payroll tax total for any year in the 2013-2022 period.

@martinholmer, why does this change influence results?

@martinholmer
Copy link
Collaborator Author

martinholmer commented Feb 6, 2017

@martinholmer said:

These changes cause minor differences in some tax results (even with the new grow_factors.csv file containing rates that are expressed with six decimal places of precision). For example, in 2022 income taxes are now $2033.2 billion, which is up from $2032.7 billion, a rise of $0.5 billion, which is an increase of about one-fourth of one-tenth of a percent. There is no change in the payroll tax total for any year in the 2013-2022 period.

Then @MattHJensen asked:

Why does this change influence results?

First, the results changes are very small: one-fourth of one-tenth of a percent is tiny.
Second, the results changes are caused by the fact that the factors (which will be generated once in the taxdata repository --- see taxdata issue 60) are rounded to six decimal digits of precision (rather than the 14 significant digits currently being held in memory using the logic on the master branch).

I created an alternative version of the factor CSV file in which the factors have nine decimal digits of precision. That file is bigger: 3913 bytes rather than 3017 bytes when factors have six decimal digits of precision. But the extra precision has no effect on income tax or payroll tax liability (because they are rounded to the nearest one-tenth of a billion dollars). The extra precision does cause changes in some intermediate income tax results. For example, the AMT liability in 2022 is $57.8 billion, which is the same as on master and down from $57.9 billion in this pull request. Another example: adding three extra decimal points of precision causes taxable income to rise by $0.2 billion, which is a change of one-fifth of one-tenth of a percent.

Does that information answer your question?

@MattHJensen
Copy link
Contributor

Does that information answer your question?

Yes, thanks a lot. +1 on the change.

@martinholmer
Copy link
Collaborator Author

@MattHJensen, @andersonfrailey and I are working together on some changes to the taxdata repo that we want to put in place before we commit pull request #1178.

@martinholmer martinholmer changed the title Replace StageIFactors.csv with puf_factors.csv and simplify logic Replace StageIFactors.csv with grow_factors.csv and simplify logic Feb 7, 2017
@martinholmer martinholmer changed the title Replace StageIFactors.csv with grow_factors.csv and simplify logic Replace StageIFactors.csv with growfactors.csv and simplify logic Feb 7, 2017
@martinholmer
Copy link
Collaborator Author

@martinholmer said:

@andersonfrailey and I are working together on some changes to the taxdata repo that we want to put in place before we commit pull request #1178.

We have made enough progress in that taxdata work to make it sensible to merge #1178 now.

@MattHJensen

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

4 participants