Skip to content

Commit 6985143

Browse files
authored
Merge pull request #482 from PolicyEngine/fix-multi-year-run-output-system
Return full outputs for budget-window runs
2 parents 630ee2a + a1428a7 commit 6985143

21 files changed

Lines changed: 868 additions & 303 deletions

.github/scripts/modal-run-integ-tests.sh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
#!/bin/bash
22
# Run simulation integration tests
3-
# Usage: ./modal-run-integ-tests.sh <environment> <base-url> [us-version]
3+
# Usage: ./modal-run-integ-tests.sh <environment> <base-url> [us-version] [uk-version]
44
# Environment: beta runs all tests, prod excludes beta_only tests
55

66
set -euo pipefail
77

88
ENVIRONMENT="${1:?Environment required (beta or prod)}"
99
BASE_URL="${2:?Base URL required}"
1010
US_VERSION="${3:-}"
11+
UK_VERSION="${4:-}"
1112

1213
truthy() {
1314
case "${1:-}" in
@@ -115,6 +116,10 @@ if [ -n "$US_VERSION" ]; then
115116
export simulation_integ_test_us_model_version="$US_VERSION"
116117
fi
117118

119+
if [ -n "$UK_VERSION" ]; then
120+
export simulation_integ_test_uk_model_version="$UK_VERSION"
121+
fi
122+
118123
if [ "$ENVIRONMENT" = "beta" ]; then
119124
echo "Running all simulation integration tests (including beta_only)"
120125
uv run pytest tests/simulation/ -v

.github/workflows/modal-deploy.reusable.yml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ on:
1515
simulation_api_url:
1616
description: 'The deployed simulation API URL'
1717
value: ${{ jobs.deploy.outputs.simulation_api_url }}
18+
us_version:
19+
description: 'The deployed policyengine-us package version'
20+
value: ${{ jobs.deploy.outputs.us_version }}
21+
uk_version:
22+
description: 'The deployed policyengine-uk package version'
23+
value: ${{ jobs.deploy.outputs.uk_version }}
1824

1925
jobs:
2026
deploy:
@@ -24,6 +30,7 @@ jobs:
2430
outputs:
2531
simulation_api_url: ${{ steps.get-url.outputs.simulation_api_url }}
2632
us_version: ${{ steps.versions.outputs.us_version }}
33+
uk_version: ${{ steps.versions.outputs.uk_version }}
2734

2835
steps:
2936
- name: Checkout repo
@@ -114,4 +121,4 @@ jobs:
114121
GATEWAY_AUTH_CLIENT_ID: ${{ secrets.GATEWAY_AUTH_CLIENT_ID }}
115122
GATEWAY_AUTH_CLIENT_SECRET: ${{ secrets.GATEWAY_AUTH_CLIENT_SECRET }}
116123
GATEWAY_AUTH_REQUIRED: ${{ vars.GATEWAY_AUTH_REQUIRED }}
117-
run: .github/scripts/modal-run-integ-tests.sh "${{ inputs.environment }}" "${{ needs.deploy.outputs.simulation_api_url }}" "${{ needs.deploy.outputs.us_version }}"
124+
run: .github/scripts/modal-run-integ-tests.sh "${{ inputs.environment }}" "${{ needs.deploy.outputs.simulation_api_url }}" "${{ needs.deploy.outputs.us_version }}" "${{ needs.deploy.outputs.uk_version }}"

changelog_entry.yaml

Lines changed: 0 additions & 4 deletions
This file was deleted.

projects/policyengine-api-simulation/fixtures/gateway/test_endpoints.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import pytest
44

5+
from tests.fixtures.budget_window_outputs import make_single_year_macro_output
6+
57

68
class MockDict:
79
"""Mock for Modal.Dict to simulate version registry."""
@@ -34,7 +36,12 @@ class MockFunctionCall:
3436

3537
def __init__(self, object_id: str = "mock-job-id-123"):
3638
self.object_id = object_id
37-
self.result = {"budget": {"total": 1000000}}
39+
self.result = make_single_year_macro_output(
40+
tax_revenue_impact=1000000,
41+
state_tax_revenue_impact=0,
42+
benefit_spending_impact=0,
43+
budgetary_impact=1000000,
44+
)
3845
self.error = None
3946
self.running = False
4047
self.__class__.registry[object_id] = self

projects/policyengine-api-simulation/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ dependencies = [
1818
"policyengine-fastapi",
1919
"policyengine==4.4.3",
2020
"policyengine-core>=3.26.1",
21-
"policyengine-uk==2.88.0",
21+
"policyengine-uk==2.88.14",
2222
"policyengine-us==1.690.7",
2323
"tables>=3.10.2",
2424
"modal>=0.73.0",
Lines changed: 70 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
"""Budget-window annual result extraction and aggregation helpers."""
1+
"""Budget-window result validation and aggregation helpers."""
22

33
from __future__ import annotations
44

55
from decimal import Decimal
66
from typing import Any
77

88
from src.modal.gateway.models import (
9-
BudgetWindowAnnualImpact,
109
BudgetWindowResult,
1110
BudgetWindowTotals,
11+
SingleYearMacroOutput,
1212
)
1313

1414
# The UK microsimulation has no state/province fiscal layer, so worker child
@@ -20,7 +20,10 @@
2020
"benefit_spending_impact",
2121
"budgetary_impact",
2222
)
23-
OPTIONAL_BUDGET_KEYS = ("state_tax_revenue_impact",)
23+
24+
25+
def _is_number(value: Any) -> bool:
26+
return isinstance(value, int | float) and not isinstance(value, bool)
2427

2528

2629
def _as_decimal(value: float | int) -> Decimal:
@@ -32,46 +35,60 @@ def _as_decimal(value: float | int) -> Decimal:
3235
return Decimal(str(value))
3336

3437

35-
def extract_annual_impact(
38+
def validate_single_year_output(
3639
*,
3740
simulation_year: str,
3841
child_result: dict[str, Any],
39-
) -> BudgetWindowAnnualImpact:
42+
) -> SingleYearMacroOutput:
43+
"""Validate and normalize a child macro result.
44+
45+
UK worker results can omit ``state_tax_revenue_impact`` because there is
46+
no state/province fiscal layer. The canonical output still includes that
47+
field, defaulted to zero, so downstream clients receive one stable shape.
48+
"""
49+
50+
if not isinstance(child_result, dict):
51+
raise ValueError(
52+
"Malformed budget-window child result: expected object for "
53+
f"{simulation_year}"
54+
)
55+
4056
budget = child_result.get("budget", {})
4157
if not isinstance(budget, dict):
4258
raise ValueError("Malformed budget-window child result: missing budget object")
4359

4460
missing_keys = [
45-
key
46-
for key in REQUIRED_BUDGET_KEYS
47-
if not isinstance(budget.get(key), int | float)
61+
key for key in REQUIRED_BUDGET_KEYS if not _is_number(budget.get(key))
4862
]
4963
if missing_keys:
5064
missing = ", ".join(f"budget.{key}" for key in missing_keys)
5165
raise ValueError(
5266
f"Malformed budget-window child result: missing numeric {missing}"
5367
)
5468

55-
tax_revenue_impact = budget["tax_revenue_impact"]
56-
# UK worker results omit the state fiscal layer entirely; coerce to 0.0
57-
# so the parent aggregator can still report federal/state splits with a
58-
# uniform shape across countries.
59-
state_tax_revenue_impact = budget.get("state_tax_revenue_impact")
60-
if not isinstance(state_tax_revenue_impact, int | float):
61-
state_tax_revenue_impact = 0.0
62-
63-
return BudgetWindowAnnualImpact(
64-
year=simulation_year,
65-
taxRevenueImpact=tax_revenue_impact,
66-
federalTaxRevenueImpact=tax_revenue_impact - state_tax_revenue_impact,
67-
stateTaxRevenueImpact=state_tax_revenue_impact,
68-
benefitSpendingImpact=budget["benefit_spending_impact"],
69-
budgetaryImpact=budget["budgetary_impact"],
70-
)
69+
normalized = dict(child_result)
70+
normalized_budget = dict(budget)
71+
if "state_tax_revenue_impact" not in normalized_budget:
72+
normalized_budget["state_tax_revenue_impact"] = 0.0
73+
elif not _is_number(normalized_budget["state_tax_revenue_impact"]):
74+
raise ValueError(
75+
"Malformed budget-window child result: missing numeric "
76+
"budget.state_tax_revenue_impact"
77+
)
78+
normalized["budget"] = normalized_budget
79+
80+
try:
81+
return SingleYearMacroOutput.model_validate(normalized)
82+
except Exception as exc:
83+
raise ValueError(
84+
f"Malformed budget-window child result for {simulation_year}: {exc}"
85+
) from exc
7186

7287

73-
def sum_annual_impacts(
74-
annual_impacts: list[BudgetWindowAnnualImpact],
88+
def sum_single_year_outputs(
89+
*,
90+
outputs_by_year: dict[str, SingleYearMacroOutput],
91+
years: list[str],
7592
) -> BudgetWindowTotals:
7693
"""Sum per-year impacts using Decimal accumulators.
7794
@@ -93,18 +110,21 @@ def sum_annual_impacts(
93110
"budgetaryImpact": Decimal(0),
94111
}
95112

96-
for annual_impact in annual_impacts:
97-
totals["taxRevenueImpact"] += _as_decimal(annual_impact.taxRevenueImpact)
113+
for year in years:
114+
output = outputs_by_year[year]
115+
budget = output.model_dump(mode="json")["budget"]
116+
tax_revenue_impact = budget["tax_revenue_impact"]
117+
state_tax_revenue_impact = budget.get("state_tax_revenue_impact")
118+
119+
totals["taxRevenueImpact"] += _as_decimal(tax_revenue_impact)
98120
totals["federalTaxRevenueImpact"] += _as_decimal(
99-
annual_impact.federalTaxRevenueImpact
100-
)
101-
totals["stateTaxRevenueImpact"] += _as_decimal(
102-
annual_impact.stateTaxRevenueImpact
121+
tax_revenue_impact - state_tax_revenue_impact
103122
)
123+
totals["stateTaxRevenueImpact"] += _as_decimal(state_tax_revenue_impact)
104124
totals["benefitSpendingImpact"] += _as_decimal(
105-
annual_impact.benefitSpendingImpact
125+
budget["benefit_spending_impact"]
106126
)
107-
totals["budgetaryImpact"] += _as_decimal(annual_impact.budgetaryImpact)
127+
totals["budgetaryImpact"] += _as_decimal(budget["budgetary_impact"])
108128

109129
return BudgetWindowTotals(**{key: float(value) for key, value in totals.items()})
110130

@@ -113,12 +133,25 @@ def build_budget_window_result(
113133
*,
114134
start_year: str,
115135
window_size: int,
116-
annual_impacts: list[BudgetWindowAnnualImpact],
136+
outputs_by_year: dict[str, SingleYearMacroOutput],
117137
) -> BudgetWindowResult:
138+
years = [str(int(start_year) + offset) for offset in range(window_size)]
139+
missing_years = [year for year in years if year not in outputs_by_year]
140+
if missing_years:
141+
raise ValueError(
142+
"Cannot build budget-window result: missing outputs for "
143+
+ ", ".join(missing_years)
144+
)
145+
146+
ordered_outputs = {year: outputs_by_year[year] for year in years}
118147
return BudgetWindowResult(
119148
startYear=start_year,
120149
endYear=str(int(start_year) + window_size - 1),
121150
windowSize=window_size,
122-
annualImpacts=annual_impacts,
123-
totals=sum_annual_impacts(annual_impacts),
151+
years=years,
152+
outputsByYear=ordered_outputs,
153+
totals=sum_single_year_outputs(
154+
outputs_by_year=ordered_outputs,
155+
years=years,
156+
),
124157
)

projects/policyengine-api-simulation/src/modal/budget_window_scheduler.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
)
1515
from src.modal.budget_window_results import (
1616
build_budget_window_result,
17-
extract_annual_impact,
17+
validate_single_year_output,
1818
)
1919
from src.modal.budget_window_state import (
2020
build_batch_status_response,
@@ -167,7 +167,7 @@ def poll_running_children_once(self) -> bool:
167167
return False
168168

169169
try:
170-
annual_impact = extract_annual_impact(
170+
single_year_output = validate_single_year_output(
171171
simulation_year=simulation_year,
172172
child_result=child_result,
173173
)
@@ -189,7 +189,7 @@ def poll_running_children_once(self) -> bool:
189189
mark_child_completed(
190190
self.state,
191191
year=simulation_year,
192-
annual_impact=annual_impact,
192+
single_year_output=single_year_output,
193193
)
194194
put_batch_job_state(self.state)
195195
progress_made = True
@@ -222,15 +222,15 @@ def fail_batch_for_child_error(
222222
put_batch_job_state(self.state)
223223

224224
def complete_batch(self) -> dict[str, Any]:
225-
annual_impacts = [
226-
self.state.partial_annual_impacts[simulation_year]
225+
outputs_by_year = {
226+
simulation_year: self.state.partial_outputs_by_year[simulation_year]
227227
for simulation_year in self.state.years
228-
if simulation_year in self.state.partial_annual_impacts
229-
]
228+
if simulation_year in self.state.partial_outputs_by_year
229+
}
230230
result = build_budget_window_result(
231231
start_year=self.state.start_year,
232232
window_size=self.state.window_size,
233-
annual_impacts=annual_impacts,
233+
outputs_by_year=outputs_by_year,
234234
)
235235
mark_batch_complete(self.state, result=result)
236236
put_batch_job_state(self.state)

projects/policyengine-api-simulation/src/modal/budget_window_state.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99

1010
from src.modal.gateway.models import (
1111
BatchChildJobStatus,
12-
BudgetWindowAnnualImpact,
1312
BudgetWindowBatchRequest,
1413
BudgetWindowBatchState,
1514
BudgetWindowBatchStatusResponse,
1615
BudgetWindowResult,
1716
PolicyEngineBundle,
17+
SingleYearMacroOutput,
1818
)
1919

2020
logger = logging.getLogger(__name__)
@@ -79,7 +79,7 @@ def create_initial_batch_state(
7979
completed_years=[],
8080
failed_years=[],
8181
child_jobs={},
82-
partial_annual_impacts={},
82+
partial_outputs_by_year={},
8383
result=None,
8484
error=None,
8585
created_at=now,
@@ -166,7 +166,7 @@ def mark_child_completed(
166166
state: BudgetWindowBatchState,
167167
*,
168168
year: str,
169-
annual_impact: BudgetWindowAnnualImpact,
169+
single_year_output: SingleYearMacroOutput,
170170
) -> BudgetWindowBatchState:
171171
if year in state.running_years:
172172
state.running_years.remove(year)
@@ -178,7 +178,7 @@ def mark_child_completed(
178178
job_id=child.job_id,
179179
status="complete",
180180
)
181-
state.partial_annual_impacts[year] = annual_impact
181+
state.partial_outputs_by_year[year] = single_year_output
182182
return _touch(state)
183183

184184

0 commit comments

Comments
 (0)