-
-
Notifications
You must be signed in to change notification settings - Fork 159
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
Add UBI to benefits totals #2418
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2418 +/- ##
=======================================
Coverage 99.92% 99.92%
=======================================
Files 13 13
Lines 2554 2560 +6
=======================================
+ Hits 2552 2558 +6
Misses 2 2
Continue to review full report at Codecov.
|
This is ready for review @Peter-Metz @MattHJensen. Note, I'm assuming we don't need a consumption benefits scale parameter for UBI since it's a cash transfer. |
@jdebacker, thanks a lot for the PR. It looks like benefit_value_total_ubi = self.array('ubi') + self.array('benefit_value_total')
self.array('benefit_value_total', benefit_value_total_ubi)
benefit_cost_total_ubi = self.array('ubi') + self.array('benefit_cost_total')
self.array('benefit_cost_total', benefit_cost_total_ubi) This solution feels a little hardcoded, but we wouldn't have to modify Also, to avoid double counting when calculating expanded income, we need to remove this line. Finally, I was looking at the three Tax-Calc tables -- difference, distribution, and diagnostic -- and it looks like the first two have line items for UBI and benefits totals. We might want to consider adding those variables to the diagnostic table as well. |
@Peter-Metz Thanks for those comments. Changing this to a WIP while I resolve those issues you raise. |
@Peter-Metz Can you review and see that my latest commits address the issues you raise about computing UBI for benefits totals and adding to the diagnostic tables. Thanks! |
Also, @Peter-Metz, can you share code you used to find that |
Hey @jdebacker, these latest changes look good to me at first glance, but I'll take a closer look tomorrow. Here's how I tested if from taxcalc import *
ubi_ref = {'UBI_21': {2020: 1000}}
pol = Policy()
recs = Records()
calc_base = Calculator(pol, recs)
calc_base.advance_to_year(2020)
calc_base.calc_all()
pol_ubi = Policy()
pol_ubi.implement_reform(ubi_ref)
calc_ubi = Calculator(pol_ubi, recs)
calc_ubi.advance_to_year(2020)
calc_ubi.calc_all()
(calc_ubi.weighted_total('ubi') - calc_base.weighted_total('ubi'))/1e9
### 241.02410248
(calc_ubi.weighted_total('benefit_cost_total') - calc_base.weighted_total('benefit_cost_total'))/1e9
### 0.0 |
Thanks @Peter-Metz. Just added a test that confirms UBI is included in the benefits totals coming out of |
@jdebacker all looks good to me, thanks again for the PR. I'll leave this open for @MattHJensen to review and merge. |
Thanks a lot for the PR, @jdebacker and review @Peter-Metz. This looks good to me, too. Merging. |
This PR updates the computation of benefits totals to include UBI program benefits.