Add function to allow separate taxation of pass-through income #913

GoFroggyRun · 2016-09-12T18:34:48Z

This PR streamlines the function ptTaxer that was original being added in PR #868, and thus should yield the exact same results as PR #868 does.
Essentially, a for-loop has been used to replace some tedious code lines. In order to do so, a new array, II, is defined to collect income brackets information.
Merging conflicts have also been resolved.

Comments, concerns or remarks would be appreciated.
@MattHJensen @martinholmer

pass tests

codecov-io · 2016-09-12T19:19:14Z

Current coverage is 98.12% (diff: 100%)

Merging #913 into master will not change coverage

@@             master       #913   diff @@
==========================================
  Files            13         13          
  Lines          1818       1818          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1784       1784          
  Misses           34         34          
  Partials          0          0

Powered by Codecov. Last update ee87ccd...cb37b5a

martinholmer · 2016-09-12T20:01:55Z

@GoFroggyRun, Please remove tab characters from the current_law_policy.json file, so that the following PEP8 warnings go away:

PEP8 current_law_policy.json
current_law_policy.json:1711:1: E101 indentation contains mixed spaces and tabs
current_law_policy.json:1711:1: W191 indentation contains tabs
current_law_policy.json:1712:1: W191 indentation contains tabs
current_law_policy.json:1713:1: W191 indentation contains tabs
current_law_policy.json:1714:1: W191 indentation contains tabs
current_law_policy.json:1715:1: W191 indentation contains tabs
current_law_policy.json:1716:1: E101 indentation contains mixed spaces and tabs
current_law_policy.json:1720:1: E101 indentation contains mixed spaces and tabs
current_law_policy.json:1720:1: W191 indentation contains tabs
current_law_policy.json:1721:1: E101 indentation contains mixed spaces and tabs
current_law_policy.json:1724:1: E101 indentation contains mixed spaces and tabs
current_law_policy.json:1724:1: W191 indentation contains tabs
current_law_policy.json:1725:1: W191 indentation contains tabs
current_law_policy.json:1726:1: W191 indentation contains tabs
current_law_policy.json:1727:1: W191 indentation contains tabs
current_law_policy.json:1728:1: W191 indentation contains tabs
current_law_policy.json:1729:1: W191 indentation contains tabs
current_law_policy.json:1730:1: E101 indentation contains mixed spaces and tabs
current_law_policy.json:1734:1: E101 indentation contains mixed spaces and tabs
current_law_policy.json:1734:1: W191 indentation contains tabs
current_law_policy.json:1735:1: E101 indentation contains mixed spaces and tabs

martinholmer · 2016-09-12T20:10:12Z

@GoFroggyRun, Please remove the following two lines from taxcalc/tests/test_functions.py because they are not needed. All the tests pass without these two lines:

       if fname == 'ptTaxer':
           continue  # because ptTaxer is not really a function

And besides, the comment is incorrect: the new function is just as much of a function as the old Taxer_i function.

martinholmer · 2016-09-12T20:21:30Z

@GoFroggyRun, I don't understand the basic approach to implementing the kinds of reforms discussed in pull request #913. Why create a new ptTaxer function? Why not just generalize the existing Taxer_i to handle the possibility of pass-through income having different rates than non-pass-through income? Wouldn't that work? Then you could get rid the the error-prone _pt_special parameter? (And, by the way, what does special mean?) Can you provide some kind of rationale for the approach taken in pull request #913?

GoFroggyRun · 2016-09-13T03:05:33Z

@martinholmer, thank you very much for your thoughtful comments. I have modified the PEP8 error, as well as the test suite.

In regards to your third concenr:

Why create a new ptTaxer function? Why not just generalize the existing Taxer_i to handle the possibility of pass-through income having different rates than non-pass-through income?

Yes, I agree. We can definitely incorporate the ptTaxer function into the existing Taxer_i function. This shouldn't be a problem.

