Remove ltcg from ETI substitution effect base

[MattHJensen]

Resolves #36.

• needs documentation update to explain divergence from ETI literature.

[codecov-io]

Codecov Report

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

@@          Coverage Diff          @@
##           master    #37   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines         102    103    +1     
=====================================
+ Hits          102    103    +1

Impacted Files	Coverage Δ
behresp/behavior.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 38507ca...de7549f. Read the comment docs.

[MattHJensen]

This is ready for review.

[martinholmer]

@MattHJensen said in Behavioral-Responses issue #36:

If one were to apply both a non-zero substitution elasticity and a non-zero long-term capital gains elasticity, does it mean that we are double counting some behavior?

And if so, would a reasonable solution be to remove LTCG from the substitution effect by subtracting it from c04800 prior to the calculation of the substitution effect?

I know very little about the ETI research literature, but there seem to me a disconnect between what you said in #36 and the code change you propose in #37. Your statement above suggests that the double-counting occurs only when assuming "a non-zero substitution elasticity and a non-zero long-term capital gains elasticity." And by proposing #37 you have answered your first question with a yes.

So, why not revise the code to subtract LTCG out of the taxable income measure used to compute the substitution effect only when there is a non-zero LTCG elasticity. Why deviate from the ETI research literature in situations where there is no double-counting?

[MattHJensen]

I know very little about the ETI research literature, but there seem to me a disconnect between what you said in #36 and the code change you propose in #37. Your statement above suggests that the double-counting occurs only when assuming "a non-zero substitution elasticity and a non-zero long-term capital gains elasticity." And by proposing #37 you have answered your first question with a yes.

So, why not revise the code to subtract LTCG out of the taxable income measure used to compute the substitution effect only when there is a non-zero LTCG elasticity. Why deviate from the ETI research literature in situations where there is no double-counting?

You're right to spot the disconnect between this PR and my concern about double counting in #36, it probably would have been helpful for me to flesh out that issue with more background.

There was no overriding factor that caused me to remove the ETI from the substitution effect base even when the ltcg response is zero, but a few factors, together, pushed me over the edge:

1. It is generally confusing for the definition of a parameter to depend on the state of other parameters.
2. I often think of Behavioral-Responses as a tool to predict what the official estimators (JCT/Treasury/CBO) will say about a reform, and as far as I know, all of them use a separate CG elasticity.
3. I was in the middle of analyzing a proposal where the pure ETI-literature-derived substitution effect was producing counterintuitive results, aside from the double counting. Consider a tax unit with only long term capital gains and no wage income. Congress hikes the MTR wrt wage income, without changing the MTR wrt CG income. If CG is in the base for the substitution effect calculation, that tax unit will see a big decrease in taxable income.
4. Seeing that the LTCG elasticity is set at 0 by default, some users might assume that means there is no LTCG-related behavioral effect when the substitution effect parameter is positive.

[martinholmer]

@MattHJensen, Thanks for the extensive explanation of the thinking behind B-R PR #37. All this makes sense, so I have no objections to merging this change. Although if none of the tests failed, it would suggest adding one that would have failed would be a good idea. I guess we need a test that assumes both the substitution and LTCG elasticities are non-zero.

[MattHJensen]

Although if none of the tests failed, it would suggest adding one that would have failed would be a good idea. I guess we need a test that assumes both the substitution and LTCG elasticities are non-zero.

Yea, this makes sense. I'll add one to the PR.

[martinholmer]

@MattHJensen, Why does commit 828d1d3 add the return of a Calculator object?
This is a major change in the Behavioral-Responses API that you are proposing without offering any rationale.

[MattHJensen]

@MattHJensen, Why does commit 828d1d3 add the return of a Calculator object?
This is a major change in the Behavioral-Responses API that you are proposing without offering any rationale.

I am still working on this PR. I'll start a discussion of the changes when they're ready for review. In the meantime, I'll add back the WIP tag to show that I'm in progress on the testing.

