feat: basin-aware region clustering - #32
Merged
koen-vg merged 6 commits intoJul 22, 2026
Merged
Conversation
koen-vg
force-pushed
the
basin-aware-regions
branch
from
July 21, 2026 01:06
208a45b to
3c3e8d4
Compare
This was referenced Jul 21, 2026
koen-vg
force-pushed
the
basin-aware-regions
branch
2 times, most recently
from
July 21, 2026 02:08
730d02e to
3eddd05
Compare
Split GADM provinces along AWARE hydrological basin boundaries before clustering, so a province straddling an abundant and a scarce basin is no longer pooled into one region -- which averaged away exactly the sub-provincial scarcity that constrains irrigation. Each country is then partitioned into exactly target_count regions balancing geography against basin scarcity (aggregation.regions.basin_scarcity_weight; 0 recovers plain geographic clustering). Every region remains either contained in one province (large provinces split into sub-regions) or a union of whole provinces (small provinces merged), never a mix of partial pieces across provinces, so regions stay comparable to political units. allow_cross_border: true has no basin-aware path and now raises rather than silently ignoring basins. Two corrections relative to the water-aware original: - The per-country region budget summed the geodesic area of one arbitrary basin fragment per province, so countries whose provinces are cut into many basins were systematically starved of regions (Russia -27, China -12, Indonesia -10; DRC +12, Nigeria +11). Area is now summed over all pieces, and the capacity cap is the piece count, since a province can now yield more than one region. - basin_scarcity_weight is 2.0 rather than 3.0. Swept on the full 750-region set, the area-weighted within-region scarcity spread falls 10.34 (weight 0) -> 8.27 -> 7.60 -> 7.40 -> 7.18 (weight 5) while the largest region grows 92 -> 91 -> 91 -> 110 -> 133 Mha: separation saturates by 2, beyond which only the size tail degrades. Also guards the overlay: a configured country with no basin overlap is an error rather than a silent disappearance, and the dropped sliver area (0.05% of land on the default country set) is logged. This changes default region geometry for every config.
Dissolving province-basin pieces leaves a handful of self-intersecting polygons -- 3 of 750 regions at the default resolution, 1 of 200 in the test config. That is not a cosmetic problem: exactextract calls into native code that SEGFAULTS on invalid geometry rather than raising, so a single bad region kills an unrelated downstream rule with an empty log and no traceback. It surfaced as a crash in the Huang water aggregation during a calibration run, several rules removed from its cause. Repairing at the source rather than in each consumer, since every rule that aggregates a raster over regions is exposed. Regions that cannot be repaired now raise, so a real geometry problem still fails loudly.
Basin-aware clustering has no cross-border path, so the option's only valid value was false; drop the key from config, schema, rule and script instead of rejecting it at runtime.
set_precision's valid_output mode already guarantees valid snapped geometry, so the trailing buffer(0) did nothing; conversely a region collapsed to POLYGON EMPTY passes is_valid, so check emptiness explicitly both in memory and on the written file.
Area-weighted province centroids were computed twice in different idioms; factor them into _province_centroids. Pass piece/province areas as k-means sample weights so a coastal sliver does not pull a centroid as hard as a half-province piece.
The pieces-scarce fixture resolved entirely in the merge regime, so _reconcile_to_k was never hit; give the indivisible province a quota of two so reconciliation must recover the deficit from the merge group.
koen-vg
force-pushed
the
basin-aware-regions
branch
from
July 22, 2026 18:03
91e4458 to
d4d921b
Compare
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.
Model regions are now built basin-aware. GADM provinces are split along AWARE hydrological basin boundaries before clustering, so a province straddling an abundant and a scarce basin is no longer pooled into one region — which averaged away exactly the sub-provincial scarcity that constrains irrigation. Each country is then partitioned into exactly
target_countregions balancing geography against basin scarcity.The nesting invariant is preserved: every region is either contained in one province (large provinces split into sub-regions) or a union of whole provinces (small provinces merged), never a mix of partial pieces across provinces — so regions stay comparable to political units. The
allow_cross_borderconfig option is removed entirely: basin-aware clustering has no cross-border path, so a key whose only valid value isfalsewas dead weight.Two corrections relative to the water-aware original
The per-country region budget was wrong. It summed the geodesic area of one arbitrary basin fragment per province, so countries whose provinces are cut into many basins were systematically starved of regions: Russia lost 27, China 12, Indonesia 10, while the DRC gained 12 and Nigeria 11. Area is now summed over all pieces, and the capacity cap is the piece count rather than the province count, since a province can now legitimately yield more than one region. A regression test covers this and fails against the original code.
basin_scarcity_weightis 2.0, not 3.0. The original comment claimed it was "tuned empirically against region-size evenness", which the data did not support. Swept on the full 750-region default set:Scarcity separation saturates by weight 2 (against a floor of ~7.2), beyond which only the size tail degrades. The config comment now states this trade-off honestly.
Also
The basin overlay is guarded: a configured country with no basin overlap is an error rather than a silent disappearance, and the dropped sliver area — 0.05% of land on the default country set, coastal fragments and small islands outside every basin — is logged.
Only the AWARE basin geopackage (5.3 MB) is fetched here. The two AWARE workbooks that the water supply restructure needs are left for that PR.
Impact
This changes default region geometry for every config, so Snakemake rebuilds the whole
processing/{name}/tree, and regional doc figures will need regenerating.Calibration
test_calibration_provenance.py::test_default_stamp_matches_default_configfails on this branch by design:basin_scarcity_weightis a new structural key absent from the tracked stamps, and region geometry genuinely changes, so grassland yields, feed conversion, costs and deviation penalties are all fit against a different spatial partition. Per the repo owner's decision, the structural series (multi-cropping → this → water restructure) ends with a singletools/calibraterun for both tracked sets rather than three thrown-away ones. That is why this must not merge on its own.Review follow-ups
Post-review polish: dropped a redundant
buffer(0)afterset_precision(whosevalid_outputmode already guarantees validity) and flagged empty geometries alongside invalid ones in both validity checks; removed theallow_cross_borderoption; factored the duplicated area-weighted province-centroid computation into a helper; k-means now weights samples by piece area so slivers don't pull centroids; the pieces-scarce test now actually exercises the reconciliation split path.Verification
Build and integration tests (dryrun + build/solve/analyze) pass with the new geometry; 8 unit tests cover apportionment, the exact region count, the nesting invariant, scarcity-driven splitting, reconciliation, cross-border rejection, and the budget regression.