Make SS and TPI output dictionaries expose the same keys - #1185
Open
arihantlodha-cmd wants to merge 1 commit into
Open
Make SS and TPI output dictionaries expose the same keys#1185arihantlodha-cmd wants to merge 1 commit into
arihantlodha-cmd wants to merge 1 commit into
Conversation
Closes PSLmodels#1152. Adds labor_income, capital_income, and income_payroll_taxes to the SS output dict and theta and factor to the TPI output dict, so both dictionaries expose the same 72 keys. Also fixes a latent bug: SS's income_tax_ss was computed via tax.income_tax_liab but passed bqssmat (bequests) as the n (labor supply) argument. The value was previously never used; it is now surfaced as income_payroll_taxes, so the call is corrected to pass nssmat (matching the TPI income_tax_mat calculation).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1185 +/- ##
=========================================
Coverage ? 72.67%
=========================================
Files ? 22
Lines ? 5727
Branches ? 0
=========================================
Hits ? 4162
Misses ? 1565
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1152.
PR #1005 made the key names in the steady-state and transition-path output dictionaries consistent, but the two dictionaries still exposed different sets of keys. This makes the same keys available in both.
Before this PR the differences were:
labor_income,capital_income,income_payroll_taxesfactor,thetaChanges:
SS (
ogcore/SS.py)labor_income(w * n * e) andcapital_income(r_p * b_s) to the output dict, computed the same way as their TPI counterparts.income_payroll_taxesto the output dict, using the already-computedincome_tax_ss.income_tax_sswas computed viatax.income_tax_liab(...)but passedbqssmat(bequests) as then(labor supply) argument. The TPI equivalent (income_tax_mat) correctly passes labor supply.income_tax_sswas previously never read, so this had no effect until now — but it needs to be correct before it is surfaced in the output. Fixed to passnssmat.TPI (
ogcore/TPI.py)thetaandfactorto the output dict (both already in scope). These are constants, so they match their SS counterparts.After the change both dictionaries expose the same 72 keys.
Testing
pytest "tests/test_SS.py::test_SS_solver[Baseline]"andpytest "tests/test_TPI.py::test_run_TPI[Baseline]"both pass, exercising the modified output dictionaries at runtime. These tests iterate over the expected (pickled) keys, so the newly added keys don't affect them and existing values are unchanged.ruff format --check .andruff check .clean.Happy to adjust naming/placement or add regression coverage for the new keys if you'd like.