[MattHJensen]

This PR is ready for review.

In addition to addressing #36 and adding a test that fails under master but passes on this branch, the PR makes a substantial change to the behresp API by returning the calc2 with behavior calculator from response().

Here is my thinking for returning calc2_beh:

• In order to conduct the original exploratory analysis to determine that Should CG income be included in the base for calculating ETI substitution effect? #36 was a problem, I needed to look at how behresp was acting on individual records, and so I was working with a modified fork of Behavioral-Responses that returned the calculator.
• It was taking me a long time to figure out how to write a failing test for this problem w/o being able to look at the microdata, but it is easy with the microdata and I was able to do so quickly.
• We have seen many users take advantage of the tc dump capabilities. Without returning the calculator, I don't know how users could do similar work with Behavioral-Responses.
• I have seen another user modify a fork of Behavioral-Responses to return the calculator object in order to try to figure out what is going on in the microdata.

I do not know what kind of performance benchmarks Behavioral-Responses is trying to hit or the reason for those benchmarks, so I have not yet considered performance in these changes. One simple approach to alleviate some performance concerns -- which we may also want to adopt in the absence of performance concerns -- might be to have response() only return the calc2 w/behavior calculator and add a new function to only return the two distribution table dataframes.

[martinholmer]

I've moved the discussion of extra-response-data needs to issue #38 because it is logically separate from resolving issue #36, which is the focus of pull request #37. @andersonfrailey and @MattHJensen, can you please describe the details of your experiences in issue #38?

[martinholmer]

@MattHJensen, it would seem that pull request #37 contains far more than a resolution of issue #36.
I suggest that issue #38 --- the need for more response data --- be resolved in a different pull request after the discussion in #38 is finished. Meanwhile, can you refocus #37 on resolving #36? If you do that, I'll be happy to merge it because there would be no API change in #37.

[MattHJensen]

Meanwhile, can you refocus #37 on resolving #36? If you do that, I'll be happy to merge it because there would be no API change in #37.

@martinholmer, I was unable to devise an adequate test for #36 (by adequate, I mean one that fails on master) without returning extra data. I am happy to remove the testing for now and open an issue that reminds us to add tests after #38 is resolved. Is that what you are suggesting?

[martinholmer]

@MattHJensen said:

I was unable to devise an adequate test for #36 (by adequate, I mean one that fails on master) without returning extra data. I am happy to remove the testing for now and open an issue for adding tests after #38 is resolved. Is that what you are suggesting?

I don't understand. The core change in #37 causes a change (for some people) in the high-level data now returned from the response function, right? I can understand that finding one of those "some people" could have been made easier for you if the response function had the option of returning more data. But once you've found such a person, why does the new test need the API change?

[MattHJensen]

I was unable to devise an adequate test for #36 (by adequate, I mean one that fails on master) without returning extra data. I am happy to remove the testing for now and open an issue that reminds us to add tests after #38 is resolved. Is that what you are suggesting?

I retract this. The tested PR will be ready for review soon.

[MattHJensen]

This PR is ready for review.

[martinholmer]

@MattHJensen, Maybe I'm distracted by all the things going on with the move today, but I don't understand the new test.

First, I don't understand the role of the new behresp/tests/rec_only_ltcg_inc.csv file. I couldn't seem to find where it's used. Here's the file I'm talking about:

Second, I don't understand this new behresp/tests/rec_w_wo_ltcg.csv file:

How can a single individual (MARS=1) have positive spouse earnings (e00200s=500000)?
Should I add data checking to Tax-Calculator so that it stops when people inadvertently enter data like this?

[martinholmer]

@MattHJensen, Thanks for all your work on Behavioral-Responses pull request #37. All the coding-style and substantive tests ran fine on my computer. Sorry the limited output from the response function made your work more difficult, but we're discussing how best to remedy this situation in issue #38.

[MattHJensen]

@martinholmer, thanks a lot for reviewing/shepherding/merging this PR.