Changes to CPI_offset

[Peter-Metz]

This PR makes two substantive changes to the way that CPI_offset is used and implemented:

1. Change in usage: When CPI_offset is specified in a revision, that value is no longer added to the existing CPI_offset. Rather, the specified value becomes the correction from the unchained CPI.
2. Bug fix: Parameters that are scheduled to revert to their pre-TCJA values in 2026 will now do so if they are not included in the revision. The CPI_offset will still be applied to their indexing. Previously, the values did not revert.

To illustrate (1), before this PR:

In [1]: from taxcalc import *                                                                  
In [2]: import pandas as pd                                                                    
In [3]: import os                                                                              

# current CPI_offset values
In [4]: pol = Policy()                                                                         
In [5]: pol._CPI_offset                                                                        
Out[5]: 
array([ 0.    ,  0.    ,  0.    ,  0.    , -0.0025, -0.0025, -0.0025,
       -0.0025, -0.0025, -0.0025, -0.0025, -0.0025, -0.0025, -0.0025,
       -0.0025, -0.0025, -0.0025, -0.0025])

In [6]: cpi_reform = {"CPI_offset": {2020: 0.001}}                                             
In [7]: pol.implement_reform(cpi_reform)                                                       

# read inflation rates before CPI_offset
In [8]: curr_path = os.path.abspath(os.path.dirname('file'))                                   
In [9]: grow_fact = pd.read_csv(os.path.join(curr_path, 'taxcalc/growfactors.csv'))           
In [10]: cpiu = grow_fact['ACPIU'][2:]                                                         

# compare inflation rates after CPI_offset to raw CPIU
# we see that for 2020 and beyond, our revision of 0.001 was simply added to -0.0025
In [11]: round(pol._inflation_rates - cpiu + 1, 4)                                             
Out[11]: 
2     0.0000
3    -0.0000
4    -0.0000
5    -0.0000
6    -0.0025
7    -0.0025
8    -0.0025
9    -0.0015
10   -0.0015
11   -0.0015

Now, with the same procedure, we have:

# for 2020 and beyond, the effective CPI_offset is set to 0.001
In [11]: round(pol._inflation_rates - cpiu + 1, 4)                                             
Out[11]: 
2     0.0000
3    -0.0000
4    -0.0000
5    -0.0000
6    -0.0025
7    -0.0025
8    -0.0025
9     0.0010
10    0.0010
11    0.0010

To illustrate (2), before the PR:

In [1]: from taxcalc import *                                                                  
In [2]: cpi_reform = {"CPI_offset": {2020: 0.001}}                                                                         
In [3]: pol = Policy()                                             
In [4]: pol.implement_reform(cpi_reform)
# even though STD was not included in the revision, standard
# deduction values do not revert in 2026                                                       
In [5]: pol._STD                                                                               
Out[5]: 
array([[ 6100.  , 12200.  ,  6100.  ,  8950.  , 12200.  ],
       [ 6200.  , 12400.  ,  6200.  ,  9100.  , 12400.  ],
       [ 6300.  , 12600.  ,  6300.  ,  9250.  , 12600.  ],
       [ 6300.  , 12600.  ,  6300.  ,  9300.  , 12600.  ],
       [ 6350.  , 12700.  ,  6350.  ,  9350.  , 12700.  ],
       [12000.  , 24000.  , 12000.  , 18000.  , 24000.  ],
       [12200.  , 24400.  , 12200.  , 18350.  , 24400.  ],
       ...
       [13596.71, 27193.45, 13596.71, 20450.81, 27193.45],
       [13904.  , 27808.02, 13904.  , 20913.  , 27808.02],
       [14201.55, 28403.11, 14201.55, 21360.54, 28403.11],
       [14502.62, 29005.26, 14502.62, 21813.38, 29005.26],
       [14807.18, 29614.37, 14807.18, 22271.46, 29614.37],
       [15115.17, 30230.35, 15115.17, 22734.71, 30230.35],
       [15431.08, 30862.16, 15431.08, 23209.87, 30862.16]])

After the PR, with the same reform:

