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

Tax-Calculator's interaction with webapp-public and deploy #1224

Closed
PeterDSteinberg opened this issue Mar 7, 2017 · 10 comments
Closed

Tax-Calculator's interaction with webapp-public and deploy #1224

PeterDSteinberg opened this issue Mar 7, 2017 · 10 comments

Comments

@PeterDSteinberg
Copy link
Contributor

PeterDSteinberg commented Mar 7, 2017

The handling of parameters between webapp-public, deploy and taxcalc is currently in an inconsistent state. Basically, we have always received in the backend celery tasks, dicts like:

{u'2017': {u'_AMEDT_rt': [0.01], u'_ID_BenefitSurtax_Switch': [[True, True, True, True, True, True, True]]}}

webapp-public currently continues to send user_mods to celery workers like that.

The problem is that taxcalc is now strictly enforcing a new dictionary structure here and we get an exception there. The exception is that we do not have the new user_mods structure with keys of:

'policy', 'consumption', 'behavior','growdiff_baseline', 'growdiff_response','gdp_elasticity'

Questions:

  • It appears to me that the dict example above {u'2017':... should be the policy dict and we could assign every other field, such as growdiff_baseline to empty dicts, not sure if that simple fix is accurate and it may mask calculation errors later.
  • Should all the parameters from ospc.org/taxbrain go into the policy dict? It seems we may need to change the usages of package_up_vars in webapp-public to make sure we package the user_mods in the format.
@PeterDSteinberg
Copy link
Contributor Author

@MattHJensen
Copy link
Contributor

MattHJensen commented Mar 7, 2017

@PeterDSteinberg,

My initial assessment is that:

The parameters from the TaxBrain static input page should all go into the policy dict.

The parameters from the TaxBrain macro-elasticity dynamic model should go into the gdp_elasticity dict.

The parameters from the TaxBrain Partial Equilibrium dynamic model should go into the behavior dict.

@PeterDSteinberg
Copy link
Contributor Author

That makes sense to me @MattHJensen. The unused fields will just be {} so we don't get this key missing exception. Does that mean growdiff_baseline, growdiff_response, and consumption are currently not used by the apps - placeholders for future features?

@MattHJensen
Copy link
Contributor

Does that mean growdiff_baseline, growdiff_response, and consumption are currently not used by the apps - placeholders for future features?

Yes and no. They are not currently used by the TaxBrain GUI. But they will be available to taxbrain/file users after ospc-org/ospc.org#500 is resolved.

@PeterDSteinberg
Copy link
Contributor Author

Ok - I'll make changes to deploy based on what you described above, @MattHJensen . Then I'll deploy to test-app in next hour or so and, if all goes well, ospc later today.

@brittainhard
Copy link
Contributor

@PeterDSteinberg are there changes that need to be made in taxbrain? Can we tell, based on the request sent to the flask server, whether the data came from static input or one of the dynamic models?

@PeterDSteinberg
Copy link
Contributor Author

@brittainhard - the backend will know which endpoint is passing the parameters and can make some adjustments. But it seems like we need some way in macro-elasticity and Partial Equilibrium models to differentiate between policy parameters and parameters specific to the elasticity or equilibrium model. Is that correct, @MattHJensen ? It was my impression that since those models are built on taxcalc, that they have parameters that may be considered policy and some that may be either behavior or gdp_elasticity (depending on the endpoint receiving request).

@MattHJensen
Copy link
Contributor

@PeterDSteinberg, that is correct. The policy parameters are coming along from the static input, and then the behavior or gdp_elasticity params are added to those depending on the dynamic modeling case.

@martinholmer
Copy link
Collaborator

Is there likely to be any further discussion of Tax-Calculator issue #1224, Tax-Calculator's interaction with webapp-public and deploy?

If not, should #1224 be closed?

@PeterDSteinberg @MattHJensen

@MattHJensen
Copy link
Contributor

Thanks @martinholmer. I believe this is resolved and am closing. @PeterDSteinberg, please comment if this should be reopened.

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

No branches or pull requests

4 participants