Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Form Inputs #640

Closed
wants to merge 3 commits into from
Closed

Fix Form Inputs #640

wants to merge 3 commits into from

Conversation

brittainhard
Copy link
Contributor

Closes #638

@martinholmer
Copy link
Contributor

@brittainhard, The following warnings suggest a need for code revision:

`request.REQUEST` is deprecated, use `request.GET` or `request.POST` instead.
    start_year = request.REQUEST.get('start_year')
  /home/travis/build/OpenSourcePolicyCenter/webapp-public/webapp/apps/dynamic/views.py:315: RemovedInDjango19Warning: `request.REQUEST` is deprecated, use `request.GET` or `request.POST` instead.
    fields = dict(request.REQUEST)

and

DynamicElasticitySaveInputs.creation_date received a naive datetime (2015-01-01 00:00:00) while time zone support is active.
    RuntimeWarning)
  /home/travis/miniconda/envs/aei_dropq/lib/python2.7/site-packages/django/db/models/fields/__init__.py:1474: RuntimeWarning: DateTimeField DynamicElasticitySaveInputs.creation_date received a naive datetime (2015-01-01 00:00:00) while time zone support is active.
    RuntimeWarning)

@brittainhard
Copy link
Contributor Author

brittainhard commented Aug 25, 2017

@martinholmer this error effects all reforms from 2013-2016, but beyond 2017 everything it working okay.

I think I zeroed in on the problem. The inputs that fail are the ones that have validations specs in current_law_policy. For example,

    
    "_II_brk4": {
        "long_name": "Personal income (regular/non-AMT/non-pass-through) tax bracket (upper threshold) 4",
        "description": "Income below this threshold and above tax bracket 3 is taxed at tax rate 4.",
        "section_1": "Personal Income",
        "section_2": "Regular: Non-AMT, Non-Pass-Through",
        "irs_ref": "Form 1040, line 44, instruction (Schedule XYZ).",
        "notes": "",
        "row_var": "FLPDYR",
        "row_label": ["2013",
                      "2014",
                      "2015",
                      "2016",
                      "2017"],
        "start_year": 2013,
        "cpi_inflated": true,
        "col_var": "MARS",
        "col_label": ["single", "joint", "separate", "head of household", "widow"],
        "value": [[183250, 223050, 111525, 203150, 223050],
                  [186350, 226850, 113425, 206600, 226850],
                  [189300, 230450, 115225, 209850, 230450],
                  [190150, 231450, 115725, 210800, 231450],
                  [191650, 233350, 116675, 212500, 233350]],
        "validations": {"min": "_II_brk3", "max": "_II_brk5"}
    },

This one has validators that uses other params for comparison. I haven't tried all of these, but these are the ones that are failing consistently. Goof around with this and I think you'll find the same thing I did.

The source of the whole problem is this: the function, get_comp_data, is trying to compare the length of the reform inputs to the defaults, but its getting way more defaults than its expecting. In fact, the number of inputs its getting depends on the year of the reform.

It fails on this line: assert len(comp_data) == len(col_values)

So,
In 2013, len([6100.0, 6200.0, 6300.0, 6300.0, 6350.0]) != len([50000.0]).
In 2014, len([6200.0, 6300.0, 6300.0, 6350.0]) != len([50000.0]).
In 2015, len([6300.0, 6300.0, 6350.0]) != len([50000.0]).

And so on, until you get to 2017, which only has one default to compare to. 2018 also has no problems, and also has one default to compare to.

It looks like its all coming from this clause in _revised_default_data. For any year before 2017, num_values will always be greater than nyrs, and so taxbrain always gets more defaults that it wants.

I don't have experience with this part of the taxcalc code, so I think your insight is essential here. It looks like there is some serious incongruity between taxbrain and taxcalc, and I'm not sure how to tackle it yet.

Let me know what you think.

@martinholmer
Copy link
Contributor

@brittainhard said about the bug reported in issue #640:

I think I zeroed in on the problem. The inputs that fail are the ones that have validations specs in current_law_policy.json.

Please do not spend any time trying to fix the TaxBrain input validation logic. That logic is going to be completely eliminated from TaxBrain as we implement the TaxBrain redesign suggested by @talumbau in issue #619. In particular, see this comment in the discussion of that issue.

Put another was, implementing #619 will make the bug in #640 (and many others as well) go away.

So, the best use of your time is to help @hdoupe implement #619.

@MattHJensen

@brittainhard brittainhard deleted the 638_broken_form_inputs branch September 18, 2017 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants