-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
Policy parameter update with 2018 values #2212
Conversation
@Peter-Metz, Thanks for all the work that is included in your pull request #2212. But as you can see from this that there are number of procedural problems. The biggest procedural problem is that your development work branched off from the master branch seventeen days ago just after PR#2185 was merged into the master branch. (You can see this in the Network time-line graph on GitHub.) Meanwhile there have been several important revisions to the master branch (including updating the projections in PR#2196 and updating the OASDI maximum taxable earnings parameter in PR#2203). This means, among other things, that your pull request undoes all the work I did with the Your development branch (which is being submitted as PR#2212 for incorporation into the PSLmodels/Tax-Calculator master branch) is 7 commits ahead (those are your changes on your The solution is to merge the most recent version of the PSLmodels/Tax-Calculator master branch into your |
@martinholmer, thank you for the explanation. The latest commit should fix the merge conflicts. The conflicts were results of the numbers generated by |
@Peter-Metz said:
Because you were using the old projections as I explained earlier in this comment. |
Ah, understood. Thanks. |
@Peter-Metz, I'm curious about why you changed some 2017 policy parameter values as part of your 2018 parameter update work in PR #2212. For example, you changed (slightly) the values for the And here's the second ( Also, the GitHub diff for the |
@Peter-Metz, Now that PR #2211 has been merged into the PSLmodels / Tax-Calculator master branch, you need to do two things on your computer in the following order:
|
Thanks for these instructions.
This was intentional, but on second thought, I'm not sure I was correct in doing this. For example, I changed 2017,
First, I changed 2018 "widow" from $500,000 to $600,000 because I believe there was a typo. In terms of the values from 2019-2026, that looks like a difference stemming from |
@Peter-Metz said in PR #2212:
I agree, so please do these things with the EITC parameters.
Which of the IRS forms for which you provide links shows the widow amount at $600,000? Assuming you have caught a mistake (good work!), the pull request values for _II_brk6 for 2018+ seem just fine. No need to do anything except verify on the forms that $600,000 is the correct number for widow. |
@Peter-Metz, After fixing the issues discussed in this comment, you need to do one more thing before addressing the test failures. Change in |
@martinholmer said incorrectly in #2212:
Actually, the values currently in #2212 for What we want in this comparison is the 500000-to-600000 change for widow in 2018, and changes in 2019+ only for the widow value. |
This can be found in the 1040 Instructions, Schedule Y-1, page 113.
Will do.
Thanks, I came to the same conclusion while running the tests and have since fixed this locally by re-running
I have managed to make the necessary changes to pass most of the tests, but would appreciate some guidance on one failure in particular. The PR fails the function
If possible, could you please explain why those four parameters ( |
@Peter-Metz, Sounds like you're making great progress and even learning to ignore my advice when its bad advice. The test that is puzzling you has to do with the fact that in the past a few parameter values were not (as far as we could tell) available in advance documents and had to be updated only after the IRS forms were published. Hence a few had one less year label. But in the update you're doing we have all the 2018 values and so there is no need to treat a few parameters differently. If you set the |
Thanks for the explanation. Indeed, when I set the Another issue came up as I was going through test failures: Let me know if you think this observation warrants its own issue and/or PR. |
@Peter-Metz said:
Try removing the 2019 year and its value from the |
@Peter-Metz said in #2212:
Good catch! I agree that this issue should dealt with separately from this pull request. But we have to make some kind of change in order to get the tests to pass for pull request #2212 , right? |
@Peter-Metz, After removing the 2019 year and value for the |
@Peter-Metz, Actually things are not a bad as they seem with respect to the limitation of itemized deductions of state/local taxes. The two parameter you mentioned seem not to be used in current (post-TCJA) law. The parameter doing the limiting seems to be this one:
Can you check the code in the |
Codecov Report
@@ Coverage Diff @@
## master #2212 +/- ##
======================================
Coverage 100% 100%
======================================
Files 12 12
Lines 2957 2957
======================================
Hits 2957 2957
Continue to review full report at Codecov.
|
With the new commits, the PR should pass all tests.
For now, I set 2018 values for both To get one of the tests to pass, I had to add a number of 2018 parameter values to |
@Peter-Metz, What about this comment? Did you not read it? |
@martinholmer, I agree with you assessment in this comment -- that
Still, we need to populate 2018 values for |
@Peter-Metz said in PR #2212:
You mean in order to pass the tests?
Yes, just put their 2018 values at 9e99 (essentially infinity). I agree with your intuition, but making these changes and rerunning the tests will provide the evidence that either confirms or rejects our intuition. |
@Peter-Metz, After doing this don't forget to |
@martinholmer, thanks for the reminder. I have updated |
@Peter-Metz, Thank your for your contribution in pull request #2212. This was a big project and you have done a very good job, not only in #2212 but also by focusing us on the need for changes in the area of child/dependent credits. Your observations on this later topic in issue #2197 have lead to merged pull requests #2205 and #2211, and to the parameter-renaming discussion in issue #2215 that will lead to another pull request. But apparently you didn't run the |
This PR uses the process described in Issue #2183 to update policy parameter values in
policy_current_law.json
to reflect IRS final forms for tax year 2018. After updating final 2018 parameter values, I ranppp.py
to calculate parameter values through 2026. Where the IRS forms changed in layout (e.g. Form 1040), I updated the"irs_ref"
value to point users to the correct line in the 2018 forms.The IRS forms and instructions used to make these updates can be found below:
Form 1040 and instructions, Form 1040 (Schedule 1), Form 1040 (Schedule 2), Form 1040 (Schedule 4), Form 1040 (Schedule A) and instructions
Form W-2 and instructions
Form 6251 and instructions
Form 8863 and instructions
Form 8812 and instructions
Addendum (added by @martinholmer on 2019-02-01):
Using actual 2018 parameter values as the base for indexing (rather than using the actual 2017 values as before this pull request), produces more accurate tax results for years beginning with 2018. The net results of this parameter updating are modest because many important actual 2018 parameter values were already known from the TCJA legislation. For example, when using the PUF input data the 2021 income tax liability declines from $1764.7 billion to $1764.6 billion, which is a decline of $0.1 billion or about six one-thousandths of one percent, and the 2021 payroll tax liability rises from $1322.9 billion to $1324.6 billion, which is a rise of $1.7 billion or about one-tenth of one percent.
Addendum (added by @martinholmer on 2019-02-06):
A couple of the EITC parameters updated in this pull request were close approximations of the exact values. The exact values have been added in pull request #2220.