Skip to content

Conversation

@vahid-ahmadi
Copy link
Collaborator

Fixes #1090

@vahid-ahmadi vahid-ahmadi self-assigned this Jul 11, 2025
@vahid-ahmadi
Copy link
Collaborator Author

vahid-ahmadi commented Jul 11, 2025

The original issue was that the basic_state_pension formula returned 0 when simulation.dataset was None, preventing the calculation from ever reaching the amount_in_data_year logic where maximum_basic_sp would be applied. I fixed this by modifying the formula to handle the no-dataset case by using the current period for calculations instead of returning 0 early, ensuring that amount_in_data_year is always calculated and properly depends on maximum_basic_sp through the min_(reported, maximum_basic_sp) operation.

@nikhilwoodruff
Copy link
Collaborator

@PolicyEngine can you review this/fix issues and get enough done to get this either over the line, or closed if not worth continuing. Run individual failing tests (check CI) with policyengine-core test [path.yaml] -c policyengine_uk to avoid big memory usage and wait times

@policyengine
Copy link
Contributor

policyengine bot commented Dec 8, 2025

I've fixed the basic state pension calculation. Here's what I changed:

Issues found in the original PR:

  1. Used yoy_growth.triple_lock instead of indices.triple_lock - the former is year-over-year growth rate, the latter is the cumulative index needed for uprating
  2. Non-BASIC pension types returned the reported amount instead of 0 - but basic_state_pension should only return values for BASIC type pensions
  3. Test expectations were incorrect for non-BASIC types

My fix:

  • Allow basic_state_pension to calculate without a dataset (the original code returned 0 immediately if no dataset)
  • Keep returning 0 for non-BASIC pension types (preserves original behavior)
  • Only apply triple lock uprating when using dataset data
  • Use the correct indices.triple_lock cumulative index
  • Fixed test expectation for non-BASIC types to expect 0

⚠️ Branch is significantly outdated:
This branch is from July 2025 and has 18 merge conflicts with master, including unrelated changes to BRMA_LHA_rate.py that cause LHA-related tests to fail (wrong case: lha vs LHA).

Recommendation:

  • If the state pension fix is valuable, merge master into this branch to resolve conflicts, or
  • Create a fresh PR from master with just my changes to basic_state_pension.py and the test file

- Allow basic_state_pension to calculate without a dataset by using
  the current period when no dataset is present
- Keep correct behavior: return 0 for non-BASIC pension types
- Only apply triple lock uprating when using dataset data
- Use correct indices.triple_lock (cumulative index) not yoy_growth
- Fix test expectations for non-BASIC types (should return 0)

Fixes #1090

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@nikhilwoodruff
Copy link
Collaborator

Can you sort the merge conflicts @PolicyEngine

@policyengine
Copy link
Contributor

policyengine bot commented Dec 8, 2025

Resolved all 19 merge conflicts and merged master into this PR branch.

Key resolution decisions:

  • Kept the PR's fix for basic_state_pension.py that allows the calculation to work without datasets (the core fix for Check basic state pension logic #1090)
  • Accepted master's refactored system.py with modular imports
  • Accepted master's updated test values and uprating improvements
  • Accepted all new features from master (salary sacrifice cap, student loans, etc.)

The branch is now up to date with master and should pass CI.

Resolves merge conflicts, keeping:
- PR fix for basic_state_pension to work without datasets
- Master's refactored system.py with modular imports
- Master's updated test values and uprating improvements
- All new features from master (salary sacrifice cap, student loans, etc.)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@nikhilwoodruff nikhilwoodruff merged commit 1de513d into master Dec 8, 2025
2 checks passed
@nikhilwoodruff nikhilwoodruff deleted the fix-state-pension-calculation branch December 8, 2025 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check basic state pension logic

4 participants