From 5721abcfa3e3fd5b35ffbd1174b853ed5b36e209 Mon Sep 17 00:00:00 2001 From: Ziming Hua Date: Tue, 1 Sep 2026 16:09:18 -0400 Subject: [PATCH] Document scale-lookup boundary conventions for bracket parameters single_amount lookups are lower-bound inclusive by default, so a source table printed as closed integer bands must be keyed by its wording: band starts with the default calc, or band tops with calc(..., right=True). The right=True form requires a -.inf first threshold, because a value exactly equal to the first threshold otherwise falls below the scale and returns 0. Adds the convention table, the first-threshold trap table, a right=True checklist, the round-before-lookup rule, and a CI-verified example to references/parameters.md; cross-reference bullets in variables.md and tests.md; and routing/trigger updates in SKILL.md. Replaces the 0.0001 threshold-shift guidance for "above X" with right=True + -.inf. Closes #83 Co-Authored-By: Claude Fable 5.1 --- .../policyengine-model-development/SKILL.md | 4 +- .../references/parameters.md | 89 +++++++++++++++++-- .../references/tests.md | 3 + .../references/variables.md | 6 ++ 4 files changed, 92 insertions(+), 10 deletions(-) diff --git a/skills/policyengine-model-development/SKILL.md b/skills/policyengine-model-development/SKILL.md index 303acf6..063b72d 100644 --- a/skills/policyengine-model-development/SKILL.md +++ b/skills/policyengine-model-development/SKILL.md @@ -9,7 +9,7 @@ description: | value_type, definition_period, entity, SPMUnit, TaxUnit, vectorize, where/select, period vs period.this_year, monthly_age, YAML test, absolute_error_margin, contrib reform, in_effect, create_x_reform, neutralize_variable, modify_parameters, changelog.d, StateCode, TANF/SNAP - encoding, bracket parameter, -.inf, single_amount. + encoding, bracket parameter, -.inf, single_amount, right=True, band top, over X but not over Y. NOT for: household calculations, microsimulation, reform scoring, or distributional analysis (use the policyengine skill); calling the REST API (use policyengine-api). metadata: @@ -69,7 +69,7 @@ This skill is the *how-to-write-it* layer. To run calculations or score reforms, | You are working on… | Read | |---|---| | A variable: `value_type`/`entity`/`defined_for`, `adds` vs formula, naming, gotchas (age float, `monthly_age`, `is_ssi_eligible`), federal aggregators | references/variables.md | -| A parameter: YAML structure, brackets (`-.inf`, `.inf`, `single_amount`), breakdowns, metadata, path syntax | references/parameters.md | +| A parameter: YAML structure, brackets (`-.inf`, `.inf`, `single_amount`, `right=True` boundary conventions), breakdowns, metadata, path syntax | references/parameters.md | | Periods (`period` vs `period.this_year`, ÷12) or aggregation (`adds`/`add()`, `.any()`) | references/periods-and-aggregation.md | | Vectorization: `where`/`select`, `max_(A-B,0)` floors, divide-by-zero, phantom values | references/vectorization.md | | YAML tests: structure, period restrictions, error margins, enums, the CLI | references/tests.md | diff --git a/skills/policyengine-model-development/references/parameters.md b/skills/policyengine-model-development/references/parameters.md index dc92d93..dc99187 100644 --- a/skills/policyengine-model-development/references/parameters.md +++ b/skills/policyengine-model-development/references/parameters.md @@ -61,17 +61,90 @@ brackets: Use `0` only where negatives are impossible (ages, counts, resource limits). (Verified: `-.inf` first threshold in `parameters/gov/contrib/harris/rent_relief_act/.../applicable_percentage.yaml`.) -**`single_amount` brackets are "at or above" the threshold.** A value exactly at a threshold gets -that bracket. When a regulation says "**above** X" (X belongs to the *lower* bracket), shift the -threshold by `0.0001`, applied consistently to every threshold in the scale: +### Scale-lookup boundary conventions + +`single_amount` (and `marginal_amount`) scales are looked up with `numpy.digitize`. The default +`.calc(x)` is **lower-bound inclusive**: a value exactly at a threshold lands in the bracket that +*starts* there. When a source table prints closed integer bands ("$0 to 3,000 → $375; 3,001 to +5,000 → $330"), keying the thresholds at the band *tops* (`0, 3_000, 5_000`) therefore +misclassifies the boundary dollar — income of exactly $3,000 gets the 3,001–5,000 band. (NY +IT-214, policyengine-us PR #9313: this flipped an eligibility gate from $375 to $0.) + +**The source wording picks the convention** — the statute, form, or manual, never a program-wide +"keep every table on one convention" preference. Tables keyed faithfully to their own wording +agree at every boundary automatically. + +| Source says | Key thresholds at | Call | First threshold | +|---|---|---|---| +| Band starts: "3,001 to 5,000", "$5,001 or more", "X or above" | band starts `0, 3_001, 5_001` — digit for digit | `.calc(x)` | `0`, or `-.inf` when negatives are valid | +| Band tops: "over 3,000 but not over 5,000", "above 100% FPL", "exceeds $X" | band tops `3_000, 5_000` — digit for digit | `.calc(x, right=True)` at **every** call site | **`-.inf`** (must be negative) | + +**The `right=True` trap: the first threshold must be negative.** With `right=True`, a value +exactly equal to the first threshold falls *below* the scale and returns the zero sentinel. The +conventional first threshold of `0` silently zeroes every household with exactly $0 of the +looked-up income: + +| income | first `0` | first `-1` | first `-.inf` | +|---|---|---|---| +| −100 | 0 | 0 | 375 | +| **0** | **0 (wrong)** | 375 | 375 | +| 1 … 3,000 | 375 | 375 | 375 | +| 3,001 | 330 | 330 | 330 | + +`-.inf` is the defensive choice: it also reproduces the form's "if less than zero, enter 0 → first +band" even when a caller forgets to floor income. `-1` (NYC household credit) is equivalent only +once income has been floored at zero. (Verified 2026-09 against policyengine-core 3.30: +`SingleAmountTaxScale.calc` guards the scale as `[-inf] + thresholds + [inf]` and returns `0` for +anything left of `thresholds[0]`; policyengine-us +`parameters/gov/states/ny/tax/income/credits/real_property_tax/amount/*.yaml` use `-.inf`.) + ```yaml -# "0%: ≤100% FPL; 2%: above 100%–125%; ..." +# Source: "over $3,000 but not over $5,000" → band tops, right=True, -.inf first. +metadata: + type: single_amount brackets: - - threshold: {2024-01-01: 0} ; amount: {2024-01-01: 0} - - threshold: {2024-01-01: 1.0001} ; amount: {2024-01-01: 0.02} # "above 100%" - - threshold: {2024-01-01: 1.2501} ; amount: {2024-01-01: 0.05} # "above 125%" + - threshold: {2025-01-01: -.inf} # not 0 — see the trap above + amount: {2025-01-01: 375} + - threshold: {2025-01-01: 3_000} # band top, verbatim from the source + amount: {2025-01-01: 330} +``` + +**`right=True` checklist:** (1) first threshold `-.inf`; (2) `right=True` at *every* call site of +that parameter — one default call reintroduces the band-top bug; (3) YAML tests at income exactly +`0` and exactly at one band top; (4) `right=` exists only on `single_amount` / `marginal_amount` +scales — `marginal_rate` scales (`MarginalRateTaxScale.calc`) reject it with a `TypeError`, and +don't need it: a marginal rate that starts at $X applies to $0 of income when income is exactly +$X, so band starts and band tops coincide. + +**Round before you look up.** The two conventions disagree on fractional inputs: 3,000.50 stays +in the first band under band-start keying (below 3,001) but moves to the second under +`right=True` (above 3,000). When the form mandates whole dollars ("drop amounts below 50 cents, +increase 50 to 99 cents"), round **half up** with `np.floor(x + 0.5)` *before* the lookup — +`np.round` is half-to-even (3,000.5 → 3,000). Older code emulates "above X" by shifting +thresholds by `0.0001` (`1.0001` for "above 100% FPL"); prefer `right=True` with `-.inf`, which is +exact and needs no epsilon. + + +```python +import numpy as np +from policyengine_core.taxscales import SingleAmountTaxScale + +def scale(thresholds, amounts): + s = SingleAmountTaxScale() + for t, a in zip(thresholds, amounts): + s.add_bracket(t, a) + return s + +x = np.array([-100, 0, 3_000, 3_001]) +# Band-start keying, default calc: exactly 3,000 stays in the first band. +assert scale([0, 3_001], [375, 330]).calc(x).tolist() == [0, 375, 375, 330] +# Band-top keying, right=True, -.inf first: same answers, and negatives hit the first band. +assert scale([-np.inf, 3_000], [375, 330]).calc(x, right=True).tolist() == [375, 375, 375, 330] +# The trap: band-top keying with a first threshold of 0 zeroes income of exactly 0. +assert scale([0, 3_000], [375, 330]).calc(x, right=True).tolist() == [0, 0, 375, 330] +# The original bug: band-top keys with the default call push 3,000 into the next band. +assert scale([0, 3_000], [375, 330]).calc(x).tolist() == [0, 375, 330, 330] ``` -No shift for "at or above X" / "X or more" (already PolicyEngine's default). **Adding a new bracket in a later year: use `.inf` for the base year** so it's structurally present but functionally unreachable until it takes effect. Adding it with only the new date breaks the diff --git a/skills/policyengine-model-development/references/tests.md b/skills/policyengine-model-development/references/tests.md index 178c0b7..84d0020 100644 --- a/skills/policyengine-model-development/references/tests.md +++ b/skills/policyengine-model-development/references/tests.md @@ -116,6 +116,9 @@ Test inputs must be real PolicyEngine variables. Grep the formula for the exact - At least one **positive (non-zero) benefit** case — zero-only tests hide errors that cancel out. - At least one **ineligible** case returning 0/false. - The **exact threshold** edge (income/age/resource). +- For every `single_amount` lookup: a case at **exactly $0** of the looked-up input and one + **exactly at a band top** (e.g. $3,000 for a "$0–3,000" band). These are the two inputs where a + wrong keying convention, or `right=True` over a `0` first threshold, silently returns 0. - A **negative countable-income** case proving the benefit stays capped (guards `max - (-N) = max + N`): ```yaml diff --git a/skills/policyengine-model-development/references/variables.md b/skills/policyengine-model-development/references/variables.md index 201ed15..7a930bf 100644 --- a/skills/policyengine-model-development/references/variables.md +++ b/skills/policyengine-model-development/references/variables.md @@ -107,6 +107,12 @@ places (DRY extraction). eligibility variable. - **Verify boundary operators** against the statute: `<` ("less than") vs `<=` ("at or below"). The wrong one silently misclassifies people exactly at the threshold. +- **Scale lookups have a boundary convention too.** `p.table.calc(x)` is lower-inclusive. A table + keyed at band tops ("over X but not over Y") needs `calc(x, right=True)` at *every* call site + and a `-.inf` first threshold, or inputs exactly at $0 and exactly at a band top silently + return 0. Floor and round the input the way the form does (`max_(income, 0)`, then + `np.floor(income + 0.5)` for whole-dollar forms) *before* the lookup. The source wording fixes + the convention — see references/parameters.md "Scale-lookup boundary conventions". - **Child status uses age, not tax dependency.** For benefit eligibility use `age < 18`, not `is_tax_unit_dependent` (dependents include elderly parents and exclude non-dependent minors). - **Exempting a recipient exempts their income.** If a program exempts SSI recipients, their