PolicyEngine / PolicyEngine/policyengine-sim-api

Segmented-national follow-ups from the #637 review

Open
#639 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
1
Forks
1
Avg merge
10h 29m
Merged PRs (30d)
7

Description

Remaining items from the PR #637 code review, deliberately deferred:

Efficiency (review finding 9):

  • Lazy-import the reduce chain inside the runner so cold run_simulation containers don't pay ~19s importing pandas/microdf/policyengine.core before routing (regional/UK requests pay it today and never use it)
  • Add the POLICYENGINE_SKIP_COUNTRY_IMPORTS setdefault guard to segmented_national_reduce.py — a bare pytest tests/test_segmented_national*.py process currently imports both country models (~80s) to run pure-mechanics unit tests

Robustness (review finding 10):

  • National requests with a custom data/data_version currently fan out; small custom datasets make every child fail the >100-household assertion where a monolithic run succeeded. Either add custom-data requests to the monolithic fallback, or make the partition dataset-aware.

DRY leftovers (finding 14, partially addressed):

  • The poll/backoff loops in budget_window_scheduler.py and segmented_national.py remain separate (payload building + backoff step now share src/modal/fanout.py); extracting a shared poll-with-backoff helper is still open, as is consolidating the three in-repo Modal fakes (test_budget_window_scheduler.py mocks vs test_segmented_national.py fakes) into a conftest fixture

Validation debt:

  • include_cliffs segmentation: verify cliff-variable reconstruction through the reduce, then lift the monolithic fallback
  • Labor-supply-response segmentation: requires recomputing national household_income_decile at reduce time (the transported column is region-group-relative) and validating against monolithic — see the LSR_PARAMETER_PREFIX comment in segmented_national.py
  • Budget-window children remain pinned segmented=false; enabling nested fan-out (years x 21 containers) needs capacity planning against the 100/300 pools

Related: #638 (deprecating the region:"us" national spelling).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with segmented_national.py and segmented_national_reduce.py, then review budget_window_scheduler.py, src/modal/fanout.py, and the related tests named in the issue. Run the segmented-national tests first and validate each deferred item against monolithic results, custom-data behavior, import time, and pool capacity; the work is done when the listed review and validation gaps are resolved.

Written by the indexing model from the issue text.

Assessment

Tech stack
pandas, python
Domain
backend, data, performance, testing
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.