# standard deduction values revert in 2026, using the new _inflation_rates 
In [6]: pol._STD                                                                                                                                                             
Out[6]: 
array([[ 6100.  , 12200.  ,  6100.  ,  8950.  , 12200.  ],
       [ 6200.  , 12400.  ,  6200.  ,  9100.  , 12400.  ],
       [ 6300.  , 12600.  ,  6300.  ,  9250.  , 12600.  ],
       [ 6300.  , 12600.  ,  6300.  ,  9300.  , 12600.  ],
       [ 6350.  , 12700.  ,  6350.  ,  9350.  , 12700.  ],
       [12000.  , 24000.  , 12000.  , 18000.  , 24000.  ],
       [12200.  , 24400.  , 12200.  , 18350.  , 24400.  ],
       ...
       [13730.07, 27460.13, 13730.07, 20651.37, 27460.13],
       [14074.69, 28149.38, 14074.69, 21169.72, 28149.38],
       [ 7810.  , 15620.  ,  7810.  , 11500.  , 15620.  ],
       [ 7995.1 , 15990.19,  7995.1 , 11772.55, 15990.19],
       [ 8182.98, 16365.96,  8182.98, 12049.2 , 16365.96],
       [ 8373.64, 16747.29,  8373.64, 12329.95, 16747.29],
       [ 8569.58, 17139.18,  8569.58, 12618.47, 17139.18]])

cc @MattHJensen @hdoupe

[codecov]

Codecov Report

Merging #2413 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2413   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files          13       13           
  Lines        2530     2577   +47     
=======================================
+ Hits         2528     2575   +47     
  Misses          2        2           

Impacted Files	Coverage Δ
taxcalc/parameters.py	99.33% <100.00%> (+0.06%)	⬆️
taxcalc/policy.py	100.00% <100.00%> (ø)
taxcalc/data.py	100.00% <0.00%> (ø)
taxcalc/utils.py	100.00% <0.00%> (ø)
taxcalc/records.py	100.00% <0.00%> (ø)
taxcalc/growdiff.py	100.00% <0.00%> (ø)
taxcalc/taxcalcio.py	100.00% <0.00%> (ø)
taxcalc/calculator.py	100.00% <0.00%> (ø)
taxcalc/consumption.py	100.00% <0.00%> (ø)
... and 2 more

---

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 0af305e...c14e7a6. Read the comment docs.

[MattHJensen]

Thanks very much for opening this PR, @Peter-Metz! I will review, and of course anyone else is welcome to as well.

[hdoupe]

@Peter-Metz this looks good! I'll take a closer look today and tomorrow.

[hdoupe]

Hey @Peter-Metz, I'm trying to get going on this PR review again. Sorry for the long delay!

I've made some updates on this init-question branch so that we can drop the use of init_vals. I'm getting two test failures in test_reform.py. Both have to do with the TCJA.json

taxcalc/tests/test_reforms.py::test_round_trip_tcja_reform 
taxcalc/tests/test_reforms.py::test_reform_json_and_output # with TCJA.res.csv being the failing reform

I'm focusing on the test_round_trip_tcja_reform test. Hopefully, fixing the parameter issue here will resolve the error relating to the other test.

E           ValueError: 
E           Round-trip-reform and current-law-policy param values differ for:
E             II_em_ps in 2020 : rtr=[[278620.28 334344.35 167172.17 306482.31 334344.35]] clp=[[9.e+99 9.e+99 9.e+99 9.e+99 9.e+99]]
E             STD_Dep in 2020 : rtr=[1115.99] clp=[1117.38]
E             STD_Aged in 2020 : rtr=[[1647.42 1328.57 1328.57 1647.42 1647.42]] clp=[[1676.07 1320.54 1320.54 1676.07 1320.54]]
E             CG_brk1 in 2020 : rtr=[[40335.31 80670.6  40335.31 53992.97 80670.6 ]] clp=[[39997.12 79994.25 39997.12 53583.45 79994.25]]
E             CG_brk2 in 2020 : rtr=[[444698.   500285.25 250142.63 472491.63 500285.25]] clp=[[441415.89 496573.83 248286.92 468994.86 496573.83]]
E             AMT_CG_brk1 in 2020 : rtr=[[40335.31 80670.6  40335.31 53992.97 80670.6 ]] clp=[[39997.12 79994.25 39997.12 53583.45 79994.25]]
E             AMT_CG_brk2 in 2020 : rtr=[[444698.   500285.25 250142.63 472491.63 500285.25]] clp=[[441415.89 496573.83 248286.92 468994.86 496573.83]]
E             AMT_child_em in 2020 : rtr=[7971.4] clp=[0.]
E             AMT_brk1 in 2020 : rtr=[199603.93] clp=[197877.84]
E             EITC_c in 2020 : rtr=[[ 542.05 3613.7  5968.98 6715.11]] clp=[[ 537.36 3581.71 5920.08 6660.6 ]]
E             EITC_ps in 2020 : rtr=[[ 8864.2  19492.73 19492.73 19492.73]] clp=[[ 8786.67 19330.67 19330.67 19330.67]]
E             EITC_ps_MarriedJ in 2020 : rtr=[[5951.98 5951.98 5951.98 5951.98]] clp=[[5891.64 5881.48 5881.48 5881.48]]
E             EITC_InvestIncome_c in 2020 : rtr=[3666.84] clp=[3656.88]
E             ETC_pe_Single in 2020 : rtr=[70.15] clp=[69.07]
E             ETC_pe_Married in 2020 : rtr=[140.29] clp=[138.15]
E             FST_AGI_thd_lo in 2020 : rtr=[[1062853.74 1062853.74  531426.86 1062853.74 1062853.74]] clp=[[1015800. 1015800.  507900. 1015800. 1015800.]]
E             FST_AGI_thd_hi in 2020 : rtr=[[2125707.47 2125707.47 1062853.74 2125707.47 2125707.47]] clp=[[2031600. 2031600. 1015800. 2031600. 2031600.]]