Wouldn't that work? Then you could get rid the the error-prone _pt_special parameter? (And, by the way, what does special mean?)

I'm not sure about this. Although there shouldn't be any problem to combine the ptTaxer function with Taxer_i, we will still need a switch, like _pt_special, to decide which calculation to use. Both functions yield outcome variables _xyztax and c05200, which are then both used in later calculations, sorry that I don't see an obvious way to get rid of a switch.
Of course, we could come up with a better name for it, instead of _pt_special.

cc @MattHJensen

GoFroggyRun · 2016-09-13T16:13:23Z

@martinholmer, my apologies that I am not familiar with the tax logic for pass-through tax. Your suggestion to generalize the current Taxer_i function is definitely more preferable in this situation.

I added a separate set of parameters for pass-through taxes and used the new parameters in XYZD function to calculate _xyztax and c05200.

Please take a look. Comments, concerns, or remarks would be appreciated. (Especially on descriptions for those new parameters)

martinholmer · 2016-09-13T16:39:57Z

@GoFroggyRun, The changes to current_law_policy.json look good except for one thing: the "validation" information for the _PT_brk* parameters is referencing the _II_brk* parameters instead of the _PT_brk* parameters. That needs to be fixed.

With respect to the proposed changes in the functions.py file, I'm totally confused. Perhaps you should discuss the separate taxation of pass-through income with @MattHJensen.

GoFroggyRun · 2016-09-13T20:17:00Z

@martinholmer,

I just updated the generalization of Taxer_i function to allow pass-through tax calculation. The logic should remain the same. I'm not sure why there's some puf results changes, but they seemed rounding error to me. Internet TAXSIM validation continues to pass.

cc @MattHJensen

GoFroggyRun · 2016-09-13T21:48:02Z

Thanks @MattHJensen for helping me find out a typo in functions.py, now there's no puf_agg difference.

martinholmer · 2016-09-14T02:14:15Z

@GoFroggyRun, Doesn't the validations for the following entry in current_law_policy.json need to be fixed?

    "_PT_brk6": {
        "long_name": "Pass-through income tax bracket (upper threshold) 6",
        "description": "Income from sole proprietorships, partnerships and S corporations below this threshold and above tax bracket 5 is taxed at tax rate 6.",
        "irs_ref": "Form 1040, line 40, in-line (Formside notes).",
        "notes": "",
        "start_year": 2013,
        "col_var": "MARS",
        "row_var": "FLPDYR",
        "row_label": ["2013",
                      "2014",
                      "2015",
                      "2016"],
        "cpi_inflated": true,
        "col_label": ["single", "joint", "separate", "head of household", "widow", "separate"],
        "value": [[400000, 450000, 225000, 425000, 450000, 225000],
                  [406750, 457600, 228800, 432200, 457600, 228800],
                  [413200, 464850, 232425, 439000, 464850, 232425],
                  [415050, 466950, 233475, 441000, 466950, 233475]],
        "validations": {"min": "_PT_brk5"}
    },

Shouldn't that be validations: {"min": "_PT_brk5", "max": "_PT_brk7"}?

martinholmer · 2016-09-14T02:21:00Z

@GoFroggyRun, I just don't see any reason why there should be differences in the pufcsv_mtr results. Why do you think everything is OK now?

GoFroggyRun · 2016-09-14T02:43:20Z

@martinholmer:

I have modified the validation for the 6th bracket of PT, as well as II.

I just don't see any reason why there should be differences in the pufcsv_mtr results.

Sorry that I don't see an obvious explanation either, as there supposed to be no difference at all, theoretically. But given the difference is rather minor, it seems to me that these are some rounding error.

martinholmer · 2016-09-14T11:29:31Z

@GoFroggyRun said:

Sorry that I don't see an obvious explanation either, as there supposed to be no difference at all, theoretically. But given the difference is rather minor, it seems to me that these are some rounding error.

But that was the explanation you offered when there were aggregate revenue differences, but it turned out that those differences were being caused by a coding error.

