Skip to content

fix(#637): Tanzania key-soundness review — 2008-15 shocks was keyed on the panel LINE, not the household#651

Open
ligon wants to merge 1 commit into
developmentfrom
fix/637-tanzania-key-soundness
Open

fix(#637): Tanzania key-soundness review — 2008-15 shocks was keyed on the panel LINE, not the household#651
ligon wants to merge 1 commit into
developmentfrom
fix/637-tanzania-key-soundness

Conversation

@ligon

@ligon ligon commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Key-soundness review of Tanzania's groupby().first() sites — the largest remaining block on GH #637, and one that had never been reviewed.

Per the corrected reading on #637: the per-column NA-skip is a correct completion, and the composite is wrong only where the key merges different real entities — a broken identifier, fixed per #323 D1 at the identifier. So this reviews the keys, not .first(). No .first(skipna=False) (withdrawn as a regression), no aggregation: keys, nothing under lsms_library/*.py outside countries/Tanzania/**.

All measurements are cold builds: in-process patch of DataFrameGroupBy.first + runpy of each script (which sidesteps the materialize: make subprocess blind spot), isolated LSMS_DATA_DIR with dvc-cache symlinked, LSMS_COUNTRIES_ROOT asserted at the worktree. No dvc CLI invoked. cluster_features untouched — PR #642 owns it.

Verdicts — 21 sites

site key dup groups exact compl. conflicting verdict
2008-15/_/anthropometry.py:70 (t,i,pid) 0 sound
2008-15/_/food_coping.py:76 (t,i,Strategy) 54,495 54,495 0 0 sound — de-replication
2008-15/_/household_roster.py:37 (t,i,pid) 0 sound
2008-15/_/housing.py:43 (t,i) 8,488 8,488 0 0 sound — de-replication
2008-15/_/individual_education.py:52 (t,i,pid) 0 sound
2008-15/_/life_satisfaction.py:106 (t,i,Domain) 0 sound
2008-15/_/shocks.py:68 (t,i,Shock), i = UPHI 0 BROKEN — fixed here
2019-20/_/{anthropometry,food_coping,individual_education,life_satisfaction,shocks}.py 0 each sound
2020-21/_/{anthropometry,food_coping,individual_education,life_satisfaction,shocks}.py 0 each sound
_/anthropometry.py:59 (t,i,pid) 0 sound
_/individual_education.py:59 (t,i,pid) 0 sound
_/tanzania.py:1686 (people_last7days) (t,i,pid) 0 (both waves) sound
_/tanzania.py:1822 (community_prices) (t,v,j,u) 261 / 1,281 18 / 132 0 243 / 1,149 grain deliberate; arbitrary resolution flagged, not fixed

Every site gets a comment recording what was checked and the counts.

The exact column is not the evidence. For housing and food_coping the verdict rests on something stronger: that no source column at all differs within a duplicate group, plus a lineage check. #646's trap 1 is real and I did not lean on "exact".

The one broken key: 2008-15 shocks keyed on the panel LINE

2008-15/_/shocks.py built i from UPHI, the NPS panel-tracking line index (1..14985), not from r_hhid, the household id sample, household_roster, updated_ids.json and every other Tanzania table use. Two silent consequences.

1. Not one 2008-15 shocks row could be joined to its own household. The namespaces share zero values. Per wave, overlap of shocks().i with household_roster().i:

wave distinct i overlap before overlap after
2008-09 5,587 → 2,951 0 2,951 (100%)
2010-11 6,875 → 3,285 0 3,285 (100%)
2012-13 7,745 → 3,842 0 3,842 (100%)
2014-15 3,634 → 3,166 0 3,166 (100%)
2019-20 502 502 502 (untouched)
2020-21 2,552 2,552 2,552 (untouched)

The 2019-20 / 2020-21 waves key on sdd_hhid / y5_hhid and always overlapped by 100%, which is exactly why an aggregate check would have missed this. id_walk also matched nothing: updated_ids.json is r_hhid-keyed.

2. The panel-line replication survived as data. The upd4 household-level modules are keyed on the LINE, so a household-round arrives once per DESCENDANT line — the same replication sample.py:54 documents for the cover page (29,250 rows → 16,540 household-rounds, group sizes 1..11). Keyed on UPHI, each copy became its own "household":

round 1, r_hhid 01010140020171  — 3 shocks, recorded on UPHI lines 1, 2, 3:
  UPHI 1  CROP DISEASE OR CROP PESTS    INCOME LOSS
  UPHI 2  CROP DISEASE OR CROP PESTS    INCOME LOSS
  UPHI 3  CROP DISEASE OR CROP PESTS    INCOME LOSS
  UPHI 1  DEATH OF OTHER FAMILY MEMBER  NEITHER
  UPHI 2  DEATH OF OTHER FAMILY MEMBER  NEITHER
  UPHI 3  DEATH OF OTHER FAMILY MEMBER  NEITHER
  UPHI 1  LARGE RISE IN PRICE OF FOOD   INCOME LOSS
  UPHI 2  LARGE RISE IN PRICE OF FOOD   INCOME LOSS
  UPHI 3  LARGE RISE IN PRICE OF FOOD   INCOME LOSS
  old key -> households "1", "2" and "3", 9 rows, 3 shocks counted 3x

74,341 wave rows stood for 39,724 household-shock facts — any count of who suffered a shock was inflated ~1.9x. Per wave, raw 20,039 / 22,308 / 22,216 / 9,778 → de-replicated 10,241 / 10,216 / 10,741 / 8,526.

The fix is the identifier (D1): i = r_hhid. That re-exposes the replication as duplicate index entries, which the existing .first() collapses — and here the collapse is de-replication, not a choice: over the whole module (449,435 rows, 134,035 duplicate (round, r_hhid, hr_00) groups) zero groups differ on any of the nine hr_* columns. The script now asserts that before collapsing, the discipline sample.py:76 already applies to the same replication, so a future re-release that makes lines disagree fails loudly instead of silently keeping one.

At the API: 60,463 → 35,746 rows, all 13,252 (t, i) pairs known to sample() (0 unknown), and id_walk produces 0 collisions on the re-keyed table.

Cited context: GH #132 filed the 60% duplicate (t, i, Shock) rate; keying on UPHI removed the duplicates without fixing the entity. GH #114 documents map_08_15's split-off→parent collapse — shocks now shares that (separately-tracked) convention instead of standing outside the panel.

Sound: the two de-replication sites (housing, food_coping)

Same UPHI replication, and the collapse discards exact copies.

  • housing (upd4_hh_i1): 29,250 rows → 16,540 household-rounds, 8,488 duplicated (sizes 2:5477, 3:2247, 4:494, 5:164, 6:61, 7:31, 8:7, 9:4, 10:1, 11:2). Across all 8,488 groups, ZERO differ on ANY of the 74 hi_* columns — including the continuous ones and including the 59 round-4 groups sample.py flags as cluster-ambiguous. Corroborated by lineage rather than by "exact": a round-1 household with k lines maps to k distinct round-4 households in 174 of the 211 cases where more than one of its lines is still observed in round 4.
  • food_coping (upd4_hh_h): 23,122 rows → 13,275 household-rounds, 6,814 duplicated; zero differ on ANY hh_* column, not merely the eight items read. Per-round duplicate counts (R2 2,698 / R3 3,628 / R4 488) match housing's exactly — the same replication through a second module.

Sound: the 15 dead-code sites

.first() is never reached. The verdict rests on the key, not on today's counts:

  • 2008-15 individual modules (_b roster, _v anthropometry, _c education, _g satisfaction) carry (r_hhid, UPI) and no UPHI column, so the replication does not reach them: 83,706 rows / 83,706 groups / 0 duplicates / 0 null UPI each, and the pid formatting is injective (45,396 distinct UPI → 45,396 distinct pid).
  • head reductions checked directly, since they are what make (t, i, Domain) a key: exactly one HEAD per household-round — 16,540/16,540 (2008-15), 1,183/1,183 (2019-20; one of that wave's 1,184 households records no head), 4,709/4,709 (2020-21).
  • 2019-20 / 2020-21: per-person sources (hhid, indid)-unique with 0 nulls; format_id injective over each wave's households (1,184 → 1,184; 4,709 → 4,709); shocks (hhid, shock_id)-unique after the YES filter.
  • country-level concatenators: wave parquets already unique, folders contribute disjoint t, id_walk renames only i and produced 0 collisions.

Trap 2 (invariance by missingness) does not apply — every column tested is densely populated.

Flagged, not fixed: community_prices

(t, v, j, u) is the declared grain, and the collisions are exactly the intended harmonize_community_price label lump — checked rather than assumed: of 261 colliding groups in 2019-20 and 1,281 in 2020-21, 261 / 1,281 span more than one source item code and ZERO repeat a code. The survey never prices the same item twice in a cluster, and no additional level exists that would not break the shared j this feature exists to join on.

What the collapse does do is resolve genuine disagreement arbitrarily — 243 / 1,149 groups hold more than one distinct observed price:

2020-21  cluster 01-06-47-85  "Millet & Sorghum (grain)"  per Kg
    item 6:   2,000 TSh
    item 7:  16,000 TSh
    -> .first() keeps 2,000; the other observation disappears

That is the #323 "arbitrary resolution of two OBSERVED values" discussion, explicitly not the #637 completion question, and per D1 the answer may not be a reducer. Recorded at the site with the measurements rather than half-fixed.

Also corrected there, doc only: the comment claimed the collision was combined "by their MEAN"; the code has always taken the first. Two contradictory paragraphs sat one line apart.

Test

tests/test_gh637_tanzania_shocks_household_id.py, 11 tests: per-wave shocks.i ⊆ sample.i; per-wave shocks.i ⊆ household_roster.i; ids are not bare line indices; per-wave row counts (which also pin that 2019-20 / 2020-21 were untouched); and household 01010140020171 reporting each 2008-09 shock exactly once.

Follows the #323 instrument note: asserts the identifier and row counts, never post-collapse index uniqueness — uniqueness held perfectly with the bug present, which is how it hid.

Negative control — against a pristine origin/development config tree (git archive into a separate LSMS_COUNTRIES_ROOT + a cold data dir), all 11 fail:

FAILED test_shocks_i_is_a_household_known_to_sample[2008-09]  - 5587 of 5587 unknown to sample()
FAILED test_shocks_i_is_a_household_known_to_sample[2010-11]
FAILED test_shocks_i_is_a_household_known_to_sample[2012-13]
FAILED test_shocks_i_is_a_household_known_to_sample[2014-15]
FAILED test_shocks_i_is_not_a_bare_line_index[2008-09 .. 2014-15]
FAILED test_shocks_per_wave_row_counts
FAILED test_a_replicated_household_reports_each_shock_once
FAILED test_household_ids_are_shared_across_shocks_and_roster
11 failed

With the fix: 11 passed. test_tanzania_grain_gh323.py, test_tanzania_community_cluster_xwalk.py, test_gh323_grain_contract.py, test_gh323_explicit_reducers.py, test_gh323_benin_togo.py and the new module together: 73 passed, 2 skipped. test_schema_consistency.py + test_panel_id_collisions.py filtered to Tanzania: 8 passed. Every Tanzania table still builds with a unique index (shocks 35,746 / housing 22,430 / food_coping 153,315 / household_roster 112,884 / anthropometry 78,674 / individual_education 43,367 / life_satisfaction 152,048 / people_last7days 29,179 / community_prices 15,831 / sample 22,433), and Feature('shocks')(['Tanzania']) assembles. The GrainCollapseWarnings in that run are the pre-existing #323 Tanzania/cluster_features findings PR #642 addresses — unrelated.

The test imports the shared skip marker as from tests.conftest import requires_s3: tests/ is a package and the repo root has its own conftest.py, so the bare from conftest import ... spelling in tests/conftest.py's docstring resolves to the wrong module.

Scope

Config/script only — every changed file is under lsms_library/countries/Tanzania/**, plus the new test and .coder/ledger/637-tanzania-keys.md. cluster_features untouched (PR #642). CONTENTS.org deliberately untouched to avoid conflicting with #642's 170-line addition there; the findings live in the site comments and the ledger.

Refs #637.

🤖 Generated with Claude Code

…he household

Key-soundness review of Tanzania's 21 `groupby().first()` sites (GH #637).
Per the corrected reading on that issue the per-column NA-skip is a correct
completion, so this reviews the KEYS, not `.first()`.  No `.first(skipna=False)`,
no `aggregation:` keys, nothing under `lsms_library/*.py`.

20 of 21 keys are sound; 15 of those never reach `.first()` at all and two
(housing, food_coping) collapse only the NPS panel-LINE replication that
`sample.py:54` already documents -- de-replication, not a choice: 0 of 8,488 and
0 of 6,814 duplicate groups differ on ANY source column, and lineage confirms
the copies are one household (a round-1 household with k lines maps to k
distinct round-4 households in 174 of 211 observed cases).

BROKEN: `2008-15/_/shocks.py` built `i` from UPHI, the panel-line index
("1".."14985"), not `r_hhid`.  The two namespaces shared ZERO values, so no
2008-15 shocks row could be joined to its own household and `id_walk` matched
nothing (`updated_ids.json` is r_hhid-keyed); and the line replication survived
as data -- 74,341 rows standing for 39,724 household-shock facts, inflating any
shock count ~1.9x.  Fixed at the identifier per #323 D1: `i = r_hhid`, which
re-exposes the replication as duplicates that the existing `.first()` collapses,
now guarded by an assert (0 of 134,035 duplicate groups differ on any hr_*
column).  API: 60,463 -> 35,746 rows; per-wave overlap with household_roster
0/0/0/0 -> 100%/100%/100%/100%; 0 (t,i) pairs unknown to sample().

Flagged not fixed: `_/tanzania.py` community_prices resolves genuinely
disagreeing observed prices arbitrarily (243/261 and 1,149/1,281 colliding
groups) -- the #323 Option-B discussion, not #637.  100% of its collisions span
>1 source item code and 0 repeat a code, so the grain is the intended
harmonize_community_price label lump and no level is missing.  Its comment
claiming a MEAN was corrected to match the code (doc only).

Every site carries a comment recording what was checked and the counts.
Ledger: `.coder/ledger/637-tanzania-keys.md`.

Test: tests/test_gh637_tanzania_shocks_household_id.py (11 tests).  Negative
control against a pristine origin/development config tree: all 11 fail.

cluster_features untouched -- PR #642 owns it.

Refs #637.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ligon

ligon commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

⚠️ DO NOT MERGE the shocks.py change — I believe it is a regression, and the fault is mine

I briefed this review with a framing that was wrong for Tanzania, and the 2008-15/_/shocks.py "fix" follows from it. Flagging before it lands.

What I got wrong

The PR treats i = UPHI as a broken key because UPHI values do not appear in household_roster, and because the module "replicates a household-round once per descendant line". I endorsed that reading and generalised it into a "third trap" on #637.

Tanzania handles split-offs differently from most or all other surveys in this library. When a child leaves and forms a new household, the NPS does not merely add a new household going forward — it creates new observations in the EARLIER waves too, back-casting the ancestor households record onto each descendant.

The variable label says exactly this:

UPHI    Uniform Panel Household Identifier at NPSY4
r_hhid  NPS HHID: y1_hhid, y2_hhid, y3_hhid, y4_hhid

UPHI identifies the panel household as defined at wave 4, propagated backwards. r_hhid is the round-specific id.

The data confirms it

From upd4_hh_r.dta:

round rows distinct UPHI distinct r_hhid
1 20,039 5,594 2,956
2 155,097 8,163 3,924
3 189,962 9,998 5,010
4 84,337 4,961 4,341

In round 1, 1,526 r_hhid values carry more than one UPHI, up to 11. That is the back-cast signature, not a replication bug: one 2008-09 household appears once per descendant household it eventually produces.

So the "1.9x inflation" is the surveys deliberate panel construction, and UPHI is the correct identifier for this grain. Re-keying to r_hhid collapses 5,594 Y4-panel households in round 1 back into 2,956 round-1 households and destroys the panel linkage. The 60,463 -> 35,746 row reduction is that destruction, not a recovery.

What IS real here

The zero overlap with household_rosters i is a genuine interoperability problem — these two tables cannot be joined. But the likely resolution runs the other way: establish which id convention Tanzanias 2008-15 tables should share (and whether the roster itself should be on UPHI, or a documented UPHI <-> r_hhid mapping is needed), rather than dropping shocks to the round id.

See .claude/skills/tanzania-panel-design.md (NPS extended vs refresh sub-panels) — this belongs in that design space.

Requested

  • Drop or revert the shocks.py re-key and its 11 tests, which assert the collapsed counts.
  • The other 20 site verdicts in this PR are unaffected by this and look sound — worth keeping, ideally split out so they are not blocked.
  • Record the back-casting in Tanzania/_/CONTENTS.org, since it is a genuine cross-cutting fact about this country that nothing in the repo currently states.

Apologies for the misdirection — the brief was mine.

@ligon

ligon commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

Follow-up: this IS documented, I failed to read it, and the precedent cuts both ways

The maintainer pointed me at existing documentation. It exists, and I should have read it before either endorsing the re-key or flagging it. Process failure on my part: the add-feature skill says plainly "Read existing documentation FIRST … {Country}/_/CONTENTS.orgStart here", and neither my brief to the reviewing agent nor my own correction did that. Two rounds of over-confident framing followed from skipping one file.

Sources:

What the record says

A single r_hhid in round 1 … can correspond to multiple UPHIs. This happens because the UPD4 retrospectively assigned individual-level panel identifiers (UPHIs) to household members, and when household splits occurred in later rounds, the original r_hhid was associated with all resulting UPHIs.

So the back-casting is real and documented. My measurement — round 1: 2,956 r_hhid vs 5,594 UPHI, up to 11 per household — is the same phenomenon. That part of the flag stands.

But "UPHI is the correct identifier" was too strong

The same document records the opposite decision for the sibling table:

the food acquisition data (quantities, values) is identical across the duplicate rows (only UPHI differs) … The country-level script handles this with a blunt x.loc[~x.index.duplicated(),:] … Row counts halve from 408,707 to 228,281.

For food_acquired the repo already chose not to carry UPHI multiplicity — payload identical, household-level fact should not be multiplied by descendant count. That is exactly the argument for this PR's shocks re-key.

UPHI is also described as individual-level in origin ("assigned … to household members"), a further reason not to treat it as simply "the household id".

Where that leaves the PR

I no longer think the re-key is obviously wrong. I think it is an undecided design question that this PR would settle silently, and the two tables end up inconsistent either way:

table grain effect
food_acquired deduplicated to r_hhid one row per household-round; descendant multiplicity discarded
shocks (before) UPHI one row per descendant-household-round
shocks (after) r_hhid consistent with food_acquired

Consistency argues for the change. Against it: #114's own framing treats the blunt dedup as a problem it was working around, not settled policy — and the CONTENTS.org entry is still marked WAITING even though the issue is closed.

Requested

Do not merge the shocks.py re-key as an incidental part of a key-review PR. Either way it is a grain decision about a published table and deserves to be made explicitly, with #114 and .claude/skills/tanzania-panel-design.md in view. The other 20 site verdicts are unaffected and should not be held up by it.

Documentation gap worth closing regardless

CONTENTS.org documents the r_hhid/UPHI many-to-many only in the context of food_acquired. It does not state a general rule for which identifier other 2008-15 tables should use — which is precisely why this PR could reach the opposite answer in good faith. Whatever is decided here should be written there as a country-level rule, not left as a per-table accident.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant