Fix salary sacrifice headcount regression from uprating mismatch#270
Merged
vahid-ahmadi merged 2 commits intomainfrom Feb 17, 2026
Merged
Fix salary sacrifice headcount regression from uprating mismatch#270vahid-ahmadi merged 2 commits intomainfrom
vahid-ahmadi merged 2 commits intomainfrom
Conversation
PR #268 added Stage 2 imputation (records at exactly 2k) and headcount calibration targets. The calibrator classified above/below 2k at uprated 2025 prices, but the saved h5 stores 2023 prices where the classification differs (8.55% inflation flips boundary records). Now evaluates SS at base-year prices before thresholding so calibration matches end-user values. Also reduces Stage 2 target to 3M so the calibrator upweights gently rather than overshooting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cords The previous Stage 2 capped all new SS records at 2k, putting them all below-cap. At 2023 prices only ~1,308 QRF records were above-cap, not enough for the calibrator to reach 3.3M. Now donors keep their full employee pension amount so the natural above/below split provides records in both categories. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
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.
Problem
PR #268 introduced a salary sacrifice headcount regression in v1.37.0. The reported numbers shifted from correct (above-cap 3.37M, below-cap 1.12M) to wrong (above-cap 1.89M, below-cap 6.5M).
Root cause: PR #268 added two things simultaneously:
Two bugs:
Uprating mismatch: The calibrator classified above/below 2k at uprated 2025 prices, but the saved h5 stores 2023 prices where 8.55% inflation flips boundary records. PE does not uprate SS when loading, so end-users see the 2023-price classification which disagrees with what the calibrator optimised for.
All Stage 2 records below-cap:
np.minimum(employee_pension, 2000.0)capped every new SS record at 2,000, putting them all at/below the cap. At 2023 prices only ~1,308 QRF records were above-cap - not enough for the calibrator to reach the 3.3M above-cap target.Fix
loss.py: Deflate SS amounts to 2023-24 base-year prices before applying the 2,000 threshold, so the calibrator uses the same above/below classification that end-users will seesalary_sacrifice.py: Stage 2 now moves the full employee pension amount to SS (instead of capping at 2,000), so donors with pension > 2,000 become above-cap records and the rest below-cap. Target set to ~70% of 7.7M total headcount so the calibrator upweights gentlytest_salary_sacrifice_headcount.py: Removexfailmarkers so the tests provide actual signalTest plan