Need to rename child/dependent credit policy parameters

[martinholmer]

Current-law policy regarding child and dependent tax credits changed substantially beginning in 2018. The policy parameter names currently being used in Tax-Calculator are based on the notion that pre-TCJA policy is current law, and hence, a number of parameter names are confusing to people who look at the 2018 IRS forms. The need to rename some parameters was first identified by @Peter-Metz in issue #2197. The point of this issue is to begin a discussion about how to rename the parameters so they are less confusing. This discussion should also include the labeling and grouping of policy parameters on the TaxBrain webapp.

In order to get the discussion going, I'm going to propose a set of policy parameter name changes.

• _CTC_c --> _CTC_c : but redefined to be the maximum nonrefundable child credit allowed (so, for example, the 2018 value of 1400 would be changed to 2000).

• _DependentCredit_Child_c --> _ACTC_c : the maximum refundable child credit allowed (so, for example, the 2018 value of 600 would be changed to 1400) and Tax-Calculator would report an error if _ACTC_c were ever specified to be larger than _CTC_c.

• _DependentCredit_Nonchild_c --> _ODC_c : the maximum nonrefundable other-dependent credit allowed.

In addition to these name changes, it seems as if the _DependentCredit_before_CTC parameter is now redundant and can be removed.

Also, following IRS acronyms, the calculated variable dep_credit should probably be changed to odc (Other Dependent Credit).

The TaxBrain labeling/grouping of these parameters also needs review. Currently TaxBrain groups credits into nonrefundable and refundable, which is effective most of the time. But it is confusing in the case of the child and dependent credits. Why not group the CTC, ODC, and ACTC parameters all together under the heading "Child/Dependent Credits"?

@MattHJensen @hdoupe

[martinholmer]

Does the lack of comments on issue #2215 mean that everybody is OK with the proposed changes?

@MattHJensen @hdoupe @Peter-Metz

[MattHJensen]

Does the lack of comments on issue #2215 mean that everybody is OK with the proposed changes?

Not that I will necessarily suggest anything different, but I would appreciate a few more days to think about this.

Also cc @codykallen in case he is interested.

[codykallen]

The proposed name changes and elimination of _DependentCredit_before_CTC seem fine to me. Would it be possible to add warning statements somewhere in the Policy object that the _DependentCredit_* variables have been deprecated, to be raised if someone passes a reform dictionary that includes the relevant rename/eliminated variables?

Also, if we make this change, how should we alter the reform JSONs that used them?

[martinholmer]

@codykallen said in issue #2215:

The proposed name changes and elimination of _DependentCredit_before_CTC seem fine to me.
Would it be possible to add warning statements somewhere in the Policy object that [say] the _DependentCredit_* variables have been deprecated, to be raised if someone passes a reform dictionary that includes the relevant renamed/eliminated variables?

I'm going to look into how Tax-Calculator handles a JSON reform file that includes a policy parameter that is unknown to Tax-Calculator. If we need to improve the handling of such a situation that will be done as part of the pull request that resolve this issue. But the response will not be a warning, but an error. Using an unknown policy parameter is a show-stopping event for Tax-Calculator.

[martinholmer]

@codykallen said in issue #2215:

Also, if we make this change [in policy parameter names], how should we alter the reform JSONs that used them?

My view is that the existing JSON reform files in the taxcalc/reforms directory fall into two groups:

1. Some that should be revised to work with current-law as the baseline policy (like 2017_law.json, BrownKhanna.json, and perhaps others), and
2. Others that should be moved without any changes to a new taxcalc/reforms/archive directory (like RyanBrady.json) because they are no longer relevant to the current tax policy discussion.

For example, the RyanBrady.json file contains a link to a description of the reform that no longer exists given that Ryan is no long House Speaker, no longer a member of the House of Representatives, and (as far as I know) no longer a politician.

Then over the next few years, we will be able to add new JSON reform files to the taxcalc/reforms directory as politicians make serious proposals for tax reform.

[martinholmer]

In answer to @codykallen's question about what now happens when Tax-Calculator encounters a JSON reform file containing an unknown policy parameter, consider the following Python script in the unknown_param.py file:

from taxcalc import *
refyear = 2020
ref = {refyear: {'_bad_param': [7000]}}
pol = Policy()
pol.implement_reform(ref)

Here is what happens when the script is executed:

$ python unknown_param.py
Traceback (most recent call last):
  File "unknown_param.py", line 5, in <module>
    pol.implement_reform(ref)
  File "/Users/mrh/work/PSL/Tax-Calculator/taxcalc/policy.py", line 191, in implement_reform
    raise ValueError(self.parameter_errors)
ValueError: ERROR: 2020 _bad_param unknown parameter name

I suppose I could add "is an" so it reads ... "is an unknown parameter name", but the error message is reasonably clear and stops program execution as it should.