Looking at the II_em_ps parameter, the round trip value is 278620.28 for single tax units, but the expected value is 9e99. In the reform files, this parameter is only changed in 2017_law.json:

$ grep II_em_ps taxcalc/reforms/*json 
taxcalc/reforms/2017_law.json:    "II_em_ps": {"2017": [261500, 313800, 156900, 287650, 313800]},

See update here: https://github.com/PSLmodels/Tax-Calculator/blob/master/taxcalc/reforms/2017_law.json#L86

This is true for some of the other parameters in this error message:

(taxcalc-dev) ~/Documents/Tax-Calculator 
$ grep II_em_ps taxcalc/reforms/*json 
taxcalc/reforms/2017_law.json:    "II_em_ps": {"2017": [261500, 313800, 156900, 287650, 313800]},
(taxcalc-dev) ~/Documents/Tax-Calculator 
$ grep STD_Dep taxcalc/reforms/*json 
taxcalc/reforms/2017_law.json:    "STD_Dep": {"2017": 1050},
(taxcalc-dev) ~/Documents/Tax-Calculator 
$ grep STD_Aged taxcalc/reforms/*json 
taxcalc/reforms/2017_law.json:    "STD_Aged": {"2017": [1550, 1250, 1250, 1550, 1550]},
(taxcalc-dev) ~/Documents/Tax-Calculator 
$ grep CG_brk1 taxcalc/reforms/*json 
taxcalc/reforms/2017_law.json:    "CG_brk1": {"2017": [37950, 75900, 37950, 50800, 75900]},
taxcalc/reforms/2017_law.json:    "AMT_CG_brk1": {"2017": [37950, 75900, 37950, 50800, 75900]},
taxcalc/reforms/Trump2016.json:    "CG_brk1": {"2017": [37500, 75000, 37500, 37500, 75000]},
(taxcalc-dev) ~/Documents/Tax-Calculator 
$ grep EITC_c taxcalc/reforms/*json 
taxcalc/reforms/2017_law.json:    "EITC_c": {"2017": [510, 3400, 5616, 6318]},
taxcalc/reforms/BrownKhanna.json:// - 1: EITC_c
taxcalc/reforms/BrownKhanna.json:    "EITC_c": {"2017": [3000, 6528, 10783, 12131]},
taxcalc/reforms/Renacci.json://   7: EITC_c
taxcalc/reforms/Renacci.json:    "EITC_c": {"2017": [1020, 4760, 7862, 8845]}
(taxcalc-dev) ~/Documents/Tax-Calculator 

My question is: why are these values not reset to the CLP value in TCJA.json? I'm having trouble understanding how they can be reset to their current law value if they are changed in 2017_law.json, but not reset back to the current law in TCJA.json?

[hdoupe]

@Peter-Metz and I just wrapped up a debugging session. We think that this PR is ready to be merged. I can try to remove the use of init_vals in a future PR. In theory, we shouldn't need to keep a copy of the current law policy parameter values. But, in practice, this caused some problems with the test_round_trip_tcja_reform test in test_reforms.py.

I'll keep thinking about this problem, but it shouldn't block this PR.

For future reference, some approaches we tried were:

• Pin-pointing the failure with a simpler test instead of test_round_trip_tcja_reform:
  • Tried implementing multiple reforms that affect cpi_offset on the same policy object
  • Testing specific parameters after applying 2017_law.json
  • Strengthening existing tests to check values over a larger range of years.
• Testing more years in test_round_trip_tcja_reform instead of just 2020.
• Analyzing 2017_law.json and TCJA.json to see if they needed to be updated.

Independent of this debugging session, I also stepped through adjust_with_indexing using the breakpoint function to check things like the results from ParamTools queries (in case the bug was there).

I'll keep thinking about whether use of init_vals should be removed, but this shouldn't block this PR.

[Peter-Metz]

@hdoupe thanks for all of your work reviewing this PR!

[MattHJensen]

@Peter-Metz, thanks very much for this PR! The bug fix and enhanced behavior both look good and make sense to me.

As I think about the implications of the change in usage, though, I am growing somewhat concerned for users who may still be relying on old reform jsons that depend on the the old cpi_offset behavior. When they upgrade, those reform files could begin to produce erroneous results silently. Unless the user pays careful attention to the changelogs, s/he could be in for trouble.

First of all, I am interested to know if you share these concerns.

If so, what do you think about an approach along these lines:

• Fix the bug for the current cpi_offset parameter
• Introduce a new parameter, something like cpi_offset_levels
• Document and announce that cpi_offset is deprecated, will be removed with the next major release, and cpi_offset_levels is preferred.

This would allow us to wait until Max's new docs are incorporated, and perhaps a bit more, before making a major release.

This would only make sense if the bug fix and usage change are separable.

[Peter-Metz]

@MattHJensen, thanks for the feedback. Just to clarify, what would the cpi_offset_levels parameter do and what would its current law value be?

My view is that while this PR may affect a small number of users, it would still do more good than harm because of the counterintuitive behavior of cpi_offset. Especially on Tax-Brain, I've seen multiple users make the mistake of setting cpi_offset to 0 to switch back to unchained CPI, when they need to be setting the parameter to 0.0025. After taking another look at the documentation for the parameter, I don't think the behavior is explained properly in the first place:

Decimal offset ADDED to unchained CPI to get parameter indexing rate

True, but the description doesn't discuss the additive behavior of the parameter values themselves, which is what makes this parameter different from every other Tax-Calculator policy parameter.

My suggestion would be to add a warning that the definition of cpi_offset was changed, like we did for CTC_c (#2236).

[MattHJensen]

Just to clarify, what would the cpi_offset_levels parameter do and what would its current law value be?

cpi_offset_levels would be the new cpi_offset, with the new behavior, exactly as you have it in this PR, but given a new name cpi_offset_levels (or hopefully something better).

cpi_offset would keep the old cpi_offset behavior, as before this PR, but updated for the bugfix.

cpi_offset would immediately be documented as deprecated and hidden from Tax-Brain users and users of other downstream apps.

My suggestion would be to add a warning that the definition of cpi_offset was changed, like we did for CTC_c (#2236).

This is another viable solution. My recollection though is that the warning proved annoying (because everyone saw it, even if it wasn't relevant to them) and then we removed it within a couple of years, so things could still fail silently after that.

[hdoupe]

I like the idea of creating a new parameter cpi_offset_levels. This lets all users who are using cpi_offset in Python scripts continue to do so, but it updates the webapp immediately.

I'm in favor of adding a warning if users use cpi_offset. Unlike the current REDEFINIED_PARAMS, we should add a date or version number where the parameter and warning should be removed. We could then add a test that raises an error when the version or date has exceeded the sunset version or date. Once we remove the redefined parameter from REDEFINED_PARAMS, it should be put in REMOVED_PARAMS and removed policy_current_law.json.

Also, this warning should only be shown to users that use the redefined parameter and not all users who use Tax-Calculator.

[Peter-Metz]

@MattHJensen I understand the benefits of this approach but I'm wary of the complications/ unexpected interactions this could cause.

Do you propose setting the current law values for cpi_offset to zero?

Another option would be to change the name of the parameter to cpi_offset_levels and print a specialized error message for someone that tries to use cpi_offset.

[MattHJensen]

Do you propose setting the current law values for cpi_offset to zero?

Yes, that was my thinking.

Another option would be to change the name of the parameter to cpi_offset_levels and print a specialized error message for someone that tries to use cpi_offset.

This would resolve my concerns; it would just require that the next version be a major release. Since we are ripe for a major release anyways, pending #2420, I would be happy to do this. (If a similar situation comes up in the future when we aren't otherwise ready for a major release, we could reconsider methods for phased removal.)

[Peter-Metz]

Sounds good @MattHJensen. I've updated the parameter name to parameter_indexing_CPI_offset. The error message for both cpi_offset and CPI_offset now reads:

CPI_offset was renamed parameter_indexing_CPI_offset. See documentation for change in usage.

[MattHJensen]

@Peter-Metz, thanks a great deal for this PR. Merging.