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

Simple GUI-specified reform causes TaxBrain crash #628

Closed
martinholmer opened this issue Aug 18, 2017 · 2 comments
Closed

Simple GUI-specified reform causes TaxBrain crash #628

martinholmer opened this issue Aug 18, 2017 · 2 comments
Labels
Milestone

Comments

@martinholmer
Copy link
Contributor

martinholmer commented Aug 18, 2017

Specifying a simple (delayed) reform on the TaxBrain GUI input page produces a TaxBrain crash.

Here is the only changed policy parameter:

screen shot 2017-08-18 at 2 34 52 pm

And here is the response from TaBrain:

screen shot 2017-08-18 at 2 32 39 pm

@brittainhard @hdoupe @GoFroggyRun

@hdoupe
Copy link
Collaborator

hdoupe commented Aug 18, 2017

@martinholmer That's strange. I ran your reform and the key words to dropq are: u'_FICA_ss_trt': [0.124, 0, 0, 9e+99]

but when I do this reform *,*,*,.15 the key words to dropq are u'_FICA_ss_trt': [0.124, 0.124, 0.124, 0.15] and the reform runs without a problem.

Perhaps there's some issue with validation. I would think that _FICA_ss_trt is expected to be between 0 and 1.

@martinholmer martinholmer added this to the 1.0.2 Release milestone Aug 18, 2017
@martinholmer
Copy link
Contributor Author

@hdoupe said:

That's strange. I ran your reform [see above] and the key words to dropq are:
u'_FICA_ss_trt': [0.124, 0, 0, 9e+99]
but when I do this reform *,*,*,.15 the key words to dropq are:
u'_FICA_ss_trt': [0.124, 0.124, 0.124, 0.15] and the reform runs without a problem.

Perhaps there's some issue with validation. I would think that _FICA_ss_trt is expected to be between 0 and 1.

Ooooh! That was a stupid mistake by me. I meant to specify a pop-the-cap reform in 2020, but instead asked the payroll tax rate to rise to 9e99 beginning in 2020. I put the string *,*,*,9e99 in the payroll tax rate box instead of in the maximum taxable earnings box.

I'll close this issue, but before doing that I want to discuss your findings.

(1) You find that the TaxBrain GUI input logic translates the *,*,*,9e99 string (when start year is 2017) into [0.124, 0, 0, 9e99]. That is clearly incorrect. There is no reason to reduce the payroll tax rate to zero in 2018 and 2019. This is yet another reason to implement the approach proposed by @talumbau in issue #619.

(2) Actually, the _FICA_ss_trt has no validations field in the current_law_policy.json file, so TaxBrain should not be enforcing any validation tests on the value of the parameter. (What is actually going on in the TaxBrain validation logic, I have no idea. This is yet another reason to implement #619: the validation code will be in one place, Tax-Calculator.)

The reason there are no validations to ensure _FICA_ss_trt is between zero and one is a long story. Several years ago there was an extended debate about whether or not the values of policy parameters should be tested for validity. I argued the case that each parameter should have a min/max range, while others argued that only a few parameter values should be checked. I lost that debate and we have only the most minimal validation checking, only for cases where there is a strong logical constraint. One example is that the top of tax bracket 2 must be no more than the top of tax bracket 3.

What is interesting about my mistake is that the error message TaxBrain sent back was completely uninformative. Even now, after I realize what my mistake was, I still don't understand the TaxBrain error message: Length mismatch: Expected axis has 45811 elements, new values have 215525 elements. Even with the absurdly high payroll tax rate, I would think the double-precision arithmetic we are using in Tax-Calculator would simply calculate enormous payroll tax revenue beginning in 2020. Remember double-precision floats can be as big a 1e308, which is a pretty big number, much bigger than the 9e99 tax rate.

In contrast if the payroll tax rate had validation tests with min=0.0 and max=1.0, the TaxBrain error would have been something quite informative, saying simply the the specified payroll tax rate was not in the [0,1] range. And with an error message like this, I would have immediately understood what a stupid mistake I was making.

So, in conclusion, I now see that this is not a bug, but rather Tax-Calculator is working "as designed" to allow an unlimited range of policy parameter values in most cases. Given this, I'm closing issue #628.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants