Add test of TaxInc function

[jdebacker]

This PR adds tests for the calcfunctions.TaxInc function.

cc @MattHJensen @bodiyang

[jdebacker]

In the initial tests added, I test for records. For each of these, I worked through Form 8995 by hand. For the first record, I get a QBID=0, whereas taxcalc returns $70,400. For the other records, my worksheet calculations match taxcalc.

For the first record, taxable income before the QBID is $490,860.66, which is above the threshold for full phase out of QBID (which, with the parameters used in the test, is $421,400). Thus, I think the QBID should be fully phased out for this individual.

In calcfunctions.py, this full phase out is not caught in the if/else statements when PT_qbid_limit_switch=True and PT_SSTB_income=0 (i.e., one doesn't reach line 1266 if this is the case).

[MattHJensen]

@jdebacker, could you confirm which tax form you used, 8895 or 8895-A? I believe the first record should use 8895-A, and there is not a hard phaseout for deducting non-SSTB (aka, qualified) business income.

(Also relevant here: https://www.propublica.org/article/secret-irs-files-reveal-how-much-the-ultrawealthy-gained-by-shaping-trumps-big-beautiful-tax-cut)

[bodiyang]

@MattHJensen The first record is calculated based on form 8895-A. This attached excel document include the steps of calculation:
Form8995Calculations_with_Calcfunctions.xlsx

[MattHJensen]

@bodiyang, thanks for sharing the spreadsheet.

So, this taxpayer is in the bottom left hand corner of this diagram (not an SSTB; income above phasein), and they have zero w2 wages or property (& PT_qbid_limit_switch is turned on). That should leave the taxpayer with no QBID, but instead Tax-Calculator is giving the full 20% of income.

Is that how others are interpreting the situation?

[bodiyang]

@MattHJensen Yes, exactly. This diagram shows the interpretation of this taxpayer's record: not an SSTB, income above phase in, zero W2 wages & property.

[jdebacker]

@MattHJensen Yes, I agree with your assessment.

Nice diagram!

[jdebacker]

I guess I missed the in-line comment:

# if PT_qbid_limit_switch is False, assume all taxpayers
# have sufficient wage expenses and capital income to avoid
# QBID limitations.

If that is the assumption, then the $70,400 qbided is correct for the first record in this test (i.e., the intention is to ignore the zero wages and UBIA amounts in this case).

I guess the question is -- is Taxsim making the same calculation? This record is showing a difference in total tax liability between the two models.

[codecov]

Codecov Report

Merging #2618 (dd5e3a0) into master (9774b47) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2618   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files          14       14           
  Lines        2609     2609           
=======================================
  Hits         2571     2571           
  Misses         38       38           

Flag	Coverage Δ
unittests	98.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[feenberg]

If you will give me the taxsim32 input variables, we will figure out what is going on in taxsim tomorrow. daniel feenberg

…

On Tue, 24 Aug 2021, Jason DeBacker wrote: I guess I missed the in-line comment: # if PT_qbid_limit_switch is False, assume all taxpayers # have sufficient wage expenses and capital income to avoid # QBID limitations. If that is the assumption, then the $70,400 qbided is correct for the first record in this test (i.e., the intention is to ignore the zero wages and UBIA amounts in this case). I guess the question is -- is Taxsim making the same calculation? This record is showing a difference in total tax liability between the two models. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS orAndroid.[AB55AVNCXWEKWAB6N4UE56TT6RDIBA5CNFSM5CK7Q5P2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJ KTDN5WW2ZLOORPWSZGOGXZML4Q.gif]

[bodiyang]

@feenberg This excel document is the input variables for taxsim32 of this taxpayer's record, whose taxable income before the QBID is $490,860.66
record.xlsx

[jdebacker]

I'm now marking this PR as ready for review. While it doesn't test all the logic of calcfunctions.TaxInc, it does test the QBID logic and finds that the current code is correct.

I think we can merge this PR and then extend the tests of calcfunctions.TaxInc to cover other parts of the function.

cc @MattHJensen

[martinholmer]

In PR #2618, @jdebacker said:

I'm now marking this PR as ready for review. While it doesn't test all the logic of calcfunctions.TaxInc, it does test the QBID logic and finds that the current code is correct.

A conclusion that "the current code is correct" would be true only if the test in this PR is strong: that is, explores all the factors that determine the QBID amount. A much stronger test is to compare QBID amounts for many randomly-generated tax units and confirm that Tax-Calculator produces the same QBID amounts as TAXSIM35 or OpenFisca-US.

Have you concluded that the Tax-Calculator QBID logic is correct because you have determined that the current version of Tax-Calculator no longer generates the kind of large differences that I reported in this comment (which was part of the PR #2453 discussion you closed months ago)?

If there are no differences now, you should report those results to the public so that Tax-Calculator users have confidence in the model's QBID results.

@MattHJensen @MaxGhenis

[jdebacker]

@martinholmer Thanks for the comments.

A conclusion that "the current code is correct" would be true only if the test in this PR is strong: that is, explores all the factors that determine the QBID amount.

Martin, context is important here. I'm not referring to all the source code in this repo, rather the logic that determines QBID under the four scenarios tested. That's the scope of this PR (I note that there is a need to test other aspects of this function). And of course the statement is conditional on my hand calculations being correct. We need someone to review those before this PR is merged. Would you like to review this PR to help in that process?

A much stronger test is to compare QBID amounts for many randomly-generated tax units and confirm that Tax-Calculator produces the same QBID amounts as TAXSIM35 or OpenFisca-US.

That's a different test. As you know, good practice with unit testing is to test the smallest units of testable code. Unfortunately, this has been lacking from Tax-Calculator, where there did not exist unit tests of the important individual functions in calcfunctions.py until Dec 2020. This PR is part of the attempt to remedy that. Validation tests are also important, but rely on the results from many different functions. Unit tests help identify the specific source of errors by setting out tests of those individual functions.

Have you concluded that the Tax-Calculator QBID logic is correct because you have determined that the current version of Tax-Calculator no longer generates the kind of large differences that I reported in #2453 (comment) (which was part of the PR #2453 discussion you closed months ago)?

No. My suggestion that this is correct is because the calculations for the tested cases using Tax-Calculator match my calculations when working through IRS Form 8995 and 8995-A by hand with those same inputs. The validation discussion you referenced is ongoing, but has been moved to PR #2619 because the branch in PR #2453 has not been active since Jacob stopped contributing to the repo in April 2021.