fix(#323): Ethiopia + Niger geo — 3 of Ethiopia's 5 waves had ZERO coordinates (unblocks #627)#628
Merged
Merged
Conversation
…red-column raise (#627) PR #627 makes the GH #515 optional-sub-df fallback RAISE when a dropped sub-dataframe costs a REQUIRED declared column. That is correct, and it means `Country('Ethiopia').cluster_features()` and `Country('Niger').cluster_features()` raise on that branch until their configs are fixed. This fixes them. Config only -- no `lsms_library/*.py`. All ten cells are TRUE POSITIVES. Every one was diagnosed by opening the source file and listing its real columns; none was guessed. Nine of ten are case (b) -- the column EXISTS and the YAML named it wrong -- so they are WIRED UP, not deleted. ETHIOPIA -- the household geovariables file is renamed in almost every wave: 2011-12 Pub_ETH_HouseholdGeovariables_Y1 LAT_DD_MOD / LON_DD_MOD (UPPERCASE) 2013-14 Pub_ETH_HouseholdGeovars_Y2 lat_dd_mod / lon_dd_mod 2015-16 ETH_HouseholdGeovars_y3 lat_dd_mod / lon_dd_mod 2018-19 ETH_HouseholdGeovariables_Y4 lat_mod / lon_mod (no `_dd_`) 2021-22 eth_householdgeovariables_y5 lat_dd_mod / lon_dd_mod, and the file has NO ea_id AT ALL W1 (casing), W4 (dropped `_dd_` infix) and W5 (missing `ea_id` KEY -- the coordinates themselves were always there, 4,847/4,890 non-null) are the three raising cells. All three are case (b). W2 and W3 resolved their columns but merged two HOUSEHOLD-grain frames on the CLUSTER key `v` -- a cartesian product (2013-14: 65,508 rows, 60,221 phantom; 2015-16: 57,786 rows, 52,832 phantom). Same edit fixes both. All five waves now merge on the HOUSEHOLD id (1:1 -- measured unique and blank-free in BOTH frames in every wave) and carry `i` in `final_index`, so `Wave.cluster_features`'s existing GH #161 collapse reduces to the (t, v) EA grain with mean() for Latitude/Longitude (the cluster centroid) and first() for Region/District/Rural. This is the Malawi 2013-14 pattern. W2/W3 use `household_id2`, NOT `household_id`: the latter is the W1 baseline id, blank for households new to the panel, so re-keying to it would trade an EA cartesian for a NULL-KEY cartesian. `merge_how: left` in all five -- the geo files carry households the cover page does not (W2 25, W4 124, W5 35), and under `outer` those orphans arrive with a null `v` and collapse into one phantom cluster. NOTE: this deliberately does NOT reuse `fix/323-ethiopia`'s config, which used `final_index: [t, v]` + `drop: [i]` and leaned on a declared `aggregation:` key in data_scheme.yml. Per D1 of DESIGN_grain_collapse_sites_2026-07-13.org that key is DEAD CONFIG and is deliberately not honoured by the core collapse. Under that config the frame would stay household-grain on a non-unique (t, v) index and Site 1 would collapse it with first() -- giving an arbitrary household's latitude instead of the cluster centroid. NIGER 2011-12 -- the brief said "the geo file has no lat/lon at all", and the honest fix would then be `optional: true`. VERIFIED, AND THAT IS A TRAP. `NER_HouseholdGeovars_Y1.dta` indeed has no coordinates: its 43 columns are derived raster covariates only (dist_road, af_bio_*, srtm, twi, sq1..sq7, EVI/NDVI). But the coordinates ship in a SIBLING FILE in the same directory -- `NER_EA_Offsets.dta`, the WB's standard offset-EA-centroid release: 271 rows x (grappe, LAT_DD_MOD, LON_DD_MOD), 270 non-null grappes, coordinates inside Niger's bounding box, ALREADY DVC-TRACKED. Every grappe in the household file is in it and vice versa (0 orphans both ways). So Niger is case (b), not case (a): the data is real, available, and merely mis-addressed. Marking it `optional:` would have DELETED A REAL COLUMN and called it a cleanup. Wired up instead -- all 270 clusters now carry coordinates where they previously carried none. The offsets file is already EA-grain, so the merge on `v` is many-to-one, not a cartesian. `merge_how: left` drops the file's one trailing null-grappe row, which `outer` would turn into a phantom cluster. Niger 2014-15 ships no geovariables/offsets file of any kind, so its Latitude/Longitude are honestly absent rather than mis-addressed. Not wired, and that is correct. VERIFIED (site-4 code via PYTHONPATH, this config via LSMS_COUNTRIES_ROOT, LSMS_NO_CACHE=1 -- cold, no cached parquet, import asserted): development (today) this branch Ethiopia 2,169 rows / 866 lat 2,168 rows / 2,146 lat Niger 1,599 rows / 1,048 lat 1,599 rows / 1,318 lat Ethiopia by wave, clusters carrying Latitude: 2011-12 0 -> 333 2013-14 434 -> 433 2015-16 432 -> 432 2018-19 0 -> 516 2021-22 0 -> 432 Niger 2011-12: 0 -> 270 Three of Ethiopia's five waves had ZERO coordinates and now have them; Niger 2011-12 likewise. +1,280 Ethiopian and +270 Nigerien clusters gain real GPS. NO DATA LOST. The only row that disappears is Ethiopia 2013-14 434 -> 433 clusters, and that row is EA 030412088800105 -- present in the geo file, absent from the cover page, its single household absent too. It was a PHANTOM cluster manufactured by the outer merge on `v`. Dropping it is the correction, not a loss. Every other per-wave count is identical. Both countries build with 0 cartesian warnings, 0 dropped sub-dfs, 0 grain-collapse warnings, and a unique (t, v) index. Every YAML-path table in both countries still builds under LSMS_GRAIN_STRICT=1. All 16 tests in #627's tests/test_gh323_site4_dfs_merge.py pass against this config. Coordinates sanity-checked against each country's real bounding box: Ethiopia lat 3.46-14.48 N / lon 33.43-47.31 E; Niger lat 11.80-19.07 N / lon 0.28-13.78 E. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDLDHYG32Z57ipikJMywZ
This was referenced Jul 21, 2026
ligon
added a commit
that referenced
this pull request
Jul 21, 2026
…index bug VERIFICATION, not re-implementation. PR #627's body lists Ethiopia as an unfixed blocker with three raising `cluster_features` cells. That table is stale: the fix landed as `3488b791` (PR #628) and is already an ancestor of `development`. Nobody had confirmed it against #627's actual core. This does. Measured against #627's core (`development` merged with `fix/323-site4-dfs-merge` via `git merge-tree`, asserted at runtime through `lsms_library.__file__`), cold, with an isolated `LSMS_DATA_DIR` whose only warm tier is a symlinked `dvc-cache`: NEGATIVE CONTROL (pre-3488b791 config, same core) — reproduces #627's census exactly: 2011-12 / 2018-19 / 2021-22 RAISE; 2013-14 is a 65,508-row cartesian (60,221 phantom); 2015-16 is 57,786 rows (52,832 phantom). 113,053 phantom rows total. CURRENT config, same core: all five waves build, 0 cartesian cells, 0 phantom rows. `Country('Ethiopia').cluster_features()` -> 2,168 rows on a unique (t, v), 2,146 with Latitude. ALL 25 declared Ethiopia tables build: 0 raised, 0 cartesian, 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. Ethiopia is genuinely green; #627 is unblocked on Ethiopia's account. THE ONE REAL GAP FOUND — `individual_education`, 2013-14 and 2015-16. Unrelated to #627, a #323 site-1 true positive, and unfixed until now. Both waves indexed education on the W1-BASELINE `household_id` / `individual_id` while `household_roster` and `shocks` for the SAME waves correctly used the wave-native `household_id2` / `individual_id2`: W2 the baseline pair is BLANK for the urban refreshment sample (no W1 antecedent) — 5,248 of 23,785 rows, all landing on one tuple. The framework's own audit: "DESTROYED 5,247 of 23,785 rows whose values DISAGREE ... 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, 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 made education UNJOINABLE to the roster: 0 of the built (i, pid) pairs matched a roster row in W2/W3, against 100% in W1/W4/W5. Fixed by correcting the IDENTIFIER, per the standing rule — never by declaring a reducer. No `aggregation:` keys; no file under `lsms_library/*.py`. After: both wave frames unique (23,785/23,785, 23,393/23,393), both GrainCollapse warnings gone, roster join 100% in all five waves, 59,092 -> 63,181 rows under identical conditions. id_walk + panel_ids still map household_id2 back to the W1 baseline, so panel linkage is preserved. DELIBERATELY NOT SALVAGED from the abandoned `fix/323-ethiopia` branch: its `aggregation:` keys (dead config, contradicts D1 — a test now pins that Ethiopia declares none); its `interview_date` mapping.py hooks (measured redundant — `_enforce_canonical_dtypes` already honours the declared `Int_t: datetime`, and the table returns datetime64, 8,236 rows, 0 nulls); its core patch. Tests: 21 new, asserted at the WAVE level (post-collapse uniqueness holds by construction and proves nothing). Negative control run: 21/21 pass against the current config, 17/21 FAIL against the pre-3488b791 tree on the same core. Also corrects a `CONTENTS.org` claim that #628 left behind and that is now false — the cluster collapse no longer takes `mean()` for GPS. Do not assume the flip is a no-op here: measured, 38/433 (W2), 72/432 (W3) and 72/432 (W5) EAs carry more than one distinct household coordinate, one spanning 1.79 deg of longitude. That is a non-unique cluster key, not jitter — recorded, unfixed, GH #323 site 2. Ledger: .coder/ledger/323-ethiopia-config.md. Refs #323, #515. Unblocks #627. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ligon
added a commit
that referenced
this pull request
Jul 22, 2026
…index bug VERIFICATION, not re-implementation. PR #627's body lists Ethiopia as an unfixed blocker with three raising `cluster_features` cells. That table is stale: the fix landed as `3488b791` (PR #628) and is already an ancestor of `development`. Nobody had confirmed it against #627's actual core. This does. Measured against #627's core (`development` merged with `fix/323-site4-dfs-merge` via `git merge-tree`, asserted at runtime through `lsms_library.__file__`), cold, with an isolated `LSMS_DATA_DIR` whose only warm tier is a symlinked `dvc-cache`: NEGATIVE CONTROL (pre-3488b791 config, same core) — reproduces #627's census exactly: 2011-12 / 2018-19 / 2021-22 RAISE; 2013-14 is a 65,508-row cartesian (60,221 phantom); 2015-16 is 57,786 rows (52,832 phantom). 113,053 phantom rows total. CURRENT config, same core: all five waves build, 0 cartesian cells, 0 phantom rows. `Country('Ethiopia').cluster_features()` -> 2,168 rows on a unique (t, v), 2,146 with Latitude. ALL 25 declared Ethiopia tables build: 0 raised, 0 cartesian, 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. Ethiopia is genuinely green; #627 is unblocked on Ethiopia's account. THE ONE REAL GAP FOUND — `individual_education`, 2013-14 and 2015-16. Unrelated to #627, a #323 site-1 true positive, and unfixed until now. Both waves indexed education on the W1-BASELINE `household_id` / `individual_id` while `household_roster` and `shocks` for the SAME waves correctly used the wave-native `household_id2` / `individual_id2`: W2 the baseline pair is BLANK for the urban refreshment sample (no W1 antecedent) — 5,248 of 23,785 rows, all landing on one tuple. The framework's own audit: "DESTROYED 5,247 of 23,785 rows whose values DISAGREE ... 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, 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 made education UNJOINABLE to the roster: 0 of the built (i, pid) pairs matched a roster row in W2/W3, against 100% in W1/W4/W5. Fixed by correcting the IDENTIFIER, per the standing rule — never by declaring a reducer. No `aggregation:` keys; no file under `lsms_library/*.py`. After: both wave frames unique (23,785/23,785, 23,393/23,393), both GrainCollapse warnings gone, roster join 100% in all five waves, 59,092 -> 63,181 rows under identical conditions. id_walk + panel_ids still map household_id2 back to the W1 baseline, so panel linkage is preserved. DELIBERATELY NOT SALVAGED from the abandoned `fix/323-ethiopia` branch: its `aggregation:` keys (dead config, contradicts D1 — a test now pins that Ethiopia declares none); its `interview_date` mapping.py hooks (measured redundant — `_enforce_canonical_dtypes` already honours the declared `Int_t: datetime`, and the table returns datetime64, 8,236 rows, 0 nulls); its core patch. Tests: 21 new, asserted at the WAVE level (post-collapse uniqueness holds by construction and proves nothing). Negative control run: 21/21 pass against the current config, 17/21 FAIL against the pre-3488b791 tree on the same core. Also corrects a `CONTENTS.org` claim that #628 left behind and that is now false — the cluster collapse no longer takes `mean()` for GPS. Do not assume the flip is a no-op here: measured, 38/433 (W2), 72/432 (W3) and 72/432 (W5) EAs carry more than one distinct household coordinate, one spanning 1.79 deg of longitude. That is a non-unique cluster key, not jitter — recorded, unfixed, GH #323 site 2. Ledger: .coder/ledger/323-ethiopia-config.md. Refs #323, #515. Unblocks #627. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ligon
added a commit
that referenced
this pull request
Jul 22, 2026
…measure the census A false claim in a tracked file outlives the PR thread. Each correction below was re-derived here, not copied from the review. 1. `merge_how: left` prose (CLAUDE.md, the `sample` skill, the source comment) said the `outer` orphans "collapse together into one phantom null-keyed row". They do not. Ethiopia 2013-14, two isolated cold processes differing only in that key: `outer` 5,287 wave rows / 25 null-`v`; `left` 5,262 / 0; the delivered table is 433 clusters with sum(Latitude) 4070.3702 EITHER WAY, because the cluster-grain collapse DELETES null keys. `merge_how: left` is not a data fix -- it stops manufacturing rows for site 1 to delete, and stops the merge widening `District` from int8 to float64. Its cost is the lost signal `outer` gave you. (The "phantom 434th EA" story is not false, it is about the PRE-#628 config where `df_geo` was keyed on `v`. Ethiopia/_/CONTENTS.org and the 2013-14 `merge_how` comment still tell that story beside the new config; flagged for #628's owner, not edited here.) 2. Ledger §6 opened "67 `dfs:` merges exercised across 19 countries (the other 21 declare none)". The declaration count is statically checkable and is **76 blocks / 20 countries** at the branch base (80 / 20 after merging development). 67 was the number EXECUTED. Two quantities reported as one. 3. Ledger §6's Niger row said the geo file "has no lat/lon column at all" and prescribed `optional: true`. Half true and wholly the wrong conclusion: `NER_HouseholdGeovars_Y1.dta` has none, but the sibling `NER_EA_Offsets.dta` in the same DVC-tracked directory is 271 x [grappe, LAT_DD_MOD, LON_DD_MOD], 270 unique grappes, lat 11.876-18.747 N -- re-verified from source here. This is exactly the unevidenced "no module here" claim CLAUDE.md warns against. 4. Ledger §7's "0 of the 67 sub-frames is drawn from a `_pp_`/`_ph_` round file" overstated a FILENAME test. Nigeria's `df_main` is `Post Planting Wave 1/.../secta_plantingw1.dta`. The substantive conclusion (no cross-round cartesian at this site) rests on `t` being constant within every `dfs:` merge, and stands. Also corrected: §2/§5 (LSMS_GRAIN_STRICT IS on development now -- reuse `_grain_strict()`), §4 (the required-column check's blast radius was three countries and ten cells, not one), §9 (the three raises are fixed; the reason the suite was blind is `test_table_structure`'s read-only contract, NOT the v0.8.0 hash missing a core change -- see §11.5), §10 (sequencing discharged: #625, #628, #644 all merged; all three countries re-verified cold). New §11 answers each review finding, including one DISPUTED with measurement (§11.5: `country.py` build-path code IS in the v0.8.0 hash -- editing only `_merge_subframes`'s body moves `build_transforms_fingerprint`, and Nigeria's warm parquet grades `stale`). New §12 re-runs the whole census on the merged tree: 80 blocks / 90 merges exercised / **3 cartesian cells left**, whose phantom counts (183,812 + 171,230 + 59) reproduce §6 to the row. Refs #323, #515. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Su5JKX3wKChyfMAdXrdCTr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Config-only. Unblocks PR #627 (Site 4), which makes the GH #515 sub-df swallow a hard error and therefore currently makes
Ethiopia/Niger/Nigeriacluster_features()raise. (Nigeria's half is already fixed by #625.)Nine of the ten cells were case (b) — the column exists and the YAML named it wrong. They are wired up, not deleted.
Ethiopia — the geovariables file is renamed in almost every wave
lat_dd_modLAT_DD_MOD(uppercase) — 3917/3917 non-nulllat_dd_modlat_mod(no_dd_) — 6637/6804 non-nullv: ea_idea_idat all; coords present — 4847/4890 non-nullEvery one raised a
KeyErrorthat the #515 fallback silently ate. W2/W3 weren't required-column errors but were the two cartesian cells (65,508 / 57,786 phantom rows) — the same re-key fixes both.All five waves now merge on the household id (measured unique and blank-free in both frames) with
iinfinal_index, soWave.cluster_features's existing GH #161 collapse reduces to(t, v). This is the Malawi 2013-14 pattern.fix/323-ethiopia's config was deliberately NOT reused. It usedfinal_index: [t,v]+drop: [i]and leaned on a declaredaggregation:key — dead config under D1, deliberately not honoured by the core collapse. Under it the frame stays household-grain on a non-unique(t,v)index and Site 1 collapses withfirst(), yielding an arbitrary household's latitude instead of the cluster centroid.Niger — the brief was WRONG, and it was the trap
The task brief (mine) said Niger's geo file "has no lat/lon at all", implying
optional: true.NER_HouseholdGeovars_Y1.dtaindeed has none — 43 columns, all derived raster covariates.But the coordinates ship in a sibling file in the same directory.
NER_EA_Offsets.dta— 271 rows × (grappe,LAT_DD_MOD,LON_DD_MOD), 270 non-null grappes, inside Niger's bounding box, already DVC-tracked, every grappe matching both ways.So Niger is case (b), not (a): real, available, merely mis-addressed. Marking it
optional:— as the brief implied — would have deleted a real column and called it a cleanup. Wired up instead: 270 clusters gained coordinates that were previously zero.(Niger 2014-15 ships no geo file of any kind — honestly absent, correctly left unwired. No case (c) anywhere: every file involved has a
.dvcsidecar.)Acceptance test — cold,
LSMS_NO_CACHE=1, run against #627's codedevelopmenttodayClusters carrying Latitude, per wave — Ethiopia: 0→333, 434→433, 432→432, 0→516, 0→432. Niger 2011-12: 0→270.
Three of Ethiopia's five waves had zero coordinates. +1,280 Ethiopian and +270 Nigerien clusters gain real GPS.
No data lost. The only row that disappears is Ethiopia 2013-14 (434→433): EA
030412088800105, present in the geo file, absent from the cover page, its one household absent too. It was a phantom cluster the outer merge manufactured. Dropping it is the correction.Both build with 0 cartesian warnings, 0 dropped sub-dfs, 0 grain-collapse warnings, unique
(t,v). Every table in both countries builds underLSMS_GRAIN_STRICT=1(Niger 14/14). All 16 tests in #627'stest_gh323_site4_dfs_merge.pypass against this config. Coordinates sanity-checked against each country's bounding box.Idiosyncrasies found (per the CONTENTS.org rule)
cluster_featuressourcessect_cover_hh_w*.dta(the household cover), not asect*_pp_/_ph_file, and Ethiopia declares nowave_folder_map. So there is exactly one round per wave here; verified per-t. The real idiosyncrasy is the wave-keyed ID scheme (household_id/ea_idfor W1/W4/W5;household_id2/ea_id2for W2/W3 — CONTENTS.org line 431ff) plus four different GPS column spellings across five waves.**CONFIDENTIAL**) applies toplot_featuresonly — the household geovariables files carry usable offset coordinates. A note was added so the two are not conflated again.Not verified
Ethiopia's 19 script-path tables exceeded the timeout on a cold
LSMS_GRAIN_STRICT=1sweep; all YAML-path tables pass. The edits are confined to thecluster_features:block, so the script-path tables cannot be affected — but they were not observed building end-to-end.Refs #323, #515. Unblocks #627.
🤖 Generated with Claude Code