fix(GhanaLSS): GH #323 — make the silent index collapse declared-and-CHECKED (one instance; class stays open)#609
Closed
ligon wants to merge 1 commit into
Closed
fix(GhanaLSS): GH #323 — make the silent index collapse declared-and-CHECKED (one instance; class stays open)#609ligon wants to merge 1 commit into
ligon wants to merge 1 commit into
Conversation
… silent GH #323: `_normalize_dataframe_index` collapses a non-unique DECLARED index with `groupby().first()`, silently discarding the dropped rows. GhanaLSS exhibits BOTH failure modes the line enables, from the same code -- so one uniform fix would have baked in a wrong value. Three distinct fixes: (A) cluster_features, 5 waves [1991-92, 1998-99, 2005-06, 2012-13, 2016-17] -- INTENDED de-duplication, made DECLARED + VERIFIED. Each wave points `file:` at a HOUSEHOLD-grain source and projects cluster-invariant columns, so 934,584 rows -> 1,000 clusters (2016-17) is the right answer, arrived at silently. Verified cluster-invariant in all five (max distinct per cluster = 1; zero violations). `aggregation:` was already declared by 9 countries and read by NOBODY -- grep for its consumption returned zero (see SkunkWorks/ grain_aggregation_policy.org item #4, "not yet built"). It was prose. So this implements it: a new `invariant` reducer that ASSERTS the column is constant within each index group and RAISES if it is not. Declaring it without implementing it would have been the "prose is not enforcement" trap. (B) cluster_features, 1987-88 + 1988-89 -- silently WRONG; column DROPPED. `Region` was wired to Y01A.DAT's REGION, the person's REGION OF BIRTH: - mapping.py's Region() and Birthplace() are byte-identical bodies over the same region_dict, and REGION is also mapped as roster Birthplace; - the waves' own `region` code list runs to 11=Nigeria, 12=Ivory Coast, 13=Togo, 14=Burkina Faso -- a Ghanaian enumeration area cannot be LOCATED in Nigeria; only a birthplace can; - REGION varies WITHIN one household in ~1/3 of households (up to 6 distinct values), and within a cluster in 167/170 and 174/176. Collapsed with .first(), a cluster inherited its first-listed person's birth region -- labelling 9/170 (1988-89) and 7/176 (1987-88) clusters "Nigeria". No cluster-invariant region source exists in either wave, so we DROP rather than guess: class-2 (silently missing) beats class-1 (silently wrong). Also drops 1987-88's `Age: AGEY` (a person attribute). 1987-88 was NOT in the reported cell list -- only because its parquet had never been built; fixing 1988-89 alone would have left the class alive. NB the fabrication is currently MASKED: Region is 100% NaN because get_categorical_mapping returns {} for these waves (a separate defect). Repairing that -- plausible, since it also breaks Birthplace -- would ARM the fabrication. Dropping Region costs zero information today and defuses it; re-adding it now RAISES via (A)'s invariant policy. (C) food_security 2016-17 -- phantom NaN key; re-keyed on `hid`. The last 110 rows of g7sec9c.dta have clust=NaN AND nh=NaN (all 8 FIES items NaN) but a valid `hid`. Keyed on [clust, nh] they collapsed onto one NaN tuple and were dropped outright. `hid` reconstructs the compound key with 100.00% fidelity on all 13,899 good rows and is byte-identical to the id built from [clust, nh]; the 110 are distinct households, disjoint from the answered set, and all 110 appear in sample(). FIES_score() already returns pd.NA when all 8 items are missing, so this does not fabricate a food-secure score of 0. EVIDENCE (wave-level parquet, the truth layer -- var/ is written POST-collapse) food_security 2016-17 : 110 NaN keys / 109 dups -> 0 / 0; 13,899 -> 14,009 uniquely-keyed households; the 13,899 pre-existing ids and their payload are byte-identical (0 lost). cluster_features : 1988-89's 14,754-dup collapse -> table removed. 967,119 dups across the 5 good waves -> DECLARED and verified (build RAISES if invariance ever breaks). GH #323 silent warnings on a cold GhanaLSS build: 8 -> 1. ROWS RECOVERED AT THE API: 0, honestly. One row per cluster IS the correct grain, and the 110 food_security rows are all-NA in every declared column so _finalize_result's dropna(how='all') drops them (as it already drops 7 all-NA households that DID have clust/nh). The value here is correctness, not row count -- the vanity metric would be a lie. REGRESSION: 757 real (country x wave x table) cells pushed through BOTH the base and fixed `_normalize_dataframe_index` on real cached data: 0 differ. All 9 existing `aggregation:` blocks are `{visit: first}` -- `visit` is an index LEVEL, never a column, and `first` != `invariant` -- so they are doubly unaffected. Guard proven fatal end-to-end through Country() (not swallowed). tests/test_currency.py::test_feature_ghana_per_wave still fails identically (`assert ['GHC'] == {'GHC'}`, GH #589, pre-existing on base -- not touched). NOT FIXED, reported instead (see .coder/ledger/323-ghanalss.md §8): - `sample` has 2 duplicate (i,t): panel_ids is many-to-one (1988-89 HH 204922 AND 204932 -> 1987-88,101332; 255718 AND 255728 -> 1987-88,114008). A DIFFERENT root cause; choosing the "true" match would be a guess. - get_categorical_mapping returns {} for 1988-89 (Birthplace/Relationship all-NA); 1998-99 cluster_features Region/Rural all-NA. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Superseded by #622, which carries the same two GhanaLSS fixes with the rejected +84-line Per the #323 consolidation ( The country-level diagnosis here was right and is preserved intact in #622 — including the finding that Closing in favour of #622. |
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
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.
GH #323 — GhanaLSS instance (ONE instance; the class stays open)
_normalize_dataframe_indexsilently collapses a non-unique declared index withgroupby().first(). This PR fixes the GhanaLSS instance of that bug — 7 cells —and adds the first actual enforcement mechanism for the
aggregation:policy.It does NOT close #323. #323 is a class (~14 countries / ~44 cells / ~150k rows;
Mali alone loses 32,026 of 37,175 roster rows). Closing the class on an instance fix is
precisely how this bug survived its first round. #323 stays open until the framework fix
lands.
What was silently dropped, and how many rows
Measured at the L2-wave parquet (the truth layer). The L2-country parquet
(
var/) is written POST-collapse, so scanning it returns a false zero — the instrumentwas validated on the known positives first (Mali/2014-15
household_rosterdup = 32,026exactly; Guyana/1992
housingdup = 311 exactly) before any GhanaLSS number was trusted.cluster_featurescluster_featurescluster_featurescluster_featurescluster_featurescluster_featurescluster_featuresfood_securitySilent #323 warnings on a cold GhanaLSS build: 8 → 1 (the remaining one is
sample,pre-existing, a different root cause).
Rows recovered at the API = 0, honestly. One row per cluster is the correct grain for
(A)/(B); the 110 recovered
food_securityhouseholds are all-NA in every declared columnand are dropped by
_finalize_result'sdropna(how='all')(country.py:2217) — exactly asit already drops 7 all-NA households that did have
clust/nhpopulated. Claiming a110-row recovery would be a vanity metric. The value here is that the collapse is no longer
silent and the fabrication path is disarmed.
Three root causes, fixed three different ways
One uniform fix would have baked in a wrong value.
(A)
cluster_features, 5 waves — INTENDED_AGGREGATION, now DECLARED and ENFORCED.Each wave points
file:at a HOUSEHOLD-grain source and projects cluster-invariant columns.Verified invariant in all five at the source (max distinct value per cluster = 1; zero
violations).
Key finding:
aggregation:was already declared by 9 countries and read by ZERO lines ofcode — grep for its consumption returns nothing;
SkunkWorks/grain_aggregation_policy.orgitem #4 is literally "not yet built". Declaring it would have been pure prose, and prose is
not enforcement. So it is now implemented: a new
invariantreducer in_normalize_dataframe_index(+_assert_invariant_within_index) that verifies constancywithin the index group and RAISES otherwise. Proven fatal end-to-end through
Country()(projected household-varying
nhas a declared-invariant column →ValueError,"non-constant in 1000 of 1000 groups") — i.e. it is not swallowed by the wave loop's
except (KeyError, AttributeError). Guard cost: 0.30 s on the 934,584-row worst case.(B)
cluster_features, 1987-88 + 1988-89 — EXTRACTION_BUG (silently WRONG).Regionwas wired toY01A.DAT REGION= the person's REGION OF BIRTH. Confirmed 5 ways:mapping.py'sRegion()andBirthplace()are byte-identical bodies over the sameregion_dict; the waves' own code list runs to 11=Nigeria / 12=Ivory Coast / 13=Togo /14=Burkina Faso (a Ghanaian EA cannot be located in Nigeria); REGION varies within one
household in 1,019/3,192 and 1,033/3,136 HH (up to 6 distinct values in ONE household);
within-cluster in 167/170 and 174/176;
Y01A.DCT's variable order is the classicbirthplace→nationality pair. Collapsed,
.first()labelled 9/170 and 7/176 clusters"Nigeria".
No cluster-invariant region source exists in these waves → DROPPED rather than guessed
(class-2 silently-MISSING > class-1 silently-WRONG). Also dropped 1987-88's
Age: AGEY(aperson's age, made into "the cluster's age"). 1987-88 was not in the reported cell list —
its parquet had never been built — so fixing only 1988-89 would have closed the instance and
left the class.
Correction to the original diagnosis (it mattered): the (B) fabrication is currently
MASKED —
Regionis 100% NaN (0/14,924) becauseget_categorical_mappingreturns{}for these waves (a separate bug). The first guard test was therefore vacuous (all-NA →
nunique=0 → passes trivially). Re-tested against the real birth-region values (the worldwhere
region_dictis repaired — plausible, since that bug also breaksBirthplace) itRAISES: non-constant in 174/176 and 167/170. A latent class-1 bug is armed behind a
class-2 bug; dropping
Regioncosts zero rows today and defuses it.(C)
food_security2016-17 — PHANTOM_NAN_ROWS.110 tail rows have
clust=NaN andnh=NaN (all 8 FIES items NaN) but a validhid;groupby'sdropnadropped all 110 outright. Re-keyed oni: hid, which reconstructs{clust}/{nh:02d}with 100.00% fidelity on all 13,899 good rows (byte-identical ids).i/ 13,899 uniqueii/ 14,009 uniqueibyte-identical (
.equals() == True); all 110 recovered hids present insample().FIES_score()already returnspd.NAwhen all 8 items are missing → no fabricatedfood-secure 0.
Proof nothing else moved
_normalize_dataframe_indexinto one process and pushed every cached L2-wave parquet across all countries through
both, with each country's real schema entry — 757 (country × wave × table) cells: 0
differ, 0 non-GhanaLSS deltas. The framework change is data-inert everywhere else; it
alters only warnings. (An independent red-team re-ran this over 729 cells / 32 countries:
0 data differences, 0 raise/no-raise changes, 0 warnings gained; non-GhanaLSS bug: framework silently drops rows on duplicate canonical index (groupby().first()) #323 warning
count 102 base / 102 fixed.)
aggregation:(Benin, CotedIvoire, Burkina_Faso,Senegal, Albania, Togo, Malawi, Guinea-Bissau, Niger) all declare
{visit: first}.visitis an index level, never a column, andfirst≠invariant— doubly unaffected.Confirmed empirically in the sweep, not by grep.
lsms_library/in the worktree): 6 of 7 FAILon pristine base, 7/7 pass post-fix. The 1 that passes both ways
(
test_undeclared_collapse_still_warns) is a regression guard, not a fix-detector.test_normalize_index_j_preserved.py,test_schema_consistency.py,test_table_structure.py,test_id_walk.py,test_add_market_index_dedup.py+ the newfile = 2,198 passed, 2 skipped, 0 failed (2,200 of the 3,427-test suite).
tests/test_currency.py::test_feature_ghana_per_waveFAILS IDENTICALLY onpristine base and with this branch — same assertion,
AssertionError: assert ['GHC'] == {'GHC'}attest_currency.py:222(both diffed). Not touched, not "fixed".LSMS_DATA_DIR. Verified the shared cache still holds the base-config GhanaLSS parquets..pthtrap defeated and asserted: every verification script doessys.path.insert(0, WT)+ asserts'worktrees' in lsms_library.__file__and incountries_root().API level (unchanged, as predicted):
cluster_features3,145 rows / 0 duplicate(t,v);food_security13,892 rows / 0 duplicates / 0 NaNi.Feature('cluster_features')(['GhanaLSS'])assembles, and the base
No rule to make target .../1987-88/_/cluster_features.parqueterror is gone.Base-drift is nil (
git diff d572d8a9..developmentoncountry.pyis empty).Red-team verdicts — all three lenses, including the concerns that did NOT block
[correctness] passes = true, severity =
concern.Every load-bearing claim reproduces exactly under an independently-validated instrument.
Duplicate counts go to zero or to a declared, checked, non-vacuously verified aggregation.
The
invariantguard is genuinely fatal throughCountry(). ONE UNDISCLOSED HOLE (concern,not blocker): the new warning suppression at
country.py:4225keys only onundeclaredbeing empty — but
groupby(level=...).first()also silently drops NaN-KEYED rows(
dropna=True), and_assert_invariant_within_indexis itself agroupby, so it cannot seethem either. A table that declares all its columns
invarianttherefore loses NaN-keyed rowswith ZERO warning — which is exactly the phantom-NaN mechanism this same PR fixes in
food_security2016-17. Not live for GhanaLSS (rows_with_NaN_key= 0 on all five wiredwaves;
invariantis declared nowhere else), so this branch loses no data — but it is aregression in the #323 detector itself, inside the generic framework mechanism other fix-agents
are being pointed at. Flagged loudly here rather than laundered into silence.
[regression] passes = true, severity =
nit.Nothing else moves (see the 729/757-cell differential above). The scanner was rebuilt after
catching its own instrument bug —
yaml.safe_loadsilently skipped the 14 countries whosedata_scheme.ymluses!maketags, including Mali and GhanaLSS themselves, yielding aclean-looking 333-cell sweep with zero GhanaLSS rows — then re-validated on both required
positives. The newly-declared
Ecological_zone: stris a genuine no-op (basevar/alreadystored it as
StringDtype). Nit, not a blocker: the warning gate changed fromif n_dropped:toif n_dropped and undeclared:, so a frame reaching the collapse with zeronon-index columns would suppress the #323 warning for every country regardless of
declaration — unreachable in all 729 cells today, but a new silent-drop path inside a #323 fix.
[soundness] passes = true, severity =
concern.The retained/re-keyed data is CORRECT, not merely present, and the fix guesses nowhere.
(C)
hidis the framework's own composite id (format_id(hid) == f"{clust}/{nh:02d}"on13,899/13,899, 0 mismatches; hid unique 14,009/14,009; the 110 are disjoint, all in
sample(),all 8 FIES items NaN in all 110). (A) the
invariantdeclaration is true at the source.(B) the
Regiondrop is well-founded and refuses to guess. CONCERN (one-line hardening):_assert_invariant_within_indexusesnunique(dropna=True), so a declared-invariant columnthat is 100% NA at check time yields
nunique == 0and passes vacuously — and becauseundeclaredis then empty, the fix also suppresses the pre-existing #323 warning.GhanaLSS 1998-99 is a live instance: its
cluster_featuresL2-wave parquet is 5,998 rowswith
Region0/5,998 andRural0/5,998 non-null (the sameget_categorical_mapping → {}defect found in 1988-89); base warned "had 5698 duplicate tuple(s)", the fix checks nothing and
says nothing. So the exact configuration the PR exists to catch — a person-level column wired as
a cluster attribute in a wave whose categorical mapping is broken — is the one the guard cannot
see, because the broken mapping NA-s out the evidence. No wrong data today (the source is
genuinely invariant) and it is fail-safe if the mapping is repaired, but the
data_scheme.ymlcomment's "Verified cluster-invariant in all five wired waves (max distinct value per cluster =
1)" is 0, not 1, at the layer the guard actually inspects.
Both
concerns are recorded here deliberately rather than fixed in this PR: hardening thedetector (NaN-keyed rows; vacuous all-NA
nunique) belongs with the framework fix for theclass, where it can be validated against all ~44 cells rather than GhanaLSS's 7.
Refs GH #323 (do not close it). Do not merge without human review.
🤖 Generated with Claude Code