EITC phase-in rate values being changed #596

hdoupe · 2017-07-28T19:23:35Z

This issue was initially raised in Tax-Calculator issue #1485.

A TaxBrain user is wanting to set an EITC rate (eitc_rt) over 100 percent.

The EITC rate description states that the range of the parameter is between 0 and 100 percent.

To experiment, I ran two reforms, one with the rate set to 100 percent for all filing statuses and one with the rates at 150 percent, and I got identical results for both.

All EITC rates at 100 percent.
http://www.ospc.org/taxbrain/15189/

All EITC rates at 150 percent.
http://www.ospc.org/taxbrain/15190/

can we allow rates over 100 percent?

was this a silent error, and are there other scenarios where that might be happening?

Here are results from the celery log when I ran the two reforms mentioned above. Celery log for _EITC_rt set to 1:

[2017-07-28 10:16:35,533: WARNING/MainProcess] keywords to dropq
[2017-07-28 10:16:35,534: WARNING/MainProcess] {'start_year': 2017, 'user_mods': {u'growdiff_response': {}, u'consumption': {}, u'growdiff_baseline': {}, u'behavior': {}, u'policy': {2017: {u'_EITC_rt': [[1.0, 1.0, 1.0, 1.0]], u'_NIIT_PT_taxed': [False], u'_ID_BenefitCap_Switch': [[True, True, True, True, 1.0, 1.0, 1.0]], u'_ALD_InvInc_ec_base_RyanBrady': [False], u'_EITC_indiv': [False], u'_ID_BenefitSurtax_Switch': [[True, True, True, True, True, True, True]], u'_II_no_em_nu18': [False], u'_CG_nodiff': [False], u'_CTC_new_refund_limited': [False]}}, u'gdp_elasticity': {}}, 'year_n': 0}

and _EITC_rt set to 1.5:

[2017-07-28 10:32:22,549: WARNING/MainProcess] keywords to dropq
[2017-07-28 10:32:22,550: WARNING/MainProcess] {'start_year': 2017, 'user_mods': {u'growdiff_response': {}, u'consumption': {}, u'growdiff_baseline': {}, u'behavior': {}, u'policy': {2017: {u'_EITC_rt': [[1, 1, 1, 1]], u'_NIIT_PT_taxed': [False], u'_ID_BenefitCap_Switch': [[True, True, True, True, 1.0, 1.0, 1.0]], u'_ALD_InvInc_ec_base_RyanBrady': [False], u'_EITC_indiv': [False], u'_ID_BenefitSurtax_Switch': [[True, True, True, True, True, True, True]], u'_II_no_em_nu18': [False], u'_CG_nodiff': [False], u'_CTC_new_refund_limited': [False]}}, u'gdp_elasticity': {}}, 'year_n': 1}

There are two problems here:

_EITC_rt is being rounded down when it shouldn't be. This may be an issue with other parameters, too. I'm not sure what the acceptable range would be for this parameter.
It appears that if a value is edited so that it fits into some range (for example 0 to 1) then it is also converted to an integer. On the other hand, if a value is entered into the taxbrain interface, that value is stored as a float. As shown here here, if two reforms have the same effective values for a parameter but one is 1.0 and the other is 1, then a different seed is selected in the dropq algorithm and the results are slightly different. This isn't a big problem, but if there is an easy fix, then I think it would be worth it.

@martinholmer @PeterDSteinberg @brittainhard

The text was updated successfully, but these errors were encountered:

martinholmer · 2017-08-04T02:30:03Z

The following code from the apps/taxbrain/helper.py file may be the source of the bug reported in #596. It looks as if every policy parameter greater than one is being rounded to its nearest integer value. I don't understand the webapp code at all and so could be wrong about this, but the code below looks like it could be the source of this bug. This code was added during February 2016 in pull request #159 ("Round all parameter values to the nearest dollar") and pull request #161, which round only policy parameters whose value was greater than one. Given the bug report in #596, it would appear that the _EITC_rt phase-in rate parameter, which can have a value greater than one, is being mistaken by the TaxBrain GUI as a dollar parameter when it is, in reality, a decimal rate that should not be rounded to the nearest dollar.

