Skip to content

fix(#323): Albania cluster grain — a lone mover was setting a whole cluster's district#620

Open
ligon wants to merge 1 commit into
developmentfrom
fix/323-albania-config
Open

fix(#323): Albania cluster grain — a lone mover was setting a whole cluster's district#620
ligon wants to merge 1 commit into
developmentfrom
fix/323-albania-config

Conversation

@ligon

@ligon ligon commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Config-only. Zero changes to the access path (country.py / feature.py / local_tools.py) — core is owned by PR #614 (Site 1) and #617 (Site 2). Supersedes the core patch on fix/323-albania, per slurm_logs/DESIGN_grain_collapse_sites_2026-07-13.org.

The fix

Converts 4 waves of cluster_features from a YAML extraction — whose i: hh idxvar fed a silent household→cluster .first() — into script-path builds calling albania.cluster_reduce, a country-level helper in countries/Albania/_/albania.py. The reduction is now the country's, explicit and checked, not core's and silent.

Plus a genuine semantic fix. Albania is a panel that follows movers, and 2004's m0_distr is the household's current district while the PSU code is its original one. So .first() was handing movers' districts to whole clusters. Six clusters were wrong; 223 → GRAMSH was decided by a lone mover.

Also drops sentinel "PSUs" 995/999, which are not clusters (450 → 448).

Verified (cold, LSMS_NO_CACHE=1)

  • Clusters per wave: 450 / 449 / 448 / 480; sentinels 995/999 gone.
  • District corrections: PSUs 43/44/47/52/53 → KUKES; 16 → SHKODER, 259 → MALLAKASTER, 280 → SKRAPAR, 297 → KORCE, 344 → TEPELENE; 223 → GRAMSH (the lone-mover case).
  • 0 sentinel v values in sample().
  • Tests: 20 passed + 78 structural.

Structural check

Albania is a panel with movers — the 2004 follow-up reuses the original 2002 PSU, which is the entire point of the fix. It also carries a GH #445 waves list excluding 1996; the albania.py diff is two hunks (import warnings + an EOF append) with zero deletions, so that list is intact. No panel_ids, hence no id_walk chain to disturb.

Refs #323.

🤖 Generated with Claude Code

@ligon
ligon force-pushed the fix/323-albania-config branch from 68d013c to afdbfc4 Compare July 21, 2026 22:07
…el fix (#323)

Config/script only: no change to lsms_library/country.py, feature.py or
local_tools.py, and no data_scheme `aggregation:` key.  Per Ethan's decisions
in slurm_logs/DESIGN_grain_collapse_sites_2026-07-13.org (D1: core does not
aggregate), the core patch on fix/323-albania is dropped -- Site 1 is owned by
PR #614 and Site 2 by its own PR.  The original branch is left intact as the
audit trail.

Refs #323

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ligon
ligon force-pushed the fix/323-albania-config branch from afdbfc4 to 010f2b0 Compare July 22, 2026 02:52
@ligon

ligon commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

Adversarial review — APPROVE_WITH_NOTES

I tried to break this cold and could not break the data. Every headline number reproduces, and the 2004 semantic claim survives independent corroboration it did not ask for.

Method. Two detached worktrees (origin/development @ 45aee170 vs origin/fix/323-albania-config @ 010f2b0f), two isolated LSMS_DATA_DIRs with only dvc-cache symlinked to the shared blob store — genuinely cold, not LSMS_NO_CACHE=1. LSMS_COUNTRIES_ROOT + PYTHONPATH pinned per tree, lsms_library.__file__ asserted. No dvc CLI.

What I re-derived myself

claim measured
clusters 450 / 449 / 448 / 480 ✅ cold, both trees (base = 450/449/450/480)
sentinels 995, 999 gone ✅ only rows in base and not branch are ('2004','995'), ('2004','999')
83 sentinel households, 39×995 (all m0_orhh='no') + 44×999 (all 'yes'), 0 key into 2002 (psu,hh), spanning 23 districts ✅ exact
1,713 of 1,714 real (m0_q01,m0_q02) key into 2002 ✅ exact
448 real PSUs, 49 single-household ✅ exact
PSU 16 = 4v1, 259 = 3v1, 280 = 5v1, 297 = 6v1, 344 = 3/2/2, 223 = n=1 ✅ exact
43/44/47/52/53 are 1-vs-1 KUKES/TIRANE ties, all 2002 code 17 ✅ exact
2003: 8,679 rows, 711 null PSU, 449 distinct PSU, STRATUM nunique>1 = 0 clusters ✅ exact

The attacks that mattered

Could the anchor have scored well by splitting real entities? No — it does the opposite. I recomputed Region from the 2002 anchor and compared it to the within-2004 plurality district for all 448 PSUs: 447 agree, 1 disagrees — PSU 223, the n=1 lone-mover case, exactly as claimed. The anchor is not carving clusters up along movers' destinations; it is overriding first() in precisely the places first() picked a minority report.

Is the learned code→name map actually right, or just internally consistent? I dumped it and checked it against something outside this repo: it is exactly the official INSTAT 36-district numbering, including the Albanian-collation quirks — 9=GRAMSH before 10=GJIROKASTER, 31=SKRAPAR before 32=SHKODER, 14=KORCE, 17=KUKES, 16=KUCOVE. A map learned from noisy 2004 self-reports reproducing the official ordering across all 36 codes is strong external corroboration that m0_q1a is the district code and the anchor is real.

Invariance by missingness? Checked both directions. 0 NaN districts in the 2004 source, so the nunique(dropna=True)==1 & size>=2 guard is not being satisfied by nulls. And 0 clusters end with <NA> Region — the anchor loses no coverage relative to first(), so this isn't "correct because empty".

Is the >=2 households guard load-bearing, or decoration? Mutation-tested it: dropping it to >=1 makes code 9 ambiguous ({GRAMSH, ELBASAN} — PSU 223's lone mover) and yields 14 clusters with <NA> Region instead of 0. Claim confirmed.

Does the new test discriminate? Yes. Copied tests/test_albania_cluster_features.py onto the pristine development config: 11 failed / 9 passed. On the branch: 20/20. The 9 that pass on both are honestly-labelled (grain, the three unchanged wave counts, and the KUKES ties the PR itself says "stay KUKES"). Plus the 83 existing #323/grain tests are green.

Conservation — did anything else move? Every one of Albania's 10 tables, cold on both trees, is identical once v is dropped (canonical sort on the full frame, unique keys). Row counts identical to the row. The only v changes are the 78 sentinel households (38×995, 40×999) → <NA>, propagating to 373 roster rows and to Mover in household_characteristics — via the pre-existing mover_sentinel in transformations.py, not a new mechanism. No rows dropped by the NA index key.

Cache. A cache written by pre-fix code, then read under post-fix config, auto-rebuilds (448, sentinels gone, 223→GRAMSH). The v0.8.0 hash does cover this change.

Core's own instrument agrees. On development, Country('Albania').cluster_features() raises 2 GrainCollapseWarnings (Site 2, already merged). On this branch: 0.

Scope. Config + tests + ledger only. No lsms_library/*.py. No aggregation: key. D1 respected — the reduction is the country's, in the country's build script, and for 2004 it isn't a reducer at all but a deterministic identifier→attribute lookup.


Notes (none block merge)

1. (MEDIUM) albania.cluster_reduce reinvents build_transforms.collapse_to_cluster_grain. That helper landed on development in 48f4d08f ("shared explicit-reducer helpers", 2026-07-13) — i.e. it is in this PR's base — and its docstring describes this exact case. The behaviours differ where it counts: the shared helper defaults to on_conflict='raise', requires the caller to write on_conflict='na' to get blanking, and honours LSMS_GRAIN_STRICT=1; cluster_reduce hardcodes the lenient warn-and-blank mode with no strict escalation, and is outside the corpus-wide contract tests (test_gh323_explicit_reducers.py, test_gh323_grain_contract.py). No data impact today — I confirmed 0 conflicts fire in any of the four waves — so this is a latent policy divergence and a duplicate to maintain, not a defect. The ledger §4 lists prior art consulted and does not mention it (understandable: same-day landing). Worth either switching to collapse_to_cluster_grain(..., on_conflict='na') or filing a follow-up.

2. (LOW) A doc claim that does not reproduce. CONTENTS.org ("The old silent collapse emitted them as a v = NaN cluster row"), the 2003 script docstring ("2003 yields 449 real clusters, not 450"), and the test docstring all imply 2003 went 450 → 449. Measured cold on development, 2003 already returns 449, with no NaN-v row in the output — core drops it downstream (Site 3). The ledger §6 table has this right (449 → 449); the prose does not. test_cluster_counts[2003-449] therefore passes before and after — harmless, but it is not evidence of anything.

3. (LOW) Cache-hash gap on the 2002 anchor. 2004/_/cluster_features.py reads ../../2002/Data/metadata_cl.dta, which is not a declared source of the 2004 wave, so its DVC sidecar md5 is not in 2004's input hash (only the script text is). If that 2002 blob is ever re-materialized, 2004's cluster_features will not self-invalidate. Worth a line in CONTENTS.org.

4. (note) .coder/coverage/blessed.csv untouched. These 2004 numbers were read one by one; blessing is a human act, so leaving it empty is the honest default — flagging only so the choice is deliberate.

Verdict: APPROVE_WITH_NOTES.

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