Skip to content

fix(polities): make an aggregation bucket sum, under exactly one area label - #591

Merged
lbm364dl merged 1 commit into
mainfrom
polity/563
Aug 5, 2026
Merged

fix(polities): make an aggregation bucket sum, under exactly one area label#591
lbm364dl merged 1 commit into
mainfrom
polity/563

Conversation

@lbm364dl

@lbm364dl lbm364dl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

What was wrong

.aggregate_to_polities() grouped by the member's polity_name alongside
polity_area_code, then renamed the pair to area / area_code. So a bucket
that folds members resolving to different polities was never actually
summed: it came back as several rows under one area_code, under different
area labels. Mass conserved, polity_area_code was untouched, bucket
membership was identical — which is why #480 passed every check anyone ran and
still had to be reverted (#561).

The issue frames this as a blocker for a future un-fold. It is not: it is
live on the shipped crosswalk today.
Bucket 206 "Sudan (former)" folds
FAOSTAT areas 206, 276 Sudan and 277 South Sudan, and from 2012 those resolve
to three different polities. So main has been emitting a split bucket for
thirteen years of four different sources, and the split was not cosmetic —
downstream it both dropped and doubled data (numbers below).

.read_crop_residues() (R/build_cbs.R) duplicated the same aggregation
inline, which is the second half of the hazard: two sites emitting two area
vocabularies for one area_code is exactly what dropped 702,166 rows in #382.

Evidence it reproduced BEFORE the change

Driving the real readers over the real pins, at the base commit, full range:

source (area_code, year) under >1 label duplicate (area_code, year, item, element, unit) keys
faostat-fbs-new 4 2,056
faostat-production 13 2,000
faostat-trade-totals 13 3,739
faostat-emissions-livestock 12 144
faostat-fbs-old, -cbs-old-crops, -cbs-old-animal, -production-old 0 0

Every split is bucket 206, labels South Sudan | Sudan. Re-measured after
rebasing onto today's main (which had moved by #408/#405): identical.

The consequence, traced to the raw pin rather than asserted:

bucket 206 raw faostat-production build_primary_production() before after
goats, 2018, head 40,846,000 (276) + 14,449,249 (277) = 55,295,249 14,449,249 — Sudan's 40.8 M dropped 55,295,249
sugar cane, 2019, t 5,449,000 (276 only; 277 does not report) 10,898,000 — doubled 5,449,000

What I changed

  1. polity_name is no longer a grouping key. The label is attached after
    the sum, from the bucket's own code resolved for that year — which is
    what polity_bucket_coverage() already documents a fold as being called and
    what add_reporting_polity_columns() already resolves, so the aggregator now
    agrees with both instead of using a third rule. That is option 2 of the
    issue's three, carrying option 1's labelling rule.
  2. Two shared helpers, .bucket_area_labels() and
    .apply_bucket_area_labels(), in R/polity_folds.R; .read_crop_residues()
    uses them too, so the two aggregation sites cannot diverge.
  3. The label step is an update-join, not a merge, so it provably cannot drop
    or reorder a row — a missing label costs a label, never a row. That is the
    polities: link the package to whep-polities and stop resolving to dead polities #382 failure mode made structurally impossible rather than tested for.
  4. A bucket whose own code resolves to no polity for the year (an aggregate
    whose period has ended) falls back to the member label, deterministically by
    lowest area_code, so no row is left unlabelled.

How I verified

Structure, not just totals#480 is the reason:

  • Reader level, full range, all eight sources: splits 42 → 0, duplicate
    keys 7,939 → 0, and each source's total is unchanged to the digit
    (e.g. faostat-fbs-new 948,260,434,000 before and after). Row counts fall by
    exactly the duplicate-key count: fbs-new −2,056, production −2,000,
    trade-totals −3,739, emissions-livestock −144.
  • build_primary_production(), full range: 6,170,595 rows before and after,
    key set identical (0 keys added, 0 removed), 1,673 keys change value, all
    in 2012-2023 and all in bucket 206 except two derived per-head ratios for
    Belgium in 2023 (0.0062 and 0.0006). Both directions of change verified
    against the raw pin (table above).
  • build_commodity_balances() on real data, 2005-2020 (a full-range build
    was OOM-killed on a machine shared with nine agents; this range covers every
    affected year): 1,276,337 → 1,276,340 rows, 678 keys change of which 559 are
    bucket 206. Every other area moves 43.4 t in total across 119 keys, largest
    single move 3.91 t — 4e-9 % of the build, trade-balancing knock-on.
    Element totals: stock_variation +1.79 %, everything else within 0.03 %.
    reporting_polity_code for bucket 206 is SUD-1956-2011 before and after.
  • The one-area_code-one-area guard in test_read_raw_inputs.R (the one
    added after the fix(polities): give regions.csv's grid countries and pre-2011 Sudan their real polities #480 revert) still passes, and two more assertions now pin the
    live 206 case and the labelling rule.

Gates: air format . clean; devtools::document() committed; lintr with the
CI config reports 0 lints; every man/*.Rd is in _pkgdown.yml;
devtools::test() 5,781 pass / 0 fail / 8 skip (run on the rebased base).

Assertions that fail without the R change (verified by stashing R/ and
re-running): 13 — 5 in test_read_raw_inputs.R, 8 in test_polity_folds.R.

Moves published values

Yes, and only for bucket 206. The move is a correction in both directions and
each direction is checked against the raw FAOSTAT pin, not against a prior WHEP
build. NEWS.md carries the magnitudes. Nothing else in the pipeline changes by
more than 3.91 t.

One existing test changed its expectation:
.aggregate_to_polities warns when it folds Sudan into bucket 206 asserted
area_code == c(206, 206) — two rows for one bucket. It now asserts one row of
3,405,356, because that is what the warning next to it has always claimed was
happening.

What I deliberately did not do

The open decision

Bucket 206 is labelled SUD-1956-2011 "Sudan (1956-2011)" — a polity that had
ended by the years in question — because that is the polity its own code
resolves to, and because the pre-2012 half of the same series was already
labelled that way. The alternative is to keep labelling the post-2011 rows
"Sudan" (area 276's polity), which is what the split accidentally did for
roughly two thirds of them. I took the bucket's own label because it is the
rule the rest of the package already applies, and because a join key that flips
mid-series is the defect this PR exists to remove — but the choice of which
polity a two-territory bucket should name is #414's, not mine.

Closes #563
Part of the polity migration epic #458.

🤖 Generated with Claude Code

`.aggregate_to_polities()` grouped by the member's `polity_name` as well as
`polity_area_code`, so a bucket folding members that resolve to different
polities was never summed: it came back as several rows under one `area_code`
carrying different `area` labels. That is live on the shipped crosswalk --
bucket 206 folds FAOSTAT 276 Sudan and 277 South Sudan, which resolve to two
polities from 2012 -- and it is the reason #480 had to be reverted (#561).

The label is now derived after the sum, from the bucket's own code, which is
what `polity_bucket_coverage()` reports and what the reporting columns
resolve. `.read_crop_residues()` duplicated the aggregation inline and now
shares the same two helpers, so the two sites cannot emit two `area`
vocabularies for one `area_code` -- the shape that dropped 702,166 rows in
whep#382.

Measured over the real pins, the four split sources lose exactly their
duplicate-key count in rows and keep their totals to the digit.

Closes #563

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

lbm364dl commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Verified independently on main and on this branch, and merging. This is the most consequential fix in the epic so far: it is live data corruption in the published production series, not a latent blocker.

I filed #563 as "the prerequisite for redoing #480". That framing was wrong, and your reproduction is why. Bucket 206 folds FAOSTAT 206/276/277, those resolve to three polities from 2012, so the split has been live on shipped data the whole time.

Reproduced on main, to the digit

main this PR
goats 2018 (item 976) 14,449,249 55,295,249 South Sudan alone → Sudan + South Sudan
sugar cane 2019 (item 156), tonnes 10,898,000 5,449,000 exactly 2× → the reported value
sugar cane 2019, ha 142,200 71,100 the area was doubled too

Both of your headline numbers land exactly. The other bucket-206 livestock items move the same way — item 1053 goes 23.4 M → 62.9 M, 1016 goes 17.0 M → 48.8 M — so Sudan's herd was being dropped across the board.

Data was being silently dropped in one direction and doubled in the other, in the same bucket, depending on which member reported.

On my guard being too narrow

The invariant test I added in #566 passes on main — and it should not have. It uses areas 212 and 999, which resolve to the same polity there, so it only ever caught the #480 shape and never the live bucket-206 case. Your reader-level measurement (42 split area-years, 7,939 duplicate keys across eight readers, → 0) is the check that actually covers it. Worth folding that breadth into the guard so it cannot narrow again.

I also confirmed the published frame carries 0 duplicate keys on main (6,170,595 rows), which is consistent with your account rather than contradicting it: the split never survives to the output, it is resolved by dropping or doubling upstream, which is exactly why no downstream check saw it.

Full suite 5781 pass, 0 fail.

What I am not deciding

Your three open questions stay open and I have not pre-empted any of them: bucket 206's label (SUD-1956-2011 vs area 276's polity) is #414's call; whether to re-land the un-fold is #459's, now unblocked because the grouping no longer splits a bucket; and options(whep.unfold_rest_of_world = TRUE) stays off.

Also filing your #590 finding as worth acting on — two tests mocking .polity_bridge() believing it controls .aggregate_to_polities(), when it resolves through .polity_crosswalk() instead, is a mock that has silently never done anything.

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.

The polity-level ROW un-fold splits one bucket into several area labels, so .aggregate_to_polities() stops summing it

1 participant