ospc-org / ospc.org

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

Add MTR generator #464

Open MattHJensen opened 7 years ago

MattHJensen commented 7 years ago

We should include Marginal Tax Rates (MTRs) in the TaxBrain output and make them available for current law and the reform.

We can display these in a new table builder on the static output page underneath our current table builder.

The table builder options should include:

Note that the functions for generating the percentiles and deciles and various weighting schemes and breakdowns are already available in Tax-Calculator utils.py and used in an extremely similar context in mtr_graph_data.

In fact, if it is equally easy to default to a chart rather than a table and have a "download data option", that would be preferable.

A note on compute.

We can pre-calculate and cache all of the current_law marginal tax rates and various ways of slicing and dicing the data and store all of them in our database. Therefore, the user will be able to see the current law results immediately and choose get instantaneous feedback when choosing other selectors.

However, given that there are many income types, and each one requires rerunning taxcalc several times, I don't think we can/should calculate the reform results at the same time as we are calculating the other static output because it would slow things down too much. Rather, whenever the user choose a new reform result, we should calculate their marginal tax rates for their chosen income type. When they choose another income type, we will have to kick off a new calculation marginal tax rates. We'll probably want a status bar to cover up the table while those calculations are happening.

martinholmer commented 7 years ago

@MattHJensen said:

We should include Marginal Tax Rates (MTRs) in the TaxBrain output and make them available for current law and the reform.

We can display these in a new table builder on the static output page underneath our current table builder.

The table builder options should include: [snip]

Note that the functions for generating the percentiles and deciles and various weighting schemes and breakdowns are already available in Tax-Calculator utils.py and used in an extremely similar context in mtr_graph_data.

In fact, if it is equally easy to default to a chart rather than a table and have a "download data option", that would be preferable.

In order to support these TaxBrain enhancements, we will need to add the bokeh graphing package to the taxcalc conda.recipe. Right now there is a lot of code in utils.py that checks for availability of bokeh and it is difficult to maintain . That code is irrelevant in most cases because the taxcalc-dev environment include bokeh and users of the Tax-Calculator CLI have no problems with the taxcalc package not including bokeh because bokeh is part of the standard Anaconda Python distribution.

Unless I hear cogent reasons for not adding bokeh to the conda.recipe, I'll do that in the near future. Meanwhile, I'm going to drop the pesky code that checks for the availability of bokeh.

@MattHJensen @PeterDSteinberg @brittainhard

talumbau commented 7 years ago

I would suggest that @PeterDSteinberg or @brittainhard attempt to launch the web app on Heroku with bokeh in the required packages list. The package is large and there is often an issue in bumping up against maximum slug size. That's probably the most likely immediate negative impact of forcing all users of taxcalc to have bokeh. On Thu, Jun 8, 2017 at 3:49 PM Martin Holmer notifications@github.com wrote:

@MattHJensen https://github.com/matthjensen said:

We should include Marginal Tax Rates (MTRs) in the TaxBrain output and make them available for current law and the reform.

We can display these in a new table builder on the static output page underneath our current table builder.

The table builder options should include: [snip]

Note that the functions for generating the percentiles and deciles and various weighting schemes and breakdowns are already available in Tax-Calculator utils.py and used in an extremely similar context in mtr_graph_data.

In fact, if it is equally easy to default to a chart rather than a table and have a "download data option", that would be preferable.

In order to support these TaxBrain enhancements, we will need to add the bokeh graphing package to the taxcalc conda.recipe. Right now there is a lot of code in utils.py that checks for availability of bokeh and it is difficult to maintain . That code is irrelevant in most cases because the taxcalc-dev environment include bokeh and users of the Tax-Calculator CLI have no problems with the taxcalc package not including bokeh because bokeh is part of the standard Anaconda Python distribution.

Unless I hear cogent reasons for not adding bokeh to the conda.recipe, I'll do that in the near future. Meanwhile, I'm going to drop the pesky code that checks for the availability of bokeh.

@MattHJensen https://github.com/matthjensen @PeterDSteinberg https://github.com/peterdsteinberg @brittainhard https://github.com/brittainhard

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/OpenSourcePolicyCenter/webapp-public/issues/464#issuecomment-307248359, or mute the thread https://github.com/notifications/unsubscribe-auth/ABzcWoTGnKLYrjhfrHWE0PtZTABo_WIfks5sCHp0gaJpZM4Ly6Pv .

martinholmer commented 7 years ago

@talumbau said:

I would suggest that @PeterDSteinberg or @brittainhard attempt to launch the web app on Heroku with bokeh in the required packages list. The package is large and there is often an issue in bumping up against maximum slug size. That's probably the most likely immediate negative impact of forcing all users of taxcalc to have bokeh.

I have no idea what slug size means ... can you explain?

I have checked the size of the tar.bz2 files that contain pandas and bokeh. I find that the bokeh file is half the size of the pandas file. If we can upload pandas to Heroku why could we not upload bokeh to Heroku?

talumbau commented 7 years ago

https://devcenter.heroku.com/articles/slug-compiler

martinholmer commented 7 years ago

@talumbau said:

https://devcenter.heroku.com/articles/slug-compiler

Thanks for the pointer to the Heroku documentation. In part, it says this:

Your slug size is displayed at the end of a successful compile after the Compressing message. The maximum allowed slug size (after compression) is 500 MB.

You didn't bother to say how big the taxcalc slug size is now without the bokeh package included. When I install the latest bokeh package, I see this:

iMac2:pkgs mrh$ ls -l bokeh*
-rw-r--r--  1 mrh  staff  4295645 Jun  8 21:52 bokeh-0.12.5-py27_1.tar.bz2

So, bokeh compressed is about 4 megabytes. How close are we to the 500 megabyte limit?

@MattHJensen @PeterDSteinberg @brittainhard

martinholmer commented 7 years ago

@PeterDSteinberg, Can you tell us the current slug size of the taxcalc package uploaded to Heroku? This question has come up in webapp-public issue #464, but nobody has provided an answer.

@MattHJensen

martinholmer commented 7 years ago

@martinholmer asked five days ago:

@PeterDSteinberg, Can you tell us the current slug size of the taxcalc package uploaded to Heroku? This question has come up in webapp-public issue #464, but nobody has provided an answer.

I'm interpreting no answer to mean that the current Heroku slug size for the taxcalc package is substantially below the 500 megabyte maximum, and therefore, there is no problem adding the bokeh package to the taxcalc conda.recipe.

@MattHJensen @talumbau @PeterDSteinberg @brittainhard

hdoupe commented 6 years ago

@martinholmer asked

Can you tell us the current slug size of the taxcalc package uploaded to Heroku?

The current slug size is somewhere around 380 MB. Sorry for the delay in responding to this.