Skip to content

fix(#323): Mali 2021-22 cluster_features — delete the 4.3M-phantom-row cartesian (88% of #627's census) #641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ligon merged 1 commit into from
Jul 22, 2026
Merged

fix(#323): Mali 2021-22 cluster_features — delete the 4.3M-phantom-row cartesian (88% of #627's census) #641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions .coder/ledger/323-mali-cartesian.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# Prior-Art Ledger — GH #323 site 4 / #627: Mali `cluster_features` cartesian

> Per-task ledger. Inherits the repo §0 baseline in `STANDING.md`; cites
> `CLAUDE.md` and `lsms_library/data_info.yml` rather than re-copying them.

**Search tier used:** ripgrep + git floor (gitnexus not consulted; this is a
config-only change touching no symbol).

## §1 Task, restated

`Mali/2021-22/_/data_info.yml` declared `cluster_features` as a two-sub-frame
`dfs:` block merged on the cluster key `v`. Both sub-frames were finer-grained
than `v`, so `Wave.grab_data`'s `pd.merge(..., on=['v','t'], how='outer')` was a
many-to-many CARTESIAN PRODUCT: 393,480 × 6,143 → 4,718,148 rows, **4,324,668
of them phantoms** — 88% of the 4,907,774 phantom rows PR #627's 40-country
census found. `_normalize_dataframe_index` then collapsed the result to 513 rows
with `groupby().first()`, so nothing downstream ever saw it. Fix the merge (D1:
core never aggregates the explosion away afterwards). Config-only, no
`lsms_library/*.py`.

## §2 Existing machinery (this task's area)

| symbol | path:line | what it does | tested? | reuse / extend / new |
|--------|-----------|--------------|---------|----------------------|
| `Wave.grab_data` (`dfs:` region) | `lsms_library/country.py:989-1035` (merge at `:1033`) | outer-merges `dfs:` sub-frames on `merge_on ∪ {t}` | yes | **untouched** (core is out of scope) |
| `Wave._cartesian_keys` / `_merge_subframes` | `origin/fix/323-site4-dfs-merge` `country.py:~906-1000` | exact many-to-many detector + phantom count | 16 tests on that branch | **used as the measuring instrument only**; not merged here |
| `_normalize_dataframe_index` | `country.py:4540` | collapses non-unique declared index via `groupby().first()`, audited by `_audit_index_collapse` (`:4214`) | yes | untouched — it is the janitor, not the culprit |
| `_collapse_to_cluster_grain` (site 2, GH #161) | `country.py:4490` | projects household-grain `cluster_features` onto `(t, v)` | yes | **not reached**: Mali declares no `i` in `cluster_features` `idxvars` |
| `Mali/2021-22 sample.df_cover` | `Mali/2021-22/_/data_info.yml:85-97` | already reads `Rural` from `s00q04` of the SAME cover page | — | prior art proving `s00q04` is the milieu column |
| `Mali/2017-18 cluster_features` | `Mali/2017-18/_/data_info.yml:42-60` | reads `Region: s0q01`, `Rural: s0q04` from that wave's cover page | — | prior art for the cover-page-as-geography pattern |

## §3 Definitions & conventions in force

- **`dfs:` merges exist to be collapsed, not grown**: *"Do NOT use `dfs:` merge
blocks just to join `v` from a cover page — collapse to a single-file
extraction"* / *"Existing `dfs:` merges are grandfathered but should be
collapsed when touched"* — `CLAUDE.md`, "`sample()` and Cluster Identity" and
"Gotchas with Teeth".
- **D1, no aggregation in core**: *"Duplicates on a declared index mean the
IDENTIFIER IS BROKEN or a LEVEL IS MISSING — fix the index; do not declare a
reducer"*; `aggregation:` is dead config — `CLAUDE.md`, "Grain Collapse";
`SkunkWorks/grain_aggregation_policy.org` §3a.
- **EHCVM cluster identity**: `v: grappe` (not `[vague, grappe]`),
`i: [grappe, menage]` — `CLAUDE.md`, "EHCVM countries";
`Mali/_/CONTENTS.org` "Sampling Design".
- **Cluster GPS is a displaced cluster fix stamped on every household**, not a
household fix — `CLAUDE.md`, site-2 note (GH #161). Verified for Mali 2021-22
(513/513 grappes carry exactly one distinct coordinate pair).

## §4 Invariants & assumptions

- **A merge key duplicated in BOTH sub-frames is a cartesian by construction** —
`_cartesian_keys` docstring, `origin/fix/323-site4-dfs-merge`. Sound *and*
complete; the row-count-ceiling heuristic is not.
- `ehcvm_conso_*.dta` is at **(household × food item)** grain — 767 rows per
grappe in Mali 2021-22. Any `dfs:` sub-frame drawn from it and keyed only on
`v` is finer than the merge key.
- `s00_me_*.dta` is the **household cover page** — 12 households per grappe.
Also finer than `v`. Two finer-than-key frames ⇒ cartesian.
- `groupby().first()` **skips NA per column**, so a collapsed row can be a
composite assembled from different source rows. Harmless here only because
Region / Rural / Lat / Lon are each provably constant within grappe — which is
now asserted in a test, not in a comment.
- **The warm cache hides this.** The L2-country parquet is written
post-collapse; every before/after measurement here was made in an isolated
`LSMS_DATA_DIR` with only `dvc-cache` symlinked in.

## §5 Reuse decision

| quantity | decision | reason |
|----------|----------|--------|
| `Region` (2021-22) | **repoint**: `ehcvm_conso.region` → `s00_me.s00q01` | same 9 labels, 513/513 grappes agree, constant within grappe; mirrors what 2017-18 already does |
| `Rural` (2021-22) | **repoint**: `ehcvm_conso.milieu` → `s00_me.s00q04` | ditto; `sample.df_cover` in this same file already uses `s00q04` |
| `Latitude` / `Longitude` | **reuse unchanged** (`s00_me.GPS__*`) | already came from the cover page; only the merge around it is deleted |
| the merge itself | **delete** | all four columns live in one file; a `dfs:` block with one source is not a join |
| a reducer / `aggregation:` key | **rejected** | D1 — a reducer on a cartesian "only puts a signature on the corpse" |
| `merge_on: [i]` (keep the merge, fix the key) | **rejected** | works, but leaves a `dfs:` block whose only purpose is to re-join a file to itself at the grain it already has; CLAUDE.md says collapse when touched |

## §6 Open questions for the human

- **`Rural` in `cluster_features` ships the raw French `Urbain`**, while
`sample.Rural` in the same wave maps it to `Urban`. Pre-existing and
corpus-wide (2014-15 shows `Urbain` too); deliberately NOT changed here
because this PR must be value-preserving. Blocks nothing; adjacent to GH #602.
- **2018-19 merges the 366,639-row consumption file against a 549-row
grappe-GPS file.** Not a cartesian (the right side is 1 row per grappe, so
it is m:1) and not fixed here, but it reads 366k rows to produce 551. The
cover page `s00_me_mli2018.dta` would supply Region/Rural at household grain;
the GPS genuinely lives in a separate file there, so the block cannot be
deleted, only slimmed. Deferred: it changes no value and is pure waste.

---
### Phase 3 — verification

- `Mali/2021-22/_/data_info.yml cluster_features` — **OK (anchored on §3, §5)**:
single-file extraction, no `dfs:`, no `aggregation:`; the merge is cured, not
laundered.
- `tests/test_gh323_mali_cartesian.py` — **OK (anchored on §4)**: asserts at the
WAVE level because §4's cache/collapse note means a country-level assertion
passes with the bug present. **Negative control run** (pre-fix YAML restored,
fresh isolated `LSMS_DATA_DIR`): `2 failed, 9 passed` — the wave-level
cartesian test reports `4718148 == 6143` and the structural test reports the
`dfs:` block; the four country-level tests pass *with the bug present*, which
is exactly the blindness the docstring warns about. After the fix: `11 passed`.
- No `lsms_library/*.py` touched — **OK (anchored on §1)**: the D1 core patches
on `origin/fix/323-mali` were deliberately not reused; that branch is about
`pid`, a different defect.
61 changes: 39 additions & 22 deletions lsms_library/countries/Mali/2021-22/_/data_info.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,45 @@ sample:
- t

cluster_features:
dfs:
- df_main
- df_geo
df_main:
file: ehcvm_conso_mli2021.dta
idxvars:
v: grappe
myvars:
Region: region
Rural: milieu
df_geo:
file: s00_me_mli2021.dta
idxvars:
v: grappe
myvars:
Latitude: GPS__Latitude
Longitude: GPS__Longitude
merge_on:
- v
final_index:
- t
- v
# GH #323 (site 4) / GH #627. This block USED to be a two-sub-frame
# `dfs:` merge: Region/Rural from `ehcvm_conso_mli2021.dta` and the GPS
# fix from `s00_me_mli2021.dta`, joined on `v`. That was a CARTESIAN
# PRODUCT, not a join, and by a wide margin the worst one in the corpus:
#
# ehcvm_conso is the EHCVM consumption file at (household x food item)
# grain -- 393,480 rows for 6,143 households in 513 grappes. s00 is the
# household COVER PAGE -- 6,143 rows, also finer than `v`. Merging two
# frames on `v` when BOTH repeat `v` pairs every consumption line-item
# with every household in its grappe: 393,480 x 6,143 -> 4,718,148 rows,
# of which 4,324,668 were phantoms that exist in no survey (88% of the
# 4,907,774 phantom rows PR #627's 40-country census found).
# `_normalize_dataframe_index` then mopped it up with groupby().first(),
# so the table looked clean at 513 rows while nearly 4.4M invented rows
# passed through it.
#
# The merge was never needed. s00_me_mli2021.dta carries the geography
# itself: s00q01 == `region` and s00q04 == `milieu`, verified per grappe
# against ehcvm_conso -- 513/513 grappes agree on BOTH, each is constant
# within its grappe, and the two files cover the identical 6,143
# (grappe, menage) households. So this is a single-file extraction, the
# cartesian is cured at the merge (not laundered by a reducer afterwards),
# and the returned values are bit-for-bit what they were.
#
# Grain note: s00 is still household-grain (12 households per grappe), so
# the declared (t, v) index is collapsed by the framework exactly as
# before -- but now it de-duplicates 12 identical rows per grappe instead
# of ~9,200 (mean 9,197 = 767 line-items x 12 households). Region/Rural
# are provably constant within grappe (checked);
# Latitude/Longitude are the survey's per-grappe displaced fix stamped on
# every household of the grappe.
file: s00_me_mli2021.dta
idxvars:
v: grappe
myvars:
Region: s00q01
Rural: s00q04
Latitude: GPS__Latitude
Longitude: GPS__Longitude


household_roster:
Expand Down
83 changes: 83 additions & 0 deletions lsms_library/countries/Mali/_/CONTENTS.org
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,89 @@ fix should map the numeric codes to canonical labels via Mali's
in each wave's =data_info.yml=); pending that, downstream
consumers should treat Mali separately.

* cluster_features (GH #323 site 4 / GH #627)

** 2021-22 was the worst =dfs:= cartesian in the corpus — fixed

Until this fix, =2021-22/_/data_info.yml= built =cluster_features= from a
two-sub-frame =dfs:= merge:

| sub-frame | file | rows | grain |
|-----------+--------------------------+---------+--------------------------|
| df_main | ehcvm_conso_mli2021.dta | 393,480 | household × food item |
| df_geo | s00_me_mli2021.dta | 6,143 | household (cover page) |

merged =on: [v]= — the *cluster* key. 513 grappes, so each side repeated
every key value (767 consumption line-items and 12 households per grappe).
A join on a key duplicated on *both* sides is a cartesian product by
construction: every line-item paired with every household of its grappe.

: 393,480 × 6,143 -> 4,718,148 rows, 4,324,668 of them phantoms

PR #627's 40-country census found 4,907,774 phantom rows in all;
*this one cell was 88% of them.*

Why nobody noticed: =_normalize_dataframe_index= collapses the declared
=(t, v)= index with =groupby().first()=, which reduced 4.7M rows to 513 —
and the L2-country parquet is written *after* that collapse. The table
looked clean at every point anyone ever looked. The phantoms existed only
in the wave-level frame, upstream of the collapse. (See CLAUDE.md, "Grain
Collapse": /the bug hid behind the cache the bug poisoned/.)

** The merge was never needed

=s00_me_mli2021.dta= carries the geography itself. Verified against the
data, not inferred:

| canonical | cover page | consumption file | agreement |
|-----------+------------+------------------+--------------------|
| Region | =s00q01= | =region= | 513/513 grappes |
| Rural | =s00q04= | =milieu= | 513/513 grappes |
| Latitude | =GPS__Latitude= | (absent) | — |
| Longitude | =GPS__Longitude= | (absent) | — |

Both are *constant within grappe* in both files, the two files cover the
identical 6,143 =(grappe, menage)= households, and the label sets are the
same 9 regions / 2 milieux. So the block is now a *single-file
extraction* from the cover page. Per GH #323 decision D1 the cartesian is
cured *at the merge*; no reducer, no =aggregation:= key — a reducer on a
cartesian only signs the corpse.

The result is bit-for-bit unchanged: all 3,006 country-level rows across
all four waves, all four columns, identical before and after (=DataFrame.equals=
=True=). This fix removes 4,324,668 invented rows and alters no value.

Prior art within this same file: =sample.df_cover= already reads =Rural=
from =s00q04= of this very cover page, and the 2017-18 wave already reads
=Region: s0q01= / =Rural: s0q04= from *its* cover page.

Pinned by =tests/test_gh323_mali_cartesian.py= (wave-level row counts for
all four waves — a country-level assertion cannot see this bug).

** The other three waves are not cartesian

Their =df_geo= is one row per grappe, so the merge is many-to-one:

| wave | df_main | df_geo | wave rows | grappes |
|---------+--------------------------------+-------------------------------+-----------+---------|
| 2014-15 | eaci2014_agregatconso (3,804) | eaci_geovariables_2014 (989) | 3,804 | 989 |
| 2017-18 | eaci17_s00p1 (8,390) | eaci_geovariables_2017 (953) | 8,390 | 953 |
| 2018-19 | ehcvm_conso_mli2018 (366,639) | grappe_gps_mli2018 (549) | 366,639 | 551 |
| 2021-22 | s00_me_mli2021 (6,143) | — (single file) | 6,143 | 513 |

*Deferred, not a defect*: 2018-19 still reads the whole 366,639-row
consumption file to produce 551 cluster rows. It is wasteful, not wrong —
its geo file is grappe-grain so no rows are manufactured — and it cannot be
collapsed to one file the way 2021-22 was, because the GPS genuinely lives
in a separate =grappe_gps_mli2018.dta= there. Slimming =df_main= to the
cover page =s00_me_mli2018.dta= would be a pure-performance change; left
alone so this PR alters no value anywhere.

*Known cosmetic inconsistency (unchanged here)*: =cluster_features.Rural=
ships the raw French =Urbain= while =sample.Rural= maps it to =Urban=.
Pre-existing and visible in 2014-15 too; fixing it would change returned
values, which this PR deliberately does not do. Adjacent to GH #602.

* Household Presence / MonthsSpent

The EHCVM questionnaire (used in the 2018-19 and 2021-22 waves) does
Expand Down
Loading
Loading