Why don't you use the inctax.py interface to Tax-Calculator to identify filing units in the puf.csv file that have income tax mtr or liability differences between master branch and your PR#913 branch?

GoFroggyRun · 2016-09-14T16:47:48Z

Thanks @martinholmer for your recent PR #916 on the MTR calculation.
After resolving the merging conflicts, there's no puf MTR change.

martinholmer · 2016-09-14T17:05:25Z

@GoFroggyRun said:

After resolving the merging conflicts, there's no puf MTR change.

So, then why is the taxcalc/tests/pufcsv_mtr_expect.txt file different from master branch?

martinholmer · 2016-09-14T17:19:18Z

@GoFroggyRun, OK this looks better after your last commit. Later this afternoon, I'll test pull request #913 on my computer, and if all goes well (as I now expect), I'll then merge #913 into the master branch. Thanks for all your work on this important addition to Tax-Calculator.

@MattHJensen @feenberg @Amy-Xu @andersonfrailey

GoFroggyRun · 2016-09-14T17:28:46Z

@martinholmer
Of course Martin. Thanks for all your thoughtful comments as well.

martinholmer · 2016-09-14T21:26:40Z

@GoFroggyRun, Your pull request #913 is incomplete because apparently you have not run all the tests: namely the reform comparison test in the taxcalc/comparison directory. Here are the diffs I get (probably because you failed to revise several of the reforms to use the new _PT_* parameters.

comparison$ git diff
diff --git a/taxcalc/comparison/reform_results.txt b/taxcalc/comparison/reform_results.txt
index a14fd0f..78d08ae 100644
--- a/taxcalc/comparison/reform_results.txt
+++ b/taxcalc/comparison/reform_results.txt
@@ -124,15 +124,15 @@ Budget Options,5,5,5,6
 REGULAR TAXES
 ""
 Increase each bracket rate by 1%
-Tax-Calculator,56.2,58.7,61.5,64.0
+Tax-Calculator,51.2,53.8,56.4,58.8
 Budget Options,56,60,65,69
 ""
 Increase top 4 rates by 1%
-Tax-Calculator,11.8,12.3,13.0,13.4
+Tax-Calculator,8.9,9.5,10.1,10.5
 Budget Options,11,12,14,15
 ""
 Increase top 2 rates by 1%
-Tax-Calculator,7.4,7.6,8.0,8.3
+Tax-Calculator,5.4,5.7,6.0,6.2
 Budget Options,7,8,9,10
 ""
 Alternative Minimum Tax
comparison$

You really do need to get in the habit of running all the tests before submitting a pull request.

@MattHJensen @Amy-Xu @andersonfrailey

GoFroggyRun · 2016-09-14T23:00:46Z

@martinholmer
Sorry. My bad Martin. Haven't been working on a PR for too long.
I have updated the parameters in reforms.json, and right now there's no change in reform comparison.

martinholmer · 2016-09-15T02:03:21Z

@GoFroggyRun, Thanks for all the work on PR#913. Now merged into the master branch.

MattHJensen · 2016-09-15T14:49:12Z

Yes, thanks a lot @GoFroggyRun and also @codykallen for getting things started and @martinholmer for review.

MattHJensen · 2016-09-15T15:03:27Z

This PR breaks tax-calculator's backwards compatibility.

@GoFroggyRun, could you please open a PR if possible or if not post an issue to www.github.com/opensourcepolicycenter/webapp-public that makes it so that TaxBrain's regular tax rate and bracket parameters modify both II and PT rt and brk parameters? This must be done before a new version of TC is deployed to TB.

Alternatively, we would need to solve #861 in some way that restores backwards compatibility.

cc @martinholmer, @talumbau, @andersonfrailey, @Amy-Xu, @rickecon, @jdebacker

Cody Kallen and others added 8 commits August 19, 2016 13:46

Modified to allow separate taxation of pass-through income

be25d72

resolve conflicts stemming from 858