@MattHJensen @hdoupe

[codykallen]

@martinholmer, I agree that it's fine to stop given a bad parameter. My thought is more along the lines of changing the error message. For example, in [policy.py, in _validate_parameter_names_types()],

Tax-Calculator/taxcalc/policy.py

Lines 441 to 442 in 3e71828

	if pname not in param_names:
	msg = '{} {} unknown parameter name'

This uses the error unknown parameter name. Could we instead do something like:

if pname not in param_names:
    if pname in deprecated_param_names:
        msg = '{} {} deprecated parameter name'
    else:
        msg = '{} {} unknown parameter name'

[martinholmer]

@codykallen, Your idea in this comment is good. I'll make that change in the pull request that resolves issue #2215.

[MattHJensen]

I talked about this issue today on the phone with @martinholmer, and after the conversation, these changes make sense to me.

I had been wondering whether there was a way to prevent the loss of backwards compatibility brought on by redefining parameter names, particularly _CTC_c and _ACTC_c. But the child tax credit and additional child tax credit have been redefined in the IRS forms, and the only way to maintain backwards compatibility -- that I can see -- would be to deprecate the param names _CTC_c and _ACTC_c and replace them with new names that conform less closely to our parameter naming conventions.

What's more, as Martin pointed out, it is too much to expect us to be able to maintain backwards compatibility of a tax simulation model when the underlying tax system changes in structural (as opposed to parametric) ways. If TCJA had happened after Tax-Calculator was at version 1.x, for example, the new tax law, with its structural changes to the code, would have been plenty of reason to bump the version of Tax-Calculator to 2.0.

Given that we will be breaking backwards compatibility in a way that doesn't announce itself when the code is run, it will be necessary to use our communications infrastructure (changelogs, what's new, PSL twitter, PSL newsletter, etc.) to let users know that the parameters behave differently.

Thanks to @codykallen for looking at this issue already, and copying in @Peter-Metz for relevance to PSL comms. Also copying @andersonfrailey and @hdoupe because the discussion has relevance to the backwards compatibility of TaxBrain runs (it might not be much of an issue this time since we are archiving ospc.org/taxbrain runs and users won't be able to re-compute past reforms, but it would definitely be an issue otherwise, and in future analogous situations we might need to notify users that a run can no longer be re-simulated because some of its parameters have been redefined or deprecated.)

[hdoupe]

TaxBrain already has this built in. When users try to view reforms with parameters that used to be valid but are not in the current set of Tax-Calculator parameters, then a warning message is displayed indicating that some parameters used in the simulation are no loner available:

http://apps.ospc.org/taxbrain/edit/20000/?start_year=2017

One solution for this would be to maintain a list of deprecated parameters. Then, when the user tries to use a parameter in that list, a helpful error message could be given. You could even keep a JSON file with all deprecated parameters as keys and the values being a JSON object with related parameters and/or a PR where that parameter was removed:

{
    "_DependentCredit_Child_c": {"related_parameters": ["_ACTC_c"], "pr_removed": 2215}
}

[martinholmer]

Merging pull request #2223 resolves issue #2215.

The recent comment by @MattHJensen is a nice statement of the releasing procedures we could follow in the future. But as I thought about his comment, it seems to me as if the future is right now. We are breaking backwards compatibility in two ways with the next release: (1) the Behavior class has been removed and hence the structure of JSON assumption files has changed, and (2) five policy parameters have been removed and importantly one, _CTC_c, has been redefined. Given all this, shouldn't the next Tax-Calculator release be 1.0.0 (rather than the previously planned 0.25.0)?

@feenberg @andersonfrailey @hdoupe @codykallen @Peter-Metz

[MattHJensen]

@hdoupe, thanks very much for the information on how TaxBrain handles deprecated parameters. It seems like there may be some wrinkles for parameters that have been redefined but not deprecated, like _CTC_c, for instance. Do you have any suggestions for how to handle those?

[hdoupe]

Sure thing. Also, I just read the entire thread and saw that @codykallen recommended the same thing that I did several comments before I chimed in. Good idea Cody!

From TaxBrain's point of view, I think it's up to Tax-Calculator to provide error messages that differentiate between deprecated parameters and redefined parameters, if that's what they want to do. As a Tax-Calculator user, I'd appreciate those types of error messages or a document containing parameters that no longer exist, why they were removed (deprecated/redefined), related parameters, and relevant issues/pr's. GitHub's issue tracking and pull request comments functionality can be really nice when you are trying to figure out why some decision was made. However, it turns you into a bit of a historian who's going over primary source material. Users may appreciate a clean, concise table that tells them why their reform from X months ago no longer works.

[martinholmer]

The policy parameter renaming was done in pull request #2223 and a warning message about the redefinition of the _CTC_c policy parameter was added in pull request #2236.