[Question] How does the round trip test work?

[hdoupe]

I'm trying to understand the test_round_trip_tcja_reform test:

Tax-Calculator/taxcalc/tests/test_reforms.py

Lines 93 to 104 in 9c8be55

	pol = Policy()
	reform_file = os.path.join(tests_path, '..', 'reforms', '2017_law.json')
	with open(reform_file, 'r') as rfile:
	rtext = rfile.read()
	pol.implement_reform(Policy.read_json_reform(rtext))
	assert not pol.parameter_warnings
	assert not pol.errors
	reform_file = os.path.join(tests_path, '..', 'reforms', 'TCJA.json')
	with open(reform_file, 'r') as rfile:
	rtext = rfile.read()
	pol.implement_reform(Policy.read_json_reform(rtext))

My understanding is that it:

1. implements 2017_law.json
2. implements TCJA.json
3. checks that the parameters are back to current law (i.e. equal to policy_current_law.json)

My question is: Why are some parameters changed in 2017_law.json, but not in TCJA.json? For example, EITC_ps_MarriedJ is changed in 2017_law.json, but EITC_ps_MarriedJ is not included in TCJA.json. So, how it could it get back to its current law value if the value is updated via the 2017_law.json file but isn't changed again by the TCJA.json file?

There are a couple other parameters like this such as II_em_ps and EITC_c.

There's a hack in the parameters code that exists specifically to get this one test to pass and I'm trying to figure out if I can remove it:

Tax-Calculator/taxcalc/parameters.py

Lines 314 to 324 in 9c8be55

	init_vals[param] = pt.select_lte(
	self._init_values[param],
	True,
	{"year": cpi_min_year["year"]},
	)
	to_delete[param] = self.select_gt(
	param, year=cpi_min_year["year"]
	)
	needs_reset.append(param)
	self.delete(to_delete, **kwargs)
	super().adjust(init_vals, **kwargs)

[Peter-Metz]

The way I've thought about this test is that 2017_law.json is current law as of 2017 while TCJA.json is a reform using 2017_law.json as the baseline. So, if a policy parameter appears in 2017_law.json but not TCJA.json, this should mean that the parameter was not changed by TCJA.

For your three parameter examples, EITC_ps_MarriedJ and EITC_c were not changed by TCJA, so we don't expect them to be in TCJA.json. On the other hand, II_em_ps (personal exemption phaseout) was changed by TCJA and I think should belong in TCJA.json. Results aren't actually affected by this omission because II_em (personal exemption amount) is set to zero in TCJA.json.

As for the code snippet in parameters.py, I wrote that as part of the transition from CPI_offset to parameter_indexing_CPI_offset. I think that particular snippet deletes the parameter values that need to be re-indexed.

Hope that helps.

[hdoupe]

Thanks for the reply @Peter-Metz.

As for the code snippet in parameters.py, I wrote that as part of the transition from CPI_offset to parameter_indexing_CPI_offset. I think that particular snippet deletes the parameter values that need to be re-indexed.

Wait, I was referring to the use of the _init_vals dict. Your code is fine! I'm trashing my own code!

The way I've thought about this test is that 2017_law.json is current law as of 2017 while TCJA.json is a reform using 2017_law.json as the baseline. So, if a policy parameter appears in 2017_law.json but not TCJA.json, this should mean that the parameter was not changed by TCJA.

Ok, this makes sense to me. I think I'm remembering you telling me this in the past. I'm still hung up on this:

So, how could it (e.g. EITC_ps_MarriedJ) could it get back to its current law value if the value is updated via the 2017_law.json file but isn't changed again by the TCJA.json file?

This makes me think that the test should only check parameter values that are in the TCJA.json file. @Peter-Metz is that reasonable or no?

I also didn't write this test so I'm a little hesitant to update it. My main motivation is to remove the update logic that re-instates the current law parameters prior to last_known_year every time the parameter_indexing_CPI_offset is changed. This update is what makes EITC_ps_MarriedJ complete the round-trip, for example. I would think that it should be sufficient to just remove the parameters after last_known_year....

Tax-Calculator/taxcalc/parameters.py

Lines 370 to 391 in 9c8be55

	init_vals = {}
	to_delete = {}
	for param in self._data:
	if (
	param in params or
	param == "parameter_indexing_CPI_offset" or
	param in self._wage_indexed
	):
	continue
	if self._data[param].get("indexed", False):
	init_vals[param] = pt.select_lte(
	self._init_values[param],
	True,
	{"year": last_known_year}
	)
	to_delete[param] = self.select_eq(
	param, strict=True, _auto=True
	)
	needs_reset.append(param)
	self.delete(to_delete, **kwargs)
	super().adjust(init_vals, **kwargs)

[jdebacker]

@hdoupe Can this issue be closed?

[hdoupe]

@jdebacker I'm still pretty confused about this test, but happy to re-open the issue or discuss elsewhere when I come back around to it.

[jdebacker]

My understanding of this test is similar to what Peter's saying: It tests that if you revert to 2017 law, then layer on each change since 2017, you should get back to the current, current law baseline.

But I do not understand the snipped in parameters.py that @hdoupe cites. That code should be better documented and hacks to pass tests should be removed from the codebase.

@hdoupe writes:

I'm still hung up on this:

So, how could it (e.g. EITC_ps_MarriedJ) could it get back to its current law value if the value is updated via the 2017_law.json file but isn't changed again by the TCJA.json file?

I need help understanding this. Is this an issue with indexed parameters generally (whose indexing changed with the TCJA)?

[jdebacker]

@hdoupe I think the parameters that are not changed in TCJA.json and are changed in 2017_law.jsonare parameters that were updated sometime before the TCJA reform and so were in the current law baseline in 2017, but were omitted in the2017_law.json` file.

E.g.,

• The 2017_law.json specifies "EITC_ps_MarriedJ": {"2017": [5600, 5600, 5600, 5600]} and the TCJA.json does not change this value. But the current values of 5590 (in policy_current_law.json here) were set in in PR Fix mistaken change in 2017 _EITC_ps_MarriedJ parameter value made in PR 2212 #2240, which was after both the 2017_law.json and TCJA.json files were originally created.

Full list of parameters @hdoupe has found present in 2017_law.json, but not in TCJA.json, with notes explaining why this might be:

• EITC_c: While these values of set in 2017.json (see here), the values are the same as in the policy_current_law.json file (see here).
• EITC_ps: While these values of set in 2017.json (see here), the values are the same as in the policy_current_law.json file (see here).
• EITC_ps_MarriedJ: The current values of 5590 (in policy_current_law.json here) were set in in PR Fix mistaken change in 2017 _EITC_ps_MarriedJ parameter value made in PR 2212 #2240, which was after both the 2017_law.json and TCJA.json files were originally created.
• EITC_InvestIncome_c: While these values of set in 2017.json (see here), the values are the same as in the policy_current_law.json file (see here).
• ETC_pe_Single: While these values of set in 2017.json (see here), the values are the same as in the policy_current_law.json file (see here).
• ETC_pe_Married: While these values of set in 2017.json (see here), the values are the same as in the policy_current_law.json file (see here).
• FST_AGI_thd_lo: While these values of set in 2017.json (see here), the values are the same as in the policy_current_law.json file (see here).
• FST_AGI_thd_h: While these values of set in 2017.json (see here), the values are the same as in the policy_current_law.json file (see here).

In short, I think any discrepancy between the TCJA and 2017 law files can be explained by (1) unnecessary parameter adjustments in the 2017_law.json (because it sets values that are equal to the baseline values) and (2) a change in a parameter that happened after the 2017_law.json was created, but was not reflected in the reform file.