0484b46

pass tests

a8bdce0

Merge pull request #1 from MattHJensen/pr-868

beaf04b

pass tests

reshape

5432265

conflicts

b0b6d14

typo

8419f53

test error clear

672fc8f

GoFroggyRun changed the title ~~[WIP]Added funciton to allow separate taxation of pass-through income~~ Added funciton to allow separate taxation of pass-through income Sep 12, 2016

martinholmer changed the title ~~Added funciton to allow separate taxation of pass-through income~~ Add function to allow separate taxation of pass-through income Sep 12, 2016

GoFroggyRun added 2 commits September 12, 2016 16:57

PEP8

8f40638

Not skipping ptTaxer function

b74a0dc

GoFroggyRun added 2 commits September 13, 2016 12:02

added separate brackets/rates for PT tax

240bd59

modified test behav

04d98d8

GoFroggyRun added 3 commits September 13, 2016 12:46

modified validation information

91edfaa

generalize taxer_i

a972b56

no validation error

2b2ba5a

typo

398122b

fixed validation for 6th PT and II bracket

b3fb19e

conflicts

f6bdc69

delimeter

77b4d80

modified reforms.json

cb37b5a

martinholmer merged commit c41c6b8 into PSLmodels:master Sep 15, 2016

MattHJensen mentioned this pull request Sep 15, 2016

Combine CG/DIV rates from AMT and Reg Tax into one set of params? #861

Closed

martinholmer mentioned this pull request Sep 15, 2016

Differential taxation of pass-through income #922

Closed

MattHJensen mentioned this pull request Sep 16, 2016

cleanup from separate PT params #927

Closed

martinholmer mentioned this pull request Sep 19, 2016

Release 0.6.7 changes #863

Closed

This was referenced Oct 18, 2016

Update B-Tax interaction with TC separation of pass-through income PSLmodels/Cost-of-Capital-Calculator#109

Closed

Update get_micro_data.py PSLmodels/OG-Core#245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add function to allow separate taxation of pass-through income #913

Add function to allow separate taxation of pass-through income #913

GoFroggyRun commented Sep 12, 2016 •

edited

Loading

codecov-io commented Sep 12, 2016 •

edited

Loading

martinholmer commented Sep 12, 2016

martinholmer commented Sep 12, 2016

martinholmer commented Sep 12, 2016

GoFroggyRun commented Sep 13, 2016

GoFroggyRun commented Sep 13, 2016

martinholmer commented Sep 13, 2016

GoFroggyRun commented Sep 13, 2016

GoFroggyRun commented Sep 13, 2016

martinholmer commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016

martinholmer commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016 •

edited

Loading

martinholmer commented Sep 15, 2016

MattHJensen commented Sep 15, 2016

MattHJensen commented Sep 15, 2016

Add function to allow separate taxation of pass-through income #913

Add function to allow separate taxation of pass-through income #913

Conversation

GoFroggyRun commented Sep 12, 2016 • edited Loading

codecov-io commented Sep 12, 2016 • edited Loading

Current coverage is 98.12% (diff: 100%)

martinholmer commented Sep 12, 2016

martinholmer commented Sep 12, 2016

martinholmer commented Sep 12, 2016

GoFroggyRun commented Sep 13, 2016

GoFroggyRun commented Sep 13, 2016

martinholmer commented Sep 13, 2016

GoFroggyRun commented Sep 13, 2016

GoFroggyRun commented Sep 13, 2016

martinholmer commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016

martinholmer commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016

martinholmer commented Sep 14, 2016

GoFroggyRun commented Sep 14, 2016 • edited Loading

martinholmer commented Sep 15, 2016

MattHJensen commented Sep 15, 2016

MattHJensen commented Sep 15, 2016

GoFroggyRun commented Sep 12, 2016 •

edited

Loading

codecov-io commented Sep 12, 2016 •

edited

Loading

GoFroggyRun commented Sep 14, 2016 •

edited

Loading