def round_gt_one_to_nearest_int(values):
    ''' round every value to the nearest integer '''
    def round_gt_one(x):
        if x >= 1.0:
            return round(x)
        else:
            return x
    try:
        rounded = map(round_gt_one, values)
    except TypeError:
        rounded = [map(round_gt_one, val) for val in values]
    return rounded


def default_taxcalc_data(cls, start_year, metadata=False):
    '''
    Call the default data function on the given class for the given
    start year with meatadata flag
    '''
    dd = cls.default_data(metadata=metadata, start_year=start_year)
    if metadata:
        for k in dd:
            dd[k]['value'] = round_gt_one_to_nearest_int(dd[k]['value'])
    else:
        for k in dd:
            dd[k] = round_gt_one_to_nearest_int(dd[k])
    return dd

@brittainhard @hdoupe

hdoupe · 2017-08-04T14:18:45Z

@martinholmer said

Given the bug report in #596, it would appear that the _EITC_rt phase-in rate parameter, which can have a value greater than one, is being mistaken by the TaxBrain GUI as a dollar parameter when it is, in reality, a decimal rate that should not be rounded to the nearest dollar.

That makes sense to me. We could do something like this:

def default_taxcalc_data(cls, start_year, metadata=False):
    '''
    Call the default data function on the given class for the given
    start year with meatadata flag
    '''
    dd = cls.default_data(metadata=metadata, start_year=start_year)
    if metadata:
        for k in dd:
            if k not in ['EITC_rt, ....]:
                dd[k]['value'] = round_gt_one_to_nearest_int(dd[k]['value'])
    else:
        for k in dd:
            if k not in ['EITC_rt, ....]:
                dd[k] = round_gt_one_to_nearest_int(dd[k])
    return dd

@brittainhard

talumbau · 2017-08-04T15:25:17Z

@hdoupe that could work. I would pull in @MattHJensen and draw his attention to issue #160

which explains the rationale for the original change. I would then propose the creation of some set of rounding_exempt variables, which would be checked in the function you show, and then added to over time.

martinholmer · 2017-08-04T16:45:27Z

Before deciding how to fix the bug being discussed in webapp-public issue #596 (which was reported by a TaxBrain user), let's take a step back to consider some basic issues.

(1) The cause of the bug seems to be this function, which would be more appropriately named round_parameters_greater_than_one:

def default_taxcalc_data(cls, start_year, metadata=False):
    ''' Call the default data function on the given class for the given
        start year with meatadata flag
    '''
    dd = cls.default_data(metadata=metadata, start_year=start_year)
    if metadata:
        for k in dd:
            dd[k]['value'] = round_gt_one_to_nearest_int(dd[k]['value'])
    else:
        for k in dd:
            dd[k] = round_gt_one_to_nearest_int(dd[k])
    return dd

(2) This function is called not only with a Policy parameters object as the cls argument, but is also called using Behavior and Growdiff objects. (I'm not sure why parameters in a Consumption object are not being rounded, but that is a minor issue). We know for a fact that a _BE_sub value in excess of one is valid (but probably implausible), and because it is an elasticity (not a dollar amount) it should not be rounded to the nearest integer. There is a validations field in behavior.json that requires this parameter be non-negative, but there is no maximum value specified in the validations field. The big question is how many other parameters are being incorrectly rounded.

(3) My basic question is why do parameter values need to be rounded in TaxBrain. The comment in the check-in that added the offending function said this:

With release of taxcalc 0.6, we have floating point values for our parameters. When these values are CPI-inflated in time, we choose to round them to the nearest dollar when displaying them on TaxBrain.

So, why exactly did "we choose to round them to the nearest dollar"? Aren't all the dollar-valued policy parameters for years 2017 and before that are being displayed in the TaxBrain GUI already expressed in whole dollars in the current_law_policy.json file? Why not just drop all this rounding business in TaxBrain. I don't think it is adding much, if any, value, and it certainly is causing bugs that TaxBrain users are noticing and reporting.

@hdoupe @talumbau @brittainhard

talumbau · 2017-08-04T17:02:16Z

I agree that no rounding makes things easier. The issue, as I recall, is that parameters that were CPI-inflated to cover the budget window were not being rounded at all, which was seen as undesirable by @MattHJensen, that is my recollection anyway. An easy fix would be to remove all rounding code, which would allow entry of parameters that are not whole dollars. Additionally, any default parameters for a year, e.g. 2017, which are computed via inflation of a specified parameter for a prior year (e.g. a parameter with a default value given for 2016), would show up as their full floating point value, not rounded to a whole dollar.

martinholmer · 2017-08-04T17:19:41Z

@talumbau said about issue #596:

I agree that no rounding makes things easier. The issue, as I recall, is that parameters that were CPI-inflated to cover the budget window were not being rounded at all, which was seen as undesirable by @MattHJensen, that is my recollection anyway. An easy fix would be to remove all rounding code, which would allow entry of parameters that are not whole dollars. Additionally, any default parameters for a year, e.g. 2017, which are computed via inflation of a specified parameter for a prior year (e.g. a parameter with a default value given for 2016), would show up as their full floating point value, not rounded to a whole dollar.

I think this is a good first step: turn off all the TaxBrain rounding. Then we can see exactly where we stand with respect to TaxBrain GUI readability.

My understanding is that all the dollar-valued policy parameters for 2017 (and earlier years) are expressed as whole numbers.

So, just adding one line to the function will "turn off" the rounding, right?

def default_taxcalc_data(cls, start_year, metadata=False):
    ''' Call the default data function on the given class for the given
        start year with meatadata flag
    '''
    dd = cls.default_data(metadata=metadata, start_year=start_year)
+   return dd
    if metadata:
        for k in dd:
            dd[k]['value'] = round_gt_one_to_nearest_int(dd[k]['value'])
    else:
        for k in dd:
            dd[k] = round_gt_one_to_nearest_int(dd[k])
    return dd

This minimal change will make it easy to go back to where we are now in case we need to do that.

Can we add that one line and then do some local testing?

@hdoupe @brittainhard

hdoupe · 2017-08-04T17:37:39Z

@martinholmer That sounds sensible to me. I can make the change and compare the results from a local taxbrain instance and taxcalc with the reform:

{"policy": 
    {'_EITC_rt': 
        {"2017": [[1.5, 1.5, 1.5, 1.5]]}
    }
}

@brittainhard

martinholmer · 2017-08-04T17:57:26Z

@hdoupe said:

That sounds sensible to me. I can make the change and compare the results from a local taxbrain instance and taxcalc with the reform:

{"policy": 
    {'_EITC_rt': 
        {"2017": [[1.5, 1.5, 1.5, 1.5]]}
    }
}

Sound good, but maybe try something other than all 1.5 so that you know which way to rounding would go under current version. Maybe [[1.4,1.4,1.6,1,6]]?

hdoupe · 2017-08-04T17:58:52Z

@martinholmer said

Sound good, but maybe try something other than all 1.5 so that you know which way to rounding would go under current version. Maybe [[1.4,1.4,1.6,1,6]]?

Good idea. I'll give that a shot.

hdoupe · 2017-08-04T18:24:09Z

@martinholmer I decided to use the reform: [[0.75,1.4,1.6,1,6]].

Before removing the rounding function the keywords to dropq were :

keywords to dropq
[2017-08-04 14:02:17,700: WARNING/MainProcess] {'start_year': 2017, 'user_mods': {u'growdiff_response': {}, u'consumption': {}, u'growdiff_baseline': {}, u'behavior': {}, u'policy': {2017: {u'_EITC_rt': [[0.75, 1, 1, 1]], u'_NIIT_PT_taxed': [False], u'_ID_BenefitCap_Switch': [[True, True, True, True, 1.0, 1.0, 1.0]], u'_ALD_InvInc_ec_base_RyanBrady': [False], u'_EITC_indiv': [False], u'_ID_BenefitSurtax_Switch': [[True, True, True, True, True, True, True]], u'_II_no_em_nu18': [False], u'_CG_nodiff': [False], u'_CTC_new_refund_limited': [False]}}, u'gdp_elasticity': {}}, 'year_n': 1}

The results after removing the rounding:

keywords to dropq
[2017-08-04 14:19:06,461: WARNING/MainProcess] {'start_year': 2017, 'user_mods': {u'growdiff_response': {}, u'consumption': {}, u'growdiff_baseline': {}, u'behavior': {}, u'policy': {2017: {u'_EITC_rt': [[0.75, 1, 1, 1]], u'_NIIT_PT_taxed': [False], u'_ID_BenefitCap_Switch': [[True, True, True, True, 1.0, 1.0, 1.0]], u'_ALD_InvInc_ec_base_RyanBrady': [False], u'_EITC_indiv': [False], u'_ID_BenefitSurtax_Switch': [[True, True, True, True, True, True, True]], u'_II_no_em_nu18': [False], u'_CG_nodiff': [False], u'_CTC_new_refund_limited': [False]}}, u'gdp_elasticity': {}}, 'year_n': 4}

Changing default_taxcalc_data does not appear to change the results. Does this function only change the default data and not the user inputs?

martinholmer · 2017-08-04T18:49:38Z

@hdoupe said:

The results after removing the rounding:

What did you do to remove the rounding?

@hdoupe said:

Changing default_taxcalc_data does not appear to change the results. Does this function only change the default data and not the user inputs?

This function is called many times in several different files, but maybe it doesn't effect reform specifications. Can you trace TaxBrain GUI to see where the values greater than one are set to one? Or, better yet, maybe the people who wrote the TaxBrain code can tell you where to look.

@brittainhard @PeterDSteinberg @talumbau

hdoupe · 2017-08-04T18:58:12Z

@martinholmer I did exactly what you recommended:

def default_taxcalc_data(cls, start_year, metadata=False):
    ''' Call the default data function on the given class for the given
        start year with meatadata flag
    '''
    dd = cls.default_data(metadata=metadata, start_year=start_year)
    return dd
    if metadata:
        for k in dd:
            dd[k]['value'] = round_gt_one_to_nearest_int(dd[k]['value'])
    else:
        for k in dd:
            dd[k] = round_gt_one_to_nearest_int(dd[k])
    return dd

I searched the directory and the function round_gt_one_to_nearest_int is only called from the function default_taxcalc_data.

HDoupe-MacBook-Pro:webapp-public henrydoupe$ grep -r round_gt_one_to_nearest_int ./
.//webapp/apps/btax/helpers.py:                                round_gt_one_to_nearest_int, expand_1D,
Binary file .//webapp/apps/btax/helpers.pyc matches
.//webapp/apps/taxbrain/helpers.py:def round_gt_one_to_nearest_int(values):
.//webapp/apps/taxbrain/helpers.py:            dd[k]['value'] = round_gt_one_to_nearest_int(dd[k]['value'])
.//webapp/apps/taxbrain/helpers.py:            dd[k] = round_gt_one_to_nearest_int(dd[k])
Binary file .//webapp/apps/taxbrain/helpers.pyc matches

martinholmer · 2017-08-04T19:42:18Z

@hdoupe said:

I searched the directory and the function round_gt_one_to_nearest_int is only called from the function default_taxcalc_data.

Yes, that's true. But search to see how many times default_taxcalc_data is called.

What does @brittainhard say about where in the code the _EITC_rt parameter is being rounded down to one?

martinholmer · 2017-08-04T20:54:35Z

The experiments conducted by @hdoupe indicate that the default_taxcalc_data function (which calls round_gt_one_to_nearest_int) is not the source of the TaxBrain bug reported in issue #596.
Apparently, it is used only to round the parameter values found in the current_law_policy.json file. Is that correct, @talumbau?

As I continue to explore the code in the webapp/apps/taxbrain directory, I have another hunch about the source of the bug. Consider the propagate_user_list function which is called from the package_up_vars(user_values, first_budget_year) function. Here it is:

def propagate_user_list(x, name, defaults, cpi, first_budget_year,
                        multi_param_idx=-1):
    """
    Dispatch to either expand_1D or expand2D depending on the dimension of x

    Parameters:
    -----------
    x : list from user to propagate forward in time. The first value is for
        year 'first_budget_year'. The value at index i is the value for
        budget year first_budget_year + i.

    defaults: list of default values; our result must be at least this long

    name: the parameter name for looking up the indexing rate

    cpi: Bool

    first_budget_year: int

    multi_param_idx: int, optional. If this parameter is multi-valued, this
        is the index for which the values for 'x' apply. So, for exampe, if
        multi_param_idx=0, the values for x are typically for the 'single'
        filer status. -1 indidcates that this is not a multi-valued
        parameter

    Returns:
    --------
    list of length 'num_years'. if 'cpi'==True, the values will be inflated
    based on the last value the user specified

    """
    # x must have a real first value
    assert len(x) > 0
    assert x[0] not in ("", None)

    num_years = max(len(defaults), len(x))

    is_rate = any([ i < 1.0 for i in x])    # <------------------------------------------

    current_policy = Policy(start_year=2013)
    current_policy.set_year(first_budget_year)
    # irates are rates for 2015, 2016, and 2017
    if cpi:
        irates = current_policy._indexing_rates_for_update(param_name=name,
                                              calyear=first_budget_year,
                                              num_years_to_expand=num_years)
    else:
        irates = [0.0] * num_years

    last_val = x[-1]
    ans = [None] * num_years
    for i in range(num_years):
        if i < len(x):
            if is_wildcard(x[i]):
                if multi_param_idx > -1:
                    ans[i] = defaults[i][multi_param_idx]
                else:
                    ans[i] = defaults[i]

            else:
                ans[i] = x[i]

        if ans[i] is not None:
            continue
        else:
            newval = ans[i-1] * (1.0 + irates[i-1])
            ans[i] = newval if is_rate else int(newval)    # <---------------------------

    return ans

Notice that the two statements marked with a <---------------- comment work together to truncate floating-point values greater than one. This means that a float value of 1.4 is converted to an integer value of 1, and that a float value of 1.6 is also converted to an integer value of 1, and that a float value of 3.9 is truncated to an integer value of 3. This is the behavior @hdoupe observed in his experiments.

If this is the source of the error, then I have the same question as before: why is TaxBrain changing the parameter values specified by users? What is the rationale for changing user specified values?

talumbau · 2017-08-04T22:13:33Z

I believe that you are pointing to the correct place in the code where the int conversion occurs.

If this is the source of the error, then I have the same question as before: why is TaxBrain changing the parameter values specified by users? What is the rationale for changing user specified values?

Looking at the history of helpers.py, the code has forced an int conversion of user parameters (that were not rates) since at least July of 2015, which is the full length of time of the existence of this repo. Discussions/code changes prior to that were on the old version of the repo, which was a private repo and I believe it was deleted. My recollection is that this was a user request. If the private repo is still around, it might be worthwhile to search the issue history or do a "git blame" operation to see if there are helpful commit messages for these changes.

Besides just being a user request, there's another reason to truncate to integer: it brings consistency to the * notation. Consider the case of when the default start year was 2016 and the user submits a * for a 2017 value. When the user submits a * entry for a parameter, it must be filled in with an actual value for processing. If the parameter is CPI inflated and we have no value specified in the current_law_policy.json, we use the inflation rate for that parameter and expand from the last specified value. The result of this operation is a floating point value. However, if the user changes the default start year to 2017 and reloads the TaxBrain page, we display for them an integer value for that parameter. So, in order to provide a consistent value between the two cases, the meaning of a * value is (in the case of a CPI inflated value): a value specified in the current_law_policy.json or, if none is specified, the inflated value based on the inflation rate for that parameter from the year last specified to the desired year, truncated to an integer. The last part (truncation to an integer) makes it so that this value is the same default value the user would get by setting the Start Year for TaxBrain to the desired future year.

Of course, one can still apply the above logic to * entries and handle them as a special case, while still allowing someone to set the standard deduction to $6350.50, for example. It's just that currently all user inputs (whether numerical literals or * entries) are treated the same.

martinholmer · 2017-08-07T17:29:17Z

@talumbau said about the webapp-public code that causes the #596 bug:

Looking at the history of helpers.py, the code has forced an int conversion of user parameters (that were not rates) since at least July of 2015, which is the full length of time of the existence of this repo.

All that is true except for the (that were not rates) part. In the initial commit on July 21, 2015, there was no is_rate variable and no propogate_user_list function. The propogate_user_list function was added nearly eight months later on March 16, 2016, and was amended the next day to include the is_rate variable in response to the bug reported in issue #202. These changes were part of pull request #203, which was merged on March 17, 2016.

So, you are correct that the #596 bug, which was reported by a TaxBrain user who is not part of the development team, has been present in the code since at least July 2015.

@talumbau also said:

... there's another reason to truncate to integer: it brings consistency to the * notation. Consider the case of when the default start year was 2016 and the user submits a * for a 2017 value. When the user submits a * entry for a parameter, it must be filled in with an actual value for processing. If the parameter is CPI inflated and we have no value specified in the current_law_policy.json, we use the inflation rate for that parameter and expand from the last specified value. The result of this operation is a floating point value. However, if the user changes the default start year to 2017 and reloads the TaxBrain page, we display for them an integer value for that parameter. So, in order to provide a consistent value between the two cases, the meaning of a * value is (in the case of a CPI inflated value): a value specified in the current_law_policy.json or, if none is specified, the inflated value based on the inflation rate for that parameter from the year last specified to the desired year, truncated to an integer. The last part (truncation to an integer) makes it so that this value is the same default value the user would get by setting the Start Year for TaxBrain to the desired future year.

What you say is true only if we continue to use what seems to me a sub-optimal approach to sending the TaxBrain Input GUI information to Tax-Calculator. It seems to me that all the problems you describe go away if we make a change in how the Input GUI info is sent to Tax-Calculator. Why doesn't TaxBrain translate the information that is changed by a user into a JSON reform file and then send that file to Tax-Calculator (in the same way as the TaxBrain File Upload page does)? This approach seems to by-pass the * problems you mention above because we never need to specify the default value of a parameter in years for which the user enters a * character.

Let me illustrate what I mean using a simple example. Suppose a TaxBrain user wants to analyze a this reform when the start year is 2017:

Social Security payroll tax rate: *,0.130
Maximum taxable earnings for Social Security: *,*,*,350000,*,*,450000

In words, the reform raises the tax rate to 0.130 beginning in 2018, and raises the MTE in two steps: to $350,000 in 2020 with indexing after that and to $450,000 in 2023 with indexing after that.

Here is what the JSON reform file looks like for this reform:

{
  "policy": {
    "_FICA_ss_trt" : {"2018": [0.130]},
    "_SS_Earnings_c": {"2020": [350000], "2023": [450000]}
  }
}

Notice that there are no leading or intermediate indexed values that need to be computed and used to characterize the reform, thus simplifying the work of TaxBrain to communicate to Tax-Calculator what the reform is. Would it be possible for the JavaScript that is controlling the logic of the TaxBrain Input GUI to make this translation using only the input boxes that have been changed? Sending such a JSON reform file to the TaxBrain server would be much easier that sending all the changed and unchanged parameter values (which is how things work now if I understand your explanation of the POST logic). Once such a JSON reform file was received by the TaxBrain server, the current version of Tax-Calculator offers the capability of converting such a JSON reform file into a user_mods dictionary required by the dropq functions.

Perhaps I'm missing something here, but Tax-Calculator has a broader set of capabilities than it had when TaxBrain was originally designed. Maybe we should be simplifying TaxBrain (and eliminating what are now code duplications) by making use of these new Tax-Calculator capabilities.

talumbau · 2017-08-08T04:55:30Z

All that is true except for the (that were not rates) part. In the initial commit on July 21, 2015, there was no is_rate variable and no propogate_user_list function. The propogate_user_list function was added nearly eight months later on March 16, 2016, and was amended the next day to include the is_rate variable in response to the bug reported in issue #202. These changes were part of pull request #203, which was merged on March 17, 2016.

So, you are correct that the #596 bug, which was reported by a TaxBrain user who is not part of the development team, has been present in the code since at least July 2015.

Actually, that is not correct. If you look at the version of the helpers.py function from July 21, 2015, you will see that at line 258 the truncation to integer occurs, but for variables processed by line 280 the integer truncation does not occur. Parameter values that differ by filing status are broken up with _1, _2, etc. by convention in the Django models. Those parameters were processed by line 258, all the rest by line 280. At this time in the history of this repo and Tax-Calculator, all parameter variables in the params.json in Tax-Calculator that were accessible via TaxBrain and had the _# suffixes were parameters specifying dollar amounts, not rates. So, at this time, the only parameter values that were truncated to an integer were ones that represent dollar amounts, and TaxBrain entries for rates became floats.

Why doesn't TaxBrain translate the information that is changed by a user into a JSON reform file and then send that file to Tax-Calculator (in the same way as the TaxBrain File Upload page does)?

This seems like a good idea. I just have one question: what is the proper JSON reform file for this entry in TaxBrain?

This is the _SS_thd50 parameter, but here the user is only changing the value for the "Married filing jointly" status for 2019 (assume 2017 start year for the entry page on TaxBrain).

If this is a trivial transformation, then I think what you suggest would be trivial to implement (although I would not be the one doing the work so it's easy for me to say this). I would go further and say that if it is not trivial, I would suggest that Tax-Calculator be modified so that it is trivial to specify such a reform in a JSON reform file. For example, perhaps should be a valid reform specification:

{
  "policy": {
    "_SS_thd50_joint": {"2019": [36000]}
  }
}

So here I'm suggesting that suffixes like _single, _joint, _separate, _hoh, _widow would be legal and that Tax-Calculator would read such parameter values, extract the "true" parameter name and modify the multi-valued parameter as appropriate. If this is all done on the Tax-Calculator side, it would be quite trivial to take the Tax Brain GUI entry and construct such a reform file. The transformation needs to be trivial though, because we also need to be able to do the reverse: populate the TaxBrain GUI based on the generated reform file (this is so we have the same functionality with the "Edit" button as before). I suppose there are two questions then: is it possible right now to specify a JSON reform for the TaxBrain entry I show, and if not, do you perceive that it would be difficult to add such a capability to Tax-Calculator?

martinholmer · 2017-08-08T15:34:40Z

@talumbau said in the discussion of the #596 bug:

Why doesn't TaxBrain translate the information that is changed by a user into a JSON reform file and then send that file to Tax-Calculator (in the same way as the TaxBrain File Upload page does)?

This seems like a good idea. I just have one question: what is the proper JSON reform file for this entry in TaxBrain?

This is the _SS_thd50 parameter, but here the user is only changing the value for the "Married filing jointly" status for 2019 (assume 2017 start year for the entry page on TaxBrain).

If this is a trivial transformation, then I think what you suggest would be trivial to implement (although I would not be the one doing the work so it's easy for me to say this). I would go further and say that if it is not trivial, I would suggest that Tax-Calculator be modified so that it is trivial to specify such a reform in a JSON reform file. For example, perhaps [the following] should be a valid reform specification:
{
 "policy": {
   "_SS_thd50_joint": {"2019": [36000]}
 }
}
So here I'm suggesting that suffixes like _single, _joint, _separate, _hoh, _widow would be legal [in JSON reform files] and that Tax-Calculator would read such parameter values, extract the "true" parameter name and modify the multi-valued parameter as appropriate. If this is all done on the Tax-Calculator side, it would be quite trivial to take the Tax Brain GUI entry and construct such a reform file. The transformation needs to be trivial though, because we also need to be able to do the reverse: populate the TaxBrain GUI based on the generated reform file (this is so we have the same functionality with the "Edit" button as before). I suppose there are two questions then: is it possible right now to specify a JSON reform for the TaxBrain entry I show, and if not, do you perceive that it would be difficult to add such a capability to Tax-Calculator?

@talumbau thanks for your thoughtful and creative response.

I have just started looking into the issues you raise, so these are tentative answers to your two questions. I will continue looking into the issues you raise, but at the moment I'm optimistic about the approach you're suggesting.

The answer to your first question is NO.
Right now the JSON reform file for a MARS-indexed parameter (like _SS_thd50) uses the whole MARS-indexed _SS_thd50 array. And in addition to the 49 MARS-indexed parameters in current_law_policy.json, there are 5 parameters that are EIC-indexed, and 2 parameters that are indexed by idedtype. The remaining 125 parameters are not arrays.

The answer to your second question is YES, I think.
I'm optimistic that the Calculator.read_json_param_files(...) static method can be enhanced to handle JSON reform files with MARS-valued suffixes, with EIC-valued suffixes, and with idedtype-valued suffixes. I think this can be made to be an optional translation feature, so that all the old JSON reform files that use the parameter-array syntax will continue to work as they do now.

@MattHJensen @Amy-Xu @andersonfrailey @hdoupe @GoFroggyRun
@brittainhard @PeterDSteinberg

hdoupe · 2017-09-28T17:21:16Z

Closed via #641

hdoupe changed the title ~~EITC phase-in rate boundaries~~ [BUG] EITC phase-in rate boundaries Jul 28, 2017

MattHJensen added the bug label Jul 28, 2017

brittainhard self-assigned this Jul 28, 2017

martinholmer changed the title ~~[BUG] EITC phase-in rate boundaries~~ EITC phase-in rate boundaries Aug 2, 2017

martinholmer changed the title ~~EITC phase-in rate boundaries~~ EITC phase-in rate values being changed Aug 7, 2017

hdoupe mentioned this issue Aug 11, 2017

Using requests for first iteration of automated webapp testing #615

Closed

martinholmer mentioned this issue Aug 16, 2017

Modify webapp backend to process all user input as files #619

Closed

hdoupe mentioned this issue Aug 17, 2017

1.0.2 Release #624

Closed

brittainhard added this to the 1.0.2 Release milestone Aug 17, 2017

martinholmer mentioned this issue Aug 18, 2017

EITC phase-in rate, "_EITC_rt" PSLmodels/Tax-Calculator#1485

Closed

hdoupe closed this as completed Sep 28, 2017

hdoupe mentioned this issue Oct 2, 2017

Add test for eitc rate greater than 1 #674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EITC phase-in rate values being changed #596

EITC phase-in rate values being changed #596

hdoupe commented Jul 28, 2017

martinholmer commented Aug 4, 2017

hdoupe commented Aug 4, 2017

talumbau commented Aug 4, 2017

martinholmer commented Aug 4, 2017

talumbau commented Aug 4, 2017 •

edited

Loading

martinholmer commented Aug 4, 2017 •

edited

Loading

hdoupe commented Aug 4, 2017

martinholmer commented Aug 4, 2017

hdoupe commented Aug 4, 2017

hdoupe commented Aug 4, 2017

martinholmer commented Aug 4, 2017

hdoupe commented Aug 4, 2017

martinholmer commented Aug 4, 2017

martinholmer commented Aug 4, 2017

talumbau commented Aug 4, 2017

martinholmer commented Aug 7, 2017

talumbau commented Aug 8, 2017

martinholmer commented Aug 8, 2017 •

edited

Loading

hdoupe commented Sep 28, 2017

EITC phase-in rate values being changed #596

EITC phase-in rate values being changed #596

Comments

hdoupe commented Jul 28, 2017

martinholmer commented Aug 4, 2017

hdoupe commented Aug 4, 2017

talumbau commented Aug 4, 2017

martinholmer commented Aug 4, 2017

talumbau commented Aug 4, 2017 • edited Loading

martinholmer commented Aug 4, 2017 • edited Loading

hdoupe commented Aug 4, 2017

martinholmer commented Aug 4, 2017

hdoupe commented Aug 4, 2017

hdoupe commented Aug 4, 2017

martinholmer commented Aug 4, 2017

hdoupe commented Aug 4, 2017

martinholmer commented Aug 4, 2017

martinholmer commented Aug 4, 2017

talumbau commented Aug 4, 2017

martinholmer commented Aug 7, 2017

talumbau commented Aug 8, 2017

martinholmer commented Aug 8, 2017 • edited Loading

hdoupe commented Sep 28, 2017

talumbau commented Aug 4, 2017 •

edited

Loading

martinholmer commented Aug 4, 2017 •

edited

Loading

martinholmer commented Aug 8, 2017 •

edited

Loading