new PUF files#2925
Conversation
merge 5.0.0
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2925 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 13
Lines 2662 2662
=========================================
Hits 2662 2662
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Recently noticed that Tax-Data producing a different Therefore, the corresponding weights and ratios files need to be updated to match the new I have shared this with @jdebacker , who confirmed this on his local machine. I am still in investigation on this issue. ~ to see if this issue only exists on our two machines or we should make an update in the model. @martinholmer If you want to have a check on your machine: Secondly, (1) run 'make all' in Tax-Data to produce new weights and ratios files, (2) Update these files into Tax-Calc and run tax liability calculation. --> This should produce a correct result |
|
note: I will proceed with this PR, after the implementation of OBBB into the model. This is to make it clear that 5.1 & 5.2 reflect OBBBA, then 5.3 will reflect this PR to update the PUF files. |
|
@bodiyang, Can you or Jason make available the new |
|
@martinholmer Do you have 2011 raw puf? if so, can run taxdata to produce |
|
@martinholmer Was doing some additional checks locally to reconfirm that the old and new PUF file tax liability calculations are the same. I will merge the new updates into this PR. Then it will be ready for review @jdebacker In next version 5.3.0 release info, would recommend to mark out that PUF users are required to produce new correct usage: |
|
all tests pass |
|
Changes seem reasonable to me. I was unable to use files generated from TaxData PR #452 and pass the test here. But it could be something I did wrong in the process of creating the new weights and ratios files. @martinholmer it sounds like you can confirm the results here. If so, please feel free to merge this PR. |
|
@jdebacker said in his review of PR #2925:
Let's start with the final statement. No, I cannot "confirm the results here" because, as the above discussion shows, I did not check that I could produce the same results as @bodiyang gets in TaxData PR PSLmodels/taxdata#452. All I did is get the new Until at least two people can independently generate on their computers the results in TaxData PR PSLmodels/taxdata#452, then I think this Tax-Calculator PR is premature and should be closed. @jdebacker, I don't agree with your statement that these "Changes seem reasonable to me" because the reason for TaxData PR PSLmodels/taxdata#452 is a mystery (as the discussion in 452 makes clear). You and @bodiyang getting different TaxData results on your computers suggests that something is amiss with your local TaxData installations, or that you used different |
|
@martinholmer I had meetings with @jdebacker and both of us can confirm the necessity of taxdata PR 452. Reason is as documented in this conversation. Tax-Data needs to be updated in order to produce correct PUF files. Then Tax-Calc will be updated to reflect such changes. Will Zoom meet with @jdebacker at a later time to discuss the issue met when producing PUF files. |
|
@bodiyang said in closed PR #2925:
But in that discussion you say:
My point is that it is not good practice to undertake changes without knowing why those changes are necessary. |
I see, probably will expand a bit more on the last comment. In a brief word, it is confirmed that this problem exists and it's necessary to be fixed (agreed by @jdebacker). While the reason for this problem is not certain (I have some speculations). (1) Current version of Tax-Data does not correctly produce PUF files. ( If we use the This mistake needs to be fixed by taxdata PR 452 and taxcalc PR 2925 (2) For the reason of this problem, it is hard to check, because the related code to produce My speculation is that there have been some changes in the software packages Tax-Data relies on (it might come from the packages related to optimization functions). The order of tax unit in |
I suggest that the two of you should discuss another issue as well. Open PR #2538 wants to eliminate the presence of all PUF-related files from the Tax-Calculator repository. When the TMD data became first available, @jdebacker pointed to PR #2538 saying I should not include any TMD-related files in the Tax-Calculator repository. So that is what I did: there is a small amount of TMD-related code in Tax-Calculator that makes it easier for users who want to use TMD data, but there are no TMD data files in the Tax-Calculator repository. So, I think the two of you should discuss whether or not the same approach should be taken with PUF-related data files (that is, remove all the PUF-related data files from the Tax-Calculator repository). In other words, quit working on PR #2925 and finish PR #2538. My personal opinion is that there is real value in including the publicly-available CPS data files in the Tax-Calculator repository. |
|
@bodiyang I am not able to replicate your results here. I appreciate @martinholmer's comments regarding the relation between this PR and PR #2538. I am in agreement with him that it would be useful to keep the publicly available CPS, but remove the PUF files from Tax-Calculator. But my suggestion is we do that in a future PR. Let's get these files updated so users get correct results. Then we can refactor the TaxData package to output the necessary files in a single location (like the TMD package). At that point, let's come back to Tax-Calculator and remove PUF-related files. |
This PR updates
puf_weights.csv.gzandpuf_ratios.csvfiles.The
puf.csvfile produced by thetaxdatarepository needs to be matched with the corresponding puf weights and ratios files.@jdebacker