ospc-org / ospc.org

Source code for PolicyBrain, ospc.org, and related assets.
MIT License
24 stars 32 forks source link

Steps for putting OG-USA back on TaxBrain #539

Open MattHJensen opened 7 years ago

MattHJensen commented 7 years ago

@rickecon, @PeterDSteinberg, @jdebacker, @brittainhard

This is based on recent conversations I've had with Jason, Rick and Peter about getting OG-USA back up on TaxBrain.

Phase 1 is a path to having a single default style of OG-USA on TaxBrain. It would not require any changes to the the TB GUI, and testing will primarily occur on TaxBrain itself from a hidden link.

Phase 2 is a path to running multiple styles of OG-USA on TaxBrain. It will require changes to the TB GUI and more work on testing.

One of the goals here is to involve OG-USA maintainers more heavily in test creation and maintenance.

Please raise flags if any aspect of the approach seems inefficient or misguided!


Phase 1.

    • [ ] Rick and Jason write an example script that displays how to run OG-USA in the way that should be the default for TaxBrain. (unbalanced budget, closed economy). They will check the results locally with latest taxcalc.
    • [ ] Peter/Brittain updates the TaxBrain method for calling OG-USA based on the example script from (1).
    • [ ] The link from the dynamic landing page remains disabled, but Peter gives team members a URL pattern to follow to get to the OLG page on TaxBrain for a particular TaxBrain run number.
    • [ ] Team kicks off example runs from TaxBrain using the hidden URLs.
    • [ ] If we are dissatisfied with results, Jason/Rick update OG-USA. When we are satisfied with results, Brittain reactivates the link from the dynamic landing page.

Phase 2.

  1. DELETED

    • [ ] During (2) - (5), Rick and Jason open a PR with 11 new ogusa/tests tests. 10 test a different configuration of the model to SS, each using the same taxcalc reform, and 1 solves the full model for the same reform. All 11 tests output results tables that are checked into the repository (following the taxcalc's “requires_pufcsv” testing style https://github.com/open-source-economics/Tax-Calculator/blob/master/taxcalc/tests/test_pufcsv.py). All tests should use the latest release of taxcalc. Ideally each configuration is tested in a separate test_[config].py script so that it can be run separately.
    • [ ] If not dealt with already by the PR in 7, Peter adds a commit to the PR from (7) that makes it so that the new tests are only triggered by py.test -m “full-single_reform".
    • [ ] Rick and Jason, with Peter’s help as necessary, refactor the “full” test suite so that each of the SS configurations is tested across several different taxcalc reforms that compare capital vs labor income, inframarginal vs marginal, and close reforms e.g. 25% vs 24.9%. Note that we will also need some bash script or similar to copy about 100 different tables that show differences from "actual" to "expect". Should be triggered by `py.test -m "full-multi-reform"'.
    • [ ] Peter/Brittain, with guidance from Rick/Jason, do #481 and #482.

.... Bonus:

  1. Peter explores new ways to trigger "full-multi-reform" test suite in a remote environment.
  2. Set up code coverage reporting for B-Tax and OG-USA (modeled after use of codecov in tax-calculator)

cc @salimfurth

salimfurth commented 7 years ago

Thanks for kicking this off, @MattHJensen. A concern in the first Phase would be to check that the calibration of OG-USA used matches CBO predictions for debt load, growth rate, and GDP per capita.

The biggest remaining problem with using OG-USA for dynamic scoring is that government debt faces a high real interest rate. We have not yet discussed how to solve that. But without a solution, it leaves the model with a strong anti-deficit bias (on top of being a closed economy, which already implies such a bias).

jdebacker commented 7 years ago

@salimfurth Thanks for these comments. Let me just clarify that this issue is about just how to get OG-USA back on TaxBrain and producing results that are sensible in terms of percentage changes. I think we were fairly happy with these before as they made sense relative to other results we were seeing from the micro simulation model and the macro elasticities model.

As you point out, in the longer run, we'll want to target the model to the levels of various variables forecast by CBO. I view this as a difficult and time consuming exercise and thus my opinion is to delay that until the model is developed much further. However, I'm glad if folks want to step in and add a wedge between interest rates on private capital and public debt or calibrate the current, more simple model to hit CBO targets. But those specific items should be handled under a separate issue, opened in the OG-USA repo.

MattHJensen commented 7 years ago

Step 3 is checked off. The link pattern to kick of OG-USA runs is http://www.ospc.org/dynamic/ogusa/[STATIC_RUN_#]/?start_year=2017

brittainhard commented 7 years ago

@MattHJensen @jdebacker @PeterDSteinberg @salimfurth It should be noted that the links to the simulation results are not /dynamic/(ogusa || behavioral || macro)/<pk>. Rather, they are /dynamic/(results || behavioral_results || macro_results/<pk>. In case there is any confusion.

MattHJensen commented 7 years ago

@jdebacker, @PeterDSteinberg, @rickecon

My first simulation failed:

Hello!

  There was a problem with your simulation and your jobs has failed.
  The tracebrack generated by the failed job is:
  Traceback (most recent call last):
File "/home/ubuntu/miniconda2/envs/aei_dropq/lib/python2.7/site-packages/celery/app/trace.py", line 367, in trace_task
  R = retval = fun(*args, **kwargs)
File "/home/ubuntu/miniconda2/envs/aei_dropq/lib/python2.7/site-packages/celery/app/trace.py", line 622, in __protected_call__
  return self.run(*args, **kwargs)
File "/home/ubuntu/deploy/celery_tasks.py", line 193, in ogusa_async
  guid=guid)
File "/home/ubuntu/deploy/run_ogusa.py", line 48, in run_micro_macro
  runner(**kwargs)
File "/home/ubuntu/miniconda2/envs/aei_dropq/lib/python2.7/site-packages/ogusa/scripts/execute.py", line 48, in runner
  start_year=user_params['start_year'], reform=reform, guid=guid)
File "/home/ubuntu/miniconda2/envs/aei_dropq/lib/python2.7/site-packages/ogusa/txfunc.py", line 1566, in get_tax_func_estimate
  analytical_mtrs, age_specific, reform)
File "/home/ubuntu/miniconda2/envs/aei_dropq/lib/python2.7/site-packages/ogusa/txfunc.py", line 997, in tax_func_estimate
  beg_yr = int(beg_yr)
ValueError: invalid literal for int() with base 10: ‘behavior’

This dynamic simulation has job ID 6927d2a6-c3e2-4204-995b-3f81bbdba3a7 was based on the microsimulation
results found here:
http://www.ospc.org/taxbrain/11369
You used the following macroeconomic model parameters: {u'g_y_annual': '0.03', u'frisch': '0.4'}
Better luck next time!
The TaxBrain Team
jdebacker commented 7 years ago

@PeterDSteinberg Can you help with the OG-USA failure above? It looks like it has to do with the start year (beg_yr) variable being passed from the web-app/TC run. OG-USA is expecting an integer, but it looks like a string is being passed. I don't know enough about the web-app to trace this.

cc @rickecon

PeterDSteinberg commented 7 years ago

@jdebacker and @rickecon - I'll add you as optional invites for a webex meeting tomorrow at 1 pm Eastern with @MattHJensen @brittainhard and @enelson1995 regarding the fixes involved on this issue. @brittainhard has worked on this one more closely.

MattHJensen commented 7 years ago

@PeterDSteinberg, is the og-usa failure above something that we should expect to be a quick fix, or do you think there is some bigger issue involved?

PeterDSteinberg commented 7 years ago

@MattHJensen - I would not expect this to be a quick fix - the bug quoted above is evidence in favor of the one-API approach to deploy and webapp-public repos we discussed (issue #543 ). The quickest fix I would estimate at >6 hours and it would be a temporary solution (temporary in the sense that it would not fully address #543 as an architecture issue). @brittainhard is more familiar with the specifics of this problem.

MattHJensen commented 7 years ago

Ok @PeterDSteinberg. Let's knock off #543 first then.

MattHJensen commented 7 years ago

Based on the principal that upstream repos should not be responsible for testing downstream repos, and based on the fact that taxcalc is now reporting all breaking API changes as part of our release notes, I am going to remove step 6 from phase 2.