Apply replacement_rate_adjust to every pension system

[marcelolafleur]

Fixes #1200.

replacement_rate_adjust is a table of multipliers on pension benefits, one per year and income group. It lets a country phase benefits down over time.

It was only read inside SS_amount, so it worked under US-Style Social Security and did nothing at all under Defined Benefits, Notional Defined Contribution and Points System. Setting it under those three changed nothing and gave no warning.

pension_amount now applies it to the other three systems as well.

The only fiddly part is looking up the right year. In the steady state there is one year, so the multiplier is the last row of the table. Along the transition the model computes a block of years at once, and each row of that block is a different year — the block starts at year t, so its first row needs the multiplier for year t, its second row year t + 1, and so on. A small helper does that lookup and returns the multipliers shaped to line up with the benefits array. This is what SS_amount was already doing for US-Style; the helper does the same for the rest.

SS_amount itself is unchanged, so US-Style results cannot move.

With this fix, thame call, with the multiplier halved:

                     mult=1.0    mult=0.5
US-Style              3.60000     1.80000
Defined Benefits      0.92659     0.46329     (before this was 0.92659 as well)

Tests

• test_replacement_rate_adjust_applies_to_every_system : halving the multiplier must halve benefits, under all four systems. Fails on master for DB, NDC and PS.
• test_replacement_rate_adjustment_indexes_by_cohort_year : checks the year lookup: on the transition, row n of the block must get the multiplier for year t + n, and the steady state must get the last row. Using one year's multiplier for the whole block would still pass a test with a flat table, and would quietly cancel out a phase-down. The existing test_pension_amount sets the table to all ones, so it cannot catch this.

test_pensions.py, test_tax.py and test_aggregates.py pass, 122 total.

cc: @jdebacker @rickecon

[codecov-commenter]

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.78%. Comparing base (2b6a668) to head (565074c).

Files with missing lines	Patch %	Lines
ogcore/pensions.py	86.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1201      +/-   ##
==========================================
+ Coverage   72.74%   72.78%   +0.03%     
==========================================
  Files          22       22              
  Lines        5768     5783      +15     
==========================================
+ Hits         4196     4209      +13     
- Misses       1572     1574       +2     

Flag	Coverage Δ
unittests	72.78% <86.66%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
ogcore/pensions.py	69.01% <86.66%> (+0.85%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[arihantlodha-cmd]

Nice, this is a clean fix for a genuinely sneaky bug. A parameter that silently does nothing under three of the four systems, with no warning, is exactly the kind of thing that bites someone quietly.

I'm still newer here so treat this as non-blocking, but I traced the indexing since that's the load-bearing claim. The helper matches SS_amount row for row: SS uses adjust[-1], the 1-D time-path case uses adjust[t], and the 2-D case's adjust[t:t+length] is exactly the t + tt per-cohort rows SS_amount loops over at line 70. So the "mirrors SS_amount" claim holds. And nice that you tested both halves: one test that the adjustment actually applies under every system, and a separate one that pins the per-cohort offset directly, which is the part most likely to hide an off-by-one.

One thing worth making explicit. The helper multiplies the whole pension array, whereas SS_amount only touches pension[retireTPI:]. Those are equivalent only because DB/NDC/PS return zero benefits before retirement, so scaling the zeros is harmless. That holds today, but it's an implicit assumption a future pension formula could break. A one-line comment, or an assert in the test that the pre-retirement entries stay zero, would keep that from quietly going wrong.

Minor: TPI_scalar returns adjust[0, j], the first year. Is row 0 the intended year for the scalar case, or should it track the period being computed like the other branches?

Thanks for catching this one.

[marcelolafleur]

Thanks @arihantlodha-cmd, good catch on the implicit assumption. I added a note to the helper's docstring: scaling the whole array only works because DB, NDC, and PS pay nothing before retirement, and a future system that pays earlier would need the SS_amount treatment.

On TPI_scalar: row 0 is intended. That branch exists for exactly one caller, firstdoughnutring, which handles one special household: the oldest person alive when the transition begins, who has just one period left to live. Their entire remaining problem happens in year 0, by construction, so row 0 is always the right row. It also matches what SS_amount already does for this case.

[arihantlodha-cmd]

thanks @marcelolafleur, the docstring note is exactly the right guard. and the TPI_scalar explanation clicks: if firstdoughnutring's one household has a single period left and lives it entirely in year 0, then row 0 is unambiguous by construction. looks good to me.