diff --git a/.coder/ledger/323-ethiopia-config.md b/.coder/ledger/323-ethiopia-config.md new file mode 100644 index 000000000..aa5faa172 --- /dev/null +++ b/.coder/ledger/323-ethiopia-config.md @@ -0,0 +1,232 @@ +# Prior-Art Ledger — GH #323 (Ethiopia config) / unblocking PR #627 + +**Search tier used:** ripgrep + git floor (gitnexus not consulted). The task is +config-tree only; the blast radius was established directly, by rebuilding every +Ethiopia table before and after against PR #627's core. + +## §1 Task, restated + +PR **#627** closes site 4 of GH #323 — the `dfs:` outer merge in +`Wave.grab_data` that *manufactures* the duplicate rows every other site then +collapses. It also converts the GH #515 swallowed `KeyError` into a hard +`RuntimeError` when a dropped sub-frame costs a **required** declared column. +#627's body lists **Ethiopia — 3 cells** as an unfixed blocker. + +Two questions, therefore: + +1. Does `Country('Ethiopia')` build under #627's core against the config now on + `development`? (The blocker table in #627's body was written 2026-07-13 and + has not been refreshed since.) +2. Are #627's two Ethiopian **cartesian** cells — 2013-14 (60,221 phantom rows) + and 2015-16 (52,832) — still present? + +**Answer to both: Ethiopia is clear.** Commit `3488b791` (PR #628) landed the +`cluster_features` fix for all five waves and is an ancestor of +`origin/development`. It closes the 3 raising cells *and* both cartesian cells — +the same re-key does both, which is why #627 could not see it coming from its +own census. **No config change was needed for #627.** This branch therefore +carries the verification (tests + this ledger) rather than a re-implementation. + +Separately, the verification surfaced **one genuinely unfixed defect** in the +same country — `individual_education` in 2013-14 / 2015-16 — which is *not* a +#627 blocker but is a #323 site-1 true positive of the same shape (a broken +identifier). It is fixed here. + +## §2 Existing machinery (this task's area) + +| symbol | path:line | what it does | tested? | reuse / extend / new | +|--------|-----------|--------------|---------|----------------------| +| `Wave._cartesian_keys` | `country.py:909` (#627 only) | exact many-to-many detection: key values duplicated in **both** sub-frames | yes (#627, 16 tests) | **reuse** — the oracle for the before/after census | +| `Wave._merge_subframes` | `country.py:945` (#627 only) | warns on a cartesian with an exact phantom count; fatal under `LSMS_GRAIN_STRICT=1` | yes | **reuse** | +| `_required_scheme_columns` | `country.py:478` (#627 only) | required vs `optional:` scheme columns; drives the #515 raise | yes | **reuse** | +| `Wave.cluster_features` | `country.py:1373` | GH #161 projection of a household-grain frame onto `(t, v)`; fires **only when `i` is an index level** | yes | **reuse** — this is why `3488b791` puts `i` in `final_index` rather than `drop:`-ing it | +| `_normalize_dataframe_index` | `country.py` | collapses a non-unique **declared** index with `groupby().first()`; audits first (`_audit_index_collapse`) | yes | **reuse as the oracle** — its own warning text quantified the education loss | +| `_join_v_from_sample` | `country.py` | joins `sample.v` at API time | yes | untouched | +| `id_walk` / `Country.panel_ids` | `local_tools.py` / `country.py:3359` | walks wave-native ids back to the panel baseline | yes | untouched — but see §4, it is order-dependent | +| `3488b791` (PR #628) | `countries/Ethiopia/{wave}/_/data_info.yml` | the landed `cluster_features` fix | **not until now** | **verified here, not re-implemented** | + +## §3 Definitions & conventions in force + +Cited, not paraphrased: + +- **D1 — the core never aggregates.** `SkunkWorks/grain_aggregation_policy.org` + §3a; restated in `CLAUDE.md` §"Grain Collapse". The `aggregation:` key in + `data_scheme.yml` is **dead config** and is deliberately not honoured. A test + in this branch pins that Ethiopia declares none. +- **Duplicates on a declared index mean the IDENTIFIER IS BROKEN or a LEVEL IS + MISSING — fix the index, do not declare a reducer.** `CLAUDE.md` §"Grain + Collapse". This is the whole argument for the `individual_education` fix. +- `cluster_features` canonical index is `(t, v)`; + `individual_education` is `(t, i, pid)` — `countries/Ethiopia/_/data_scheme.yml`. +- **Ethiopia's wave-keyed ID scheme**: `household_id` / `ea_id` in W1/W4/W5; + `household_id2` / `ea_id2` in W2/W3 — `countries/Ethiopia/_/CONTENTS.org`. +- Ethiopia is a PP/PH country (19 script-path tables), but **not at these + sites**: `cluster_features` and `individual_education` both read the + *household* cover/section files (`sect_cover_hh_w*`, `sect2_hh_w*`), not a + `_pp_`/`_ph_` file, and Ethiopia declares no `wave_folder_map`. Verified + per-`t`: one round per wave here. + +## §4 Invariants & assumptions + +- **The re-key must be `household_id2` in W2/W3, never `household_id`.** + `household_id` is the W1 baseline id and is **blank** for households with no + W1 antecedent, so it is non-unique on the empty value — re-keying to it trades + an EA cartesian for a **null-key** cartesian, because `pd.merge` matches null + keys. Measured: `household_id2` is unique and blank-free in both + `cluster_features` sub-frames (5,262 / 5,287 rows, 0 keys duplicated in both). +- **`i` must reach `final_index`, not be `drop:`-ed.** `Wave.cluster_features`' + GH #161 collapse to the cluster grain only fires when `i` is an index *level*. + `3488b791` gets this right; the abandoned `fix/323-ethiopia` branch did not — + it used `final_index: [t, v]` + `drop: [i]` and leaned on an `aggregation:` + key that nothing reads, leaving the frame household-grain on a non-unique + `(t, v)` for site 1 to `first()`. +- **Wave-level assertions only, for row counts.** `_normalize_dataframe_index` + makes the API index unique **by construction**, so a post-collapse uniqueness + assertion passes with the bug fully present. (Inherited instrument note from + `tests/test_gh323_benin_togo.py` / the CotedIvoire tests.) +- **`individual_education`'s country-level row count is ORDER-DEPENDENT and must + not be asserted on.** `id_walk` is applied only once `panel_ids` has resolved, + and whether it has depends on cache state and on what else the process built + first. Reproduced against the *unchanged* `development` config: + + | invocation (same process, same cache) | rows | + |---|---| + | `Country('Ethiopia').individual_education()` | 63,139 | + | `c.panel_ids` first, then the same call | 62,939 | + + The 200-row delta lands on **2011-12 / 2015-16 / 2018-19 / 2021-22** — waves + this branch does not touch — and reproduces identically in the pre-fix config + tree. It is **pre-existing and orthogonal**; recorded here so the next person + does not mistake it for this change. See §6. +- A **stale L2-wave parquet shadows the #627 raise.** Observed while running the + negative control: three Ethiopian waves that raise on a genuinely cold build + returned a cached `{wave}/_/cluster_features.parquet` and passed. `#627`'s own + warning ("the suite is green and that green is a lie") applies to any + data-backed test here; the config-level assertions in + `tests/test_gh323_ethiopia_config.py` are the ones that cannot be shadowed. + +## §5 Reuse decision + +| quantity | decision | reason | +|----------|----------|--------| +| Ethiopia `cluster_features` config | **reuse `3488b791` unchanged** | Already on `development`, already correct, and verified here end to end. Re-implementing would layer a second differently-shaped fix on a solved problem. | +| the cartesian census | **reuse #627's `_merge_subframes` warning** | It emits an exact phantom count; no separate oracle needed. | +| the education loss measurement | **reuse `_audit_index_collapse`'s warning** | The framework already reports destroyed and NaN-key-deleted rows by wave. Writing a bespoke counter would have been a second, unvalidated implementation of the same arithmetic. | +| `individual_education` W2/W3 index | **new (config)** — `household_id2` / `individual_id2` | Per §3: fix the identifier, never declare a reducer. No existing mechanism repairs a broken id. | +| `interview_date` W4/W5 datetime coercion | **rejected** | `fix/323-ethiopia` added `mapping.py` `interview_date` hooks. Measured on `development`: the table already returns `datetime64[us]`, 8,236 rows, 0 nulls — `_enforce_canonical_dtypes` honours the declared `Int_t: datetime` at API time. The hooks are now redundant; adding them would be inert code that reads as a fix. | +| the `2013-14` wave-level `panel_ids:` block | **deferred, not touched** | `fix/323-ethiopia` deleted it as a wrong-source (livestock-cover, holder-grain) duplicate. On `development` it now reads `i: household_id2` and the country-level `_/panel_ids.py` (`materialize: make`) is authoritative anyway. Out of #627's path; see §6. | + +## §6 Open questions for the human + +- **`individual_education`'s country-level row count is order-dependent** (§4). + The proximate cause is the lazy `self.updated_ids` probe in + `_aggregate_wave_data` (`country.py:2668`), whose `except (FileNotFoundError, + KeyError, ValueError): pass` means a first-attempt failure is recorded as + "attempted" and `id_walk` is then skipped for the rest of the process. It + affects every Ethiopian wave, in both the old and the new config, so it is not + this branch's to fix — but it means **the same call can return two different + row counts depending on what the process touched first**, which is a + correctness problem, not a performance one. Worth its own issue. +- **Ethiopia's 3 raising cells were, in #627's words, "true positives".** They + were — but they had already been fixed when #627's table was written. The + general lesson is #627's own: *a stale blocker table is a stale cache*. Nothing + in the repo re-derives that table; it is prose. +- **The 2013-14 wave-level `panel_ids:` block still points at + `sect_cover_ls_w2.dta`**, the livestock-section cover, whose grain is the + agricultural **holder** (3,812 rows / 3,670 households) and which covers only + 3,670 of the wave's 5,262 households. `data_scheme.yml` declares `panel_ids` + as `materialize: make`, so the country-level script wins and the block is + vestigial — but it is a wrong-source claim sitting in config, and it should + either be deleted or shown to be load-bearing. Not touched here: it is outside + #627's path and deleting config on a "probably unused" basis is exactly the + unevidenced move `CLAUDE.md` forbids. + +--- +### Phase 3 — verification + +- `2013-14/_/data_info.yml` + `2015-16/_/data_info.yml` (`individual_education` + idxvars) — **OK (anchored on §3/§4)**: fixes the *identifier*, per the standing + rule, rather than declaring a reducer; uses the same wave-native ids + `household_roster` and `shocks` already use for those waves, so panel linkage + via `id_walk` + `panel_ids` is unchanged. +- `tests/test_gh323_ethiopia_config.py` — **OK (anchored on §4)**: asserts at the + **wave** level, upstream of `_normalize_dataframe_index`, and deliberately does + **not** assert on post-collapse index uniqueness or on the order-dependent + country-level row count. +- `test_no_dead_aggregation_key_in_ethiopia_config` — **OK (anchored on §3)**: + pins D1 for this country, so the abandoned branch's `aggregation:` keys cannot + be revived by a future salvage attempt. +- Ethiopia `cluster_features` config — **REINVENTION AVOIDED (§5)**: the fix + exists on `development` as `3488b791`; this branch verifies it and changes + nothing. +- `interview_date` `mapping.py` hooks — **REINVENTION AVOIDED (§5)**: measured + redundant against `_enforce_canonical_dtypes`, so not salvaged. + +--- +### Measurements + +All against **PR #627's core** (`origin/development` merged with +`origin/fix/323-site4-dfs-merge`, `git merge-tree` → `59b34cc2`), asserted at +runtime via `lsms_library.__file__`, with an **isolated `LSMS_DATA_DIR`** whose +only pre-populated tier is a symlinked `dvc-cache`. + +**Negative control** — pre-`3488b791` Ethiopia config, same core: + +| wave | result | cartesian cells | phantom rows | +|---|---|---|---| +| 2011-12 | **RuntimeError** — `df_geo` lacks `lat_dd_mod` (file has `LAT_DD_MOD`) | 0 | 0 | +| 2013-14 | 65,508 rows | 1 | **60,221** | +| 2015-16 | 57,786 rows | 1 | **52,832** | +| 2018-19 | **RuntimeError** — `df_geo` lacks `lat_dd_mod` (file has `lat_mod`) | 0 | 0 | +| 2021-22 | **RuntimeError** — geo file has no `ea_id` at all | 0 | 0 | + +Reproduces #627's census exactly: 3 raising cells, 2 cartesian cells, +**113,053** phantom rows. + +**Current `development` config, same core:** + +| wave | result | cartesian cells | phantom rows | +|---|---|---|---| +| 2011-12 | 3,969 rows | 0 | 0 | +| 2013-14 | 5,262 rows | 0 | 0 | +| 2015-16 | 4,954 rows | 0 | 0 | +| 2018-19 | 6,770 rows | 0 | 0 | +| 2021-22 | 4,959 rows | 0 | 0 | + +`Country('Ethiopia').cluster_features()` → **2,168 rows on a unique `(t, v)`**, +2,146 of them carrying Latitude. Per wave: 333 / 433 / 432 / 535 / 435 clusters. + +**Full-country sweep, all 25 declared tables, same core:** `raised: 0`, +`cartesian: 0`, `dropped sub-dfs: 0`. + +**#627's own test file** (`tests/test_gh323_site4_dfs_merge.py`, 16 tests) passes +against this config tree. + +**`individual_education`**, identical conditions (`LSMS_NO_CACHE=1`, fresh +process, same script) before → after: + +| | before | after | +|---|---|---| +| country rows | 59,092 | **63,181** (+4,089) | +| 2013-14 wave frame, unique `(t, i, pid)`? | **No** | Yes (23,785 / 23,785) | +| 2015-16 wave frame, unique `(t, i, pid)`? | **No** | Yes (23,393 / 23,393) | +| `GrainCollapseWarning`s | 2 | **0** | +| W2 `(i, pid)` pairs joinable to `household_roster` | **0 / 8,505** | 12,583 / 12,583 | +| W3 `(i, pid)` pairs joinable to `household_roster` | **0 / 12,597** | 12,599 / 12,599 | + +The framework's own audit quantified the loss before the fix: + +> `Ethiopia/individual_education/2013-14: declared index (t, i, pid) is NOT +> UNIQUE. Collapsing it with groupby().first() DESTROYED 5,247 of 23,785 rows +> whose values DISAGREE (1 conflicting index tuples). ... Additionally 5,248 +> row(s) carry NaN in a declared index level and are DELETED OUTRIGHT` + +> `Ethiopia/individual_education/2015-16: ... DESTROYED 5 of 23,393 rows whose +> values DISAGREE (5 conflicting index tuples).` + +**Test negative control:** `tests/test_gh323_ethiopia_config.py` — **21/21 pass** +against the current config; **17/21 fail** against the pre-`3488b791` config tree +on the same core. (Of the 4 that still passed, 3 were the data-backed +`cluster_features` cases in waves whose raise was shadowed by a stale L2-wave +parquet — see §4; the config-level assertions failed 5/5 in every wave and cannot +be shadowed.) diff --git a/lsms_library/countries/Ethiopia/2013-14/_/data_info.yml b/lsms_library/countries/Ethiopia/2013-14/_/data_info.yml index df3f80228..b18cf33f8 100644 --- a/lsms_library/countries/Ethiopia/2013-14/_/data_info.yml +++ b/lsms_library/countries/Ethiopia/2013-14/_/data_info.yml @@ -100,11 +100,31 @@ household_roster: Relationship: hh_s1q02 MonthsAway: hh_s1q05 +# individual_education (GH #323). The ids MUST be the W2-native +# household_id2 / individual_id2 -- the same pair household_roster and shocks +# already use for this wave -- NOT the W1-baseline household_id / +# individual_id. +# +# household_id/individual_id is the W1 BASELINE identity and is simply BLANK +# for the W2 urban refreshment sample, which has no W1 antecedent: 5,248 of +# sect2_hh_w2.dta's 23,785 rows carry a blank household_id AND a blank +# individual_id. Every one of them lands on the SAME declared index tuple +# ('2013-14', '', ''), and the framework's groupby().first() collapse then +# keeps one and DESTROYS 5,247 real people. Measured on the raw file: +# (household_id, individual_id) is 18,538 unique of 23,785 rows; +# (household_id2, individual_id2) is 23,785 of 23,785, with zero blanks. +# +# It was also breaking the join to household_roster, whose pid for this wave +# IS individual_id2: BEFORE this change, 0 of the 8,505 built (i, pid) pairs +# for t=2013-14 matched a roster row, while W1/W4/W5 -- which use the +# wave-native ids -- matched 100%. Panel linkage is unaffected: id_walk plus +# the existing panel_ids chain still maps household_id2 back to the W1 +# baseline, exactly as household_roster already relies on. individual_education: file: sect2_hh_w2.dta idxvars: - i: household_id - pid: individual_id + i: household_id2 + pid: individual_id2 myvars: # Highest grade completed (free-text survey labels). Educational Attainment: diff --git a/lsms_library/countries/Ethiopia/2015-16/_/data_info.yml b/lsms_library/countries/Ethiopia/2015-16/_/data_info.yml index 6dc44849e..3a9f16029 100644 --- a/lsms_library/countries/Ethiopia/2015-16/_/data_info.yml +++ b/lsms_library/countries/Ethiopia/2015-16/_/data_info.yml @@ -101,11 +101,35 @@ household_roster: Relationship: hh_s1q02 MonthsAway: hh_s1q05 +# individual_education (GH #323). The ids MUST be the W3-native +# household_id2 / individual_id2 -- the same pair household_roster and shocks +# already use for this wave -- NOT the W1-baseline household_id / +# individual_id. +# +# household_id is the W1 BASELINE identity, and a household that SPLIT OFF +# between waves inherits its parent's baseline id: W3 households +# 010501020100105031 and 010501088800105031 (the `8888` marks the split-off) +# both carry baseline household_id 01050100105031. So (household_id, +# individual_id) is 23,385 unique of 23,393 rows -- 16 rows in 8 colliding +# pairs -- and the framework's groupby().first() collapse then MERGES TWO REAL, +# DISTINCT PEOPLE. They disagree: baseline individual 0105010010503101 is +# '10th Grade Complete' in one household and '9th Grade Complete' in the other; +# ...3105 is '3rd Grade Complete' vs '7th Grade Complete'. first() publishes +# one and destroys the other. (household_id2, individual_id2) is 23,393 of +# 23,393 unique. +# +# It was also breaking the join to household_roster, whose pid for this wave +# IS individual_id2: BEFORE this change, 0 of the 12,633 built (i, pid) pairs +# for t=2015-16 matched a roster row (and 1,231 of the 4,283 education +# "households" existed nowhere in the roster), while W1/W4/W5 -- which use the +# wave-native ids -- matched 100%. Panel linkage is unaffected: id_walk plus +# the existing panel_ids chain still maps household_id2 back to the W1 +# baseline, exactly as household_roster already relies on. individual_education: file: sect2_hh_w3.dta idxvars: - i: household_id - pid: individual_id + i: household_id2 + pid: individual_id2 myvars: # Highest grade completed (free-text survey labels). Educational Attainment: diff --git a/lsms_library/countries/Ethiopia/_/CONTENTS.org b/lsms_library/countries/Ethiopia/_/CONTENTS.org index 49459af13..9d55d26bb 100644 --- a/lsms_library/countries/Ethiopia/_/CONTENTS.org +++ b/lsms_library/countries/Ethiopia/_/CONTENTS.org @@ -480,10 +480,43 @@ Every one of these files is HOUSEHOLD-grain, not EA-grain. So =cluster_features= merges the cover page to the geo file on the HOUSEHOLD id (1:1; unique and blank-free in both frames in all five waves) and lets =Wave.cluster_features='s GH #161 collapse reduce to -the =(t, v)= EA grain -- =mean()= for Latitude/Longitude (the cluster -centroid), =first()= for Region/District/Rural. This is why =i= is in -each wave's =final_index=: that collapse only fires when =i= is an -index level. +the =(t, v)= EA grain. This is why =i= is in each wave's +=final_index=: that collapse only fires when =i= is an index level. + +*Correction (2026-07-21).* This paragraph used to say the collapse +takes =mean()= for Latitude/Longitude ("the cluster centroid") and +=first()= for Region/District/Rural. That is NO LONGER TRUE: the GPS +=.mean()= was retired when core stopped aggregating anywhere -- see +=_collapse_to_cluster_grain= in =country.py= and +=SkunkWorks/grain_aggregation_policy.org=. EVERY column is now +=.first()=-ed, and the projection is audited before it destroys +anything. + +And do NOT assume the flip is a no-op for Ethiopia. It is one in W1 +and W4 -- 0 of 333 and 0 of 516 EAs carry more than one distinct +household coordinate, i.e. the published fix really is the EA's, +stamped on each household. It is NOT one in the other three: + +| Wave | EAs | EAs with >1 distinct household coordinate | +|---------+-----+-------------------------------------------| +| 2011-12 | 333 | 0 | +| 2013-14 | 433 | 38 | +| 2015-16 | 432 | 72 | +| 2018-19 | 516 | 0 | +| 2021-22 | 432 | 72 | + +and the disagreements are not jitter: W2 EA =010103010100106= holds +three fixes spanning 0.19 deg of latitude and 0.60 deg of longitude, and +W5 EA =020103010100203= spans 1.79 deg of longitude -- roughly 200 km. +Points that far apart are not one enumeration area, so this is most +likely a *cluster key that is not unique nationally* (the Malawi +2013-14 pattern), not a GPS artifact. Under =.first()= such an EA +publishes one of its two real locations; under the old =.mean()= it +published a point in between, which was worse but louder. Either way +the =(t, v)= key is the thing that is wrong. The site-2 audit reports +it on every cold build -- e.g. "Ethiopia/cluster_features/2015-16 ... +DESTROYED 732 of 4,954 rows whose values DISAGREE (72 conflicting index +tuples)" -- and it is UNFIXED. See GH #323 site 2. Merging on the CLUSTER key =v= instead -- which is what the config used to do -- is a many-to-many join and produces a CARTESIAN PRODUCT @@ -648,3 +681,100 @@ subjective-well-being / Cantril-ladder / life-satisfaction module. A variable-na scan of all 63-75 household section files per wave returned zero matches (ladder/well-being/satisfaction/step/life). Resolves the #331 "unknown" → absent. (Food-security IS present — see #332.) Tracked in #331. + +* GH #323 — verification against PR #627's core, and one further defect (2026-07-21) + +** cluster_features is CLEAR under #627 — verified, not assumed + +PR #627 (site 4) turns the GH #515 swallowed =KeyError= into a hard +error when a dropped =dfs:= sub-frame costs a REQUIRED declared column, +and adds an exact cartesian guard on the merge. Its body lists +Ethiopia as an unfixed blocker with three raising cells. *That table +is stale.* The fix landed as commit =3488b791= (PR #628) and is an +ancestor of =development=. Measured end to end against #627's core +(=development= merged with =fix/323-site4-dfs-merge=), cold, with an +isolated data root: + +| Wave | pre-#628 config | current config | phantom rows cleared | +|---------+-------------------------------------+----------------+----------------------| +| 2011-12 | RuntimeError (=lat_dd_mod= missing) | 3,969 rows | 0 | +| 2013-14 | 65,508 rows, CARTESIAN | 5,262 rows | 60,221 | +| 2015-16 | 57,786 rows, CARTESIAN | 4,954 rows | 52,832 | +| 2018-19 | RuntimeError (=lat_dd_mod= missing) | 6,770 rows | 0 | +| 2021-22 | RuntimeError (no =ea_id=) | 4,959 rows | 0 | + +=Country('Ethiopia').cluster_features()= returns *2,168 rows on a +unique (t, v)*, 2,146 with Latitude. ALL 25 declared Ethiopia tables +build: 0 raised, 0 cartesian cells, 0 dropped sub-dfs. #627's own 16 +tests pass against this config. *The same re-key that cleared the +three raising cells also cleared both cartesian cells* -- which is why +#627's census, taken before #628 landed, could not see it. + +** individual_education (2013-14, 2015-16) — the identifier was the W1 baseline + +Found while verifying the above; UNRELATED to #627, and unfixed until +now. Both waves indexed education on =household_id= / =individual_id= +while =household_roster= and =shocks= for the SAME waves correctly used +the wave-native =household_id2= / =individual_id2=. + + - *W2*: =household_id= / =individual_id= are BLANK for the urban + REFRESHMENT sample, which has no W1 antecedent -- 5,248 of + =sect2_hh_w2.dta='s 23,785 rows. All of them land on the single + tuple =('2013-14', '', '')=. The framework audit said so exactly: + "DESTROYED 5,247 of 23,785 rows whose values DISAGREE (1 + conflicting index tuples) ... Additionally 5,248 row(s) carry NaN + in a declared index level and are DELETED OUTRIGHT". + - *W3*: SPLIT-OFF households inherit their parent's baseline id (W3 + households =010501020100105031= and =010501088800105031= -- the + =8888= marks the split-off -- both carry baseline + =01050100105031=), so 16 rows in 8 pairs collided and =first()= + MERGED TWO REAL, DISTINCT PEOPLE. They disagree: baseline + individual =0105010010503101= is "10th Grade Complete" in one + household and "9th Grade Complete" in the other. + +It also made education UNJOINABLE to the roster: *0* of the built +=(i, pid)= pairs matched a =household_roster= row in W2 and W3, against +100% in W1, W4 and W5 (which use the wave-native ids and were always +fine). In W3, 1,231 of the 4,283 education "households" existed +nowhere in the roster at all. + +Fixed by correcting the IDENTIFIER, per the standing #323 rule -- never +by declaring a reducer. After: both wave frames are unique +(23,785/23,785 and 23,393/23,393), the two GrainCollapseWarnings are +gone, the roster join is 100% in all five waves, and the country table +goes 59,092 -> 63,181 rows under identical conditions. Panel linkage +is unaffected: =id_walk= plus the existing =panel_ids= chain still map +=household_id2= back to the W1 baseline, exactly as =household_roster= +already relies on. + +** Caveat: this table's country-level row count is ORDER-DEPENDENT + +Not caused by the fix above, and not fixed by it. =id_walk= is applied +only once =panel_ids= has resolved, and whether it has depends on cache +state and on what else the process built first: + + - =Country('Ethiopia').individual_education()= -> 63,139 rows + - the same call after touching =c.panel_ids= -> 62,939 rows + +The 200-row delta lands on 2011-12 / 2015-16 / 2018-19 / 2021-22 and +reproduces identically in the pre-fix config. The proximate cause is +the lazy =self.updated_ids= probe in =_aggregate_wave_data=, whose +=except (FileNotFoundError, KeyError, ValueError): pass= records a +failed first attempt as "attempted" and then skips =id_walk= for the +rest of the process. Do NOT assert on this table's country-level row +count; assert at the wave level, which is upstream of the collapse and +deterministic. See =.coder/ledger/323-ethiopia-config.md= §6. + +** Still open: the 2013-14 wave-level panel_ids: block + +=2013-14/_/data_info.yml= still declares =panel_ids:= against +=sect_cover_ls_w2.dta= -- the LIVESTOCK-section cover, whose grain is +the agricultural HOLDER (3,812 rows / 3,670 households) and which +covers only 3,670 of the wave's 5,262 households. =data_scheme.yml= +declares =panel_ids= as =materialize: make=, so the country-level +=_/panel_ids.py= (which reads the household-grain +=sect_cover_hh_w2.dta=, 5,262 rows, unique) is authoritative and the +block appears to be vestigial. Left in place: "appears to be unused" +is not evidence, and deleting config on that basis is exactly the move +the repo forbids. Someone should confirm it is dead and then remove +it. diff --git a/tests/test_gh323_ethiopia_config.py b/tests/test_gh323_ethiopia_config.py new file mode 100644 index 000000000..a8484cc13 --- /dev/null +++ b/tests/test_gh323_ethiopia_config.py @@ -0,0 +1,286 @@ +"""GH #323 -- Ethiopia's config must not feed the framework a broken identifier. + +Two independent defects, both CONFIG, both true positives that the framework +was silently absorbing. This file pins them so they cannot come back. + +-------------------------------------------------------------------------- +1. `cluster_features` (all five waves) -- SITE 4, the `dfs:` merge +-------------------------------------------------------------------------- + +Ethiopia is the only country in the corpus whose `dfs:` block appears in +`cluster_features` for *every* wave, and it used to merge two HOUSEHOLD-grain +sub-frames (the survey cover page and the geovariables file) on the CLUSTER key +`v`. That is a many-to-many join, i.e. a cartesian product within each EA, and +`_normalize_dataframe_index`'s `groupby().first()` then tidied the evidence +away so the table reported clean. Measured against PR #627's core: + + 2013-14 5,262 x 5,287 households over 433 EAs -> 65,508 rows, 60,221 phantom + 2015-16 4,954 x 4,954 households over 432 EAs -> 57,786 rows, 52,832 phantom + +and in the other three waves the geo sub-frame did not load at all -- a +`KeyError` the GH #515 fallback swallowed, so Latitude/Longitude were 100% +ABSENT with only a warning. PR #627 turns that swallow into a hard error, so +these three cells RAISED. Fixed in PR #628 by merging on the household id with +`i` in `final_index`; this file is the regression net. + +THE KEY MUST BE THE WAVE-NATIVE HOUSEHOLD ID. For W2/W3 that is +`household_id2`, NOT `household_id`: `household_id` is the W1 baseline id and +is BLANK for households new to the panel, so re-keying to it would trade an EA +cartesian for a NULL-KEY cartesian (`pd.merge` matches null keys). + +-------------------------------------------------------------------------- +2. `individual_education` (2013-14, 2015-16) -- SITE 1, a broken identifier +-------------------------------------------------------------------------- + +Both waves indexed education on the W1-BASELINE ids while `household_roster` +and `shocks` for the same waves correctly used the wave-native `_id2` pair: + + W2 5,248 of sect2_hh_w2.dta's 23,785 rows -- the urban REFRESHMENT sample, + which has no W1 antecedent -- carry a BLANK household_id AND a blank + individual_id. The framework audit reported it exactly: "DESTROYED 5,247 + of 23,785 rows ... Additionally 5,248 row(s) carry NaN in a declared index + level and are DELETED OUTRIGHT". + + W3 SPLIT-OFF households inherit their parent's baseline id (W3 households + 010501020100105031 and 010501088800105031 both carry baseline + 01050100105031), so 16 rows in 8 pairs collided and `first()` MERGED TWO + REAL, DISTINCT PEOPLE -- one recorded '10th Grade Complete', the other + '9th Grade Complete'. + +It also broke the join to `household_roster`: 0% of W2/W3 (i, pid) pairs +matched a roster row, against 100% in W1/W4/W5. + +INSTRUMENT NOTE (same one the Benin/Togo tests carry -- do not undo it). Do +NOT assert on the API's post-collapse index uniqueness: `_normalize_dataframe_ +index` makes that true BY CONSTRUCTION and such a test passes with the bug +fully present. Assert at the WAVE level, which is upstream of the collapse. + +The wave level is also the only DETERMINISTIC place to assert row counts here. +Country-level counts for this table are order-dependent: `id_walk` is applied +only once `panel_ids` has resolved, and whether it has depends on cache state +and on what else the process built first (`Country('Ethiopia').individual_ +education()` alone returns 63,139 rows; the same call after touching +`c.panel_ids` returns 62,939). That instability is PRE-EXISTING, is unrelated +to these fixes -- it moves the untouched waves by the same rows in both the old +and the new config -- and is recorded in `.coder/ledger/323-ethiopia-config.md`. +""" +import warnings + +import pytest +import yaml + +from lsms_library.country import Country +from lsms_library.paths import countries_root +from lsms_library.yaml_utils import load_yaml + +ETH_WAVES = ['2011-12', '2013-14', '2015-16', '2018-19', '2021-22'] + +# The wave-native household id, per wave. W2/W3 re-identified the panel and +# renamed the key; W1/W4/W5 use the plain spelling. +NATIVE_HHID = {'2011-12': 'household_id', + '2013-14': 'household_id2', + '2015-16': 'household_id2', + '2018-19': 'household_id', + '2021-22': 'household_id'} + +NATIVE_PID = {'2013-14': 'individual_id2', + '2015-16': 'individual_id2'} + +# Raw row counts of the section-2 education files. Every row is a distinct +# person and every one must reach the wave frame on its own index tuple. +EDU_ROWS = {'2013-14': 23785, '2015-16': 23393} + + +def _wave_yaml(wave): + path = countries_root() / 'Ethiopia' / wave / '_' / 'data_info.yml' + if not path.exists(): # pragma: no cover + pytest.skip(f'Ethiopia/{wave}/_/data_info.yml absent') + with open(path) as f: + return yaml.safe_load(f) + + +@pytest.fixture(scope='module') +def ethiopia(): + try: + return Country('Ethiopia') + except Exception as exc: # pragma: no cover + pytest.skip(f'Ethiopia unavailable: {exc}') + + +# ---------------------------------------------------------------- config only + +@pytest.mark.parametrize('wave', ETH_WAVES) +def test_cluster_features_merges_on_the_household_id_not_the_cluster_id(wave): + """The `dfs:` merge key must be `i`, and `i` must reach `final_index`. + + Merging on `v` is the cartesian (GH #323 site 4). `i` must also survive + into `final_index`, because `Wave.cluster_features`' GH #161 projection to + the (t, v) cluster grain only fires when `i` is an index LEVEL -- drop it + earlier and the frame stays household-grain on a non-unique (t, v). + """ + block = _wave_yaml(wave)['cluster_features'] + assert block.get('merge_on') == ['i'], ( + f"Ethiopia/{wave}/cluster_features merges on " + f"{block.get('merge_on')!r}. Both sub-frames are HOUSEHOLD-grain, so " + f"merging on the cluster key `v` is a many-to-many CARTESIAN PRODUCT " + f"(GH #323 site 4). Merge on `i`." + ) + assert 'i' in (block.get('final_index') or []), ( + f"Ethiopia/{wave}/cluster_features drops `i` before `final_index`. " + f"Wave.cluster_features' collapse to the (t, v) grain only fires when " + f"`i` is an index level; without it the frame stays household-grain." + ) + + +@pytest.mark.parametrize('wave', ETH_WAVES) +def test_cluster_features_geo_subframe_is_keyed_on_the_native_household_id(wave): + """The geo sub-frame keys on the wave-native household id, never `v`. + + `household_id` is the W1 baseline id and is BLANK for households new to the + panel, so in W2/W3 it is non-unique on the empty value: re-keying to it + would trade the EA cartesian for a NULL-KEY one, since `pd.merge` matches + null keys like any other value. + """ + block = _wave_yaml(wave)['cluster_features'] + geo = block['df_geo']['idxvars'] + assert 'v' not in geo, ( + f"Ethiopia/{wave}/cluster_features df_geo still declares `v`; it is a " + f"household-grain file and must be keyed on the household id." + ) + assert geo.get('i') == NATIVE_HHID[wave], ( + f"Ethiopia/{wave}/cluster_features df_geo keys on {geo.get('i')!r}; " + f"the wave-native household id is {NATIVE_HHID[wave]!r}." + ) + assert block['df_main']['idxvars'].get('i') == NATIVE_HHID[wave], ( + f"Ethiopia/{wave}/cluster_features df_main must key on the same " + f"household id as df_geo ({NATIVE_HHID[wave]!r})." + ) + + +@pytest.mark.parametrize('wave', sorted(NATIVE_PID)) +def test_individual_education_uses_the_wave_native_ids(wave): + """W2/W3 education must use household_id2 / individual_id2. + + The W1-baseline pair is blank for the W2 refreshment sample and is shared + by W3 split-offs with their parent household -- either way the declared + (t, i, pid) index stops identifying a person. + """ + idx = _wave_yaml(wave)['individual_education']['idxvars'] + assert idx.get('i') == NATIVE_HHID[wave], ( + f"Ethiopia/{wave}/individual_education keys `i` on {idx.get('i')!r}; " + f"the wave-native household id is {NATIVE_HHID[wave]!r}. The baseline " + f"`household_id` is blank / shared and destroys rows (GH #323)." + ) + assert idx.get('pid') == NATIVE_PID[wave], ( + f"Ethiopia/{wave}/individual_education keys `pid` on " + f"{idx.get('pid')!r}; the wave-native person id is " + f"{NATIVE_PID[wave]!r} -- the same one household_roster uses, without " + f"which education cannot be joined to the roster at all." + ) + + +def test_no_dead_aggregation_key_in_ethiopia_config(): + """`aggregation:` is dead config and contradicts D1 -- core never reduces. + + An earlier Ethiopia branch declared reducers here. Nothing reads them + (`SkunkWorks/grain_aggregation_policy.org` §3a), so they would read as a + fix while changing nothing. + """ + path = countries_root() / 'Ethiopia' / '_' / 'data_scheme.yml' + # `data_scheme.yml` uses the repo's `!make` tag, so it needs SchemeLoader. + scheme = load_yaml(path)['Data Scheme'] + offenders = [k for k, v in scheme.items() + if isinstance(v, dict) and 'aggregation' in v] + assert not offenders, ( + f"Ethiopia/_/data_scheme.yml declares `aggregation:` for {offenders}. " + f"The core does NOT aggregate (GH #323 decision D1); the key is dead " + f"config that only puts a signature on the corpse." + ) + + +# ---------------------------------------------------------------- data-backed + +@pytest.mark.parametrize('wave', sorted(EDU_ROWS)) +def test_individual_education_wave_frame_keeps_every_person(ethiopia, wave): + """Every reported person reaches the wave frame on their own index tuple. + + Asserted at the WAVE level, upstream of `_normalize_dataframe_index` -- + post-collapse uniqueness holds by construction and proves nothing. + """ + try: + df = ethiopia[wave].grab_data('individual_education') + except Exception as exc: # pragma: no cover + pytest.skip(f'Ethiopia/{wave} individual_education unbuildable: ' + f'{type(exc).__name__}: {exc}') + assert len(df) == EDU_ROWS[wave], ( + f'Ethiopia/{wave} individual_education wave frame has {len(df)} rows, ' + f'expected {EDU_ROWS[wave]} (one per person in the section-2 file).' + ) + dupes = int(df.index.duplicated().sum()) + assert dupes == 0, ( + f'Ethiopia/{wave} individual_education lands {dupes} row(s) on an ' + f'already-used (t, i, pid) tuple. The framework will collapse them ' + f'with groupby().first() and those people will silently disappear ' + f'(GH #323). The identifier is wrong -- use the wave-native ' + f'{NATIVE_HHID[wave]} / {NATIVE_PID[wave]}, not the W1 baseline.' + ) + + +def test_individual_education_recovers_the_two_conflated_students(ethiopia): + """The W3 split-off pair that `first()` used to merge into one person. + + Baseline individual 0105010010503101 appears in BOTH halves of a split + household, with genuinely different attainment. Under the baseline ids the + two rows shared one index tuple and one was destroyed. + """ + try: + df = ethiopia['2015-16'].grab_data('individual_education') + except Exception as exc: # pragma: no cover + pytest.skip(f'Ethiopia/2015-16 unbuildable: {type(exc).__name__}: {exc}') + flat = df.reset_index() + pids = {'01050102010010503101', '01050108880010503101'} + got = flat[flat['pid'].isin(pids)] + assert set(got['pid']) == pids, ( + f'expected both halves of the split household ({pids}); ' + f'got {set(got["pid"])}. One of two REAL, DISTINCT people is being ' + f'destroyed by the collapse (GH #323).' + ) + attainment = set(got['Educational Attainment'].dropna()) + assert len(attainment) == 2, ( + f'the two students should record DIFFERENT attainment; got ' + f'{attainment}. If they are identical the rows have been merged.' + ) + + +@pytest.mark.parametrize('wave', ETH_WAVES) +def test_cluster_features_merge_manufactures_no_phantom_rows(ethiopia, wave): + """The `dfs:` merge must emit no cartesian warning (GH #323 site 4). + + This is the assertion PR #627's guard exists to make possible. On cores + predating that guard the warning cannot be raised at all, so the test + degrades to "the wave builds and the merge did not explode", checked + against the cover page's own household count. + """ + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + try: + df = ethiopia[wave].grab_data('cluster_features') + except Exception as exc: # pragma: no cover + pytest.fail( + f'Ethiopia/{wave} cluster_features raised ' + f'{type(exc).__name__}: {exc}. Under PR #627 a dropped sub-df ' + f'that costs a REQUIRED declared column is fatal -- the geo ' + f'file most likely spells its coordinate columns differently ' + f'in this wave.') + cartesian = [str(w.message) for w in caught + if 'CARTESIAN' in str(w.message)] + assert not cartesian, ( + f'Ethiopia/{wave} cluster_features: {cartesian[0][:400]}' + ) + # A cartesian would multiply the frame far past household grain; the cover + # page has ~3.9k-6.8k households in every wave. + assert len(df) < 10000, ( + f'Ethiopia/{wave} cluster_features wave frame has {len(df)} rows -- ' + f'far more than the wave has households. The `dfs:` merge has ' + f'MANUFACTURED rows (GH #323 site 4).' + )