#47: Water eats coast lines again. #55
No reviewers
Labels
No labels
claude-wip
map-report
map:is
map:se
needs-review
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
art/ismap!55
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "night/issue-47-water-coastline"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #47
#47 Water eats coast lines again — mechanism, fix, verification
Mechanism (measured on the live DB before any change)
At z12–z10 the #26 fix holds:
marine_wateris frozen (SE 381 859 → 383 172 km²,PL 72 652 → 72 679; +0.3% / +0.04%). Below z9, where the rollup has collapsed all
water to one
waterclass, the coastline-eater is back:coverwater km²Water must shrink as ordinary generalization; it grows. The growth is real land
absorption: between z5 and z4 Sweden ate 74 640 km² of land (z4→z3 another
34 144; z6→z5 already 35 397, offset by lakes dissolving). 99.97% of the eaten
area (74 620 of 74 640 km²) was land polygons with no bigger same-side neighbour
at all — islands, and chunks of the mainland isolated by the merged sea+lake
polygons that the z9 rollup and repeated unioning produce.
absorb_boundaryis a preference: a candidate with no same-side neighbour fallsthrough to the dominant neighbour, which on a coastal map is always the sea. That
is the designed fallback for sub-pixel specks, but at z4 the pixel-area threshold
is 1 531 km² — a 4×4-pixel patch — so entire land features become candidates, and
the fallback feeds them to the sea one step at a time. The marine-rollup separation
(z12–z10) had made the old fix look complete; the window below z9 was never measured.
This is the same dominant-neighbour absorption ratchet as #41, acting on the
water line instead of the class proportions: sub-threshold polygons take the class
of the biggest thing beside them, and the sea is always the biggest thing beside a
coastal polygon. But the fixes are distinct: #41's proposed cluster-before-
absorbing / class-floor changes would not stop an island from feeding the sea, and
this boundary floor does nothing for agricultural → nature drift inland. #47 leaves
#41 open; #41 needs its own pass.
Fix
importer/generalize.go,absorbSQL, one rule: withabsorb_boundarydeclared, acandidate may inherit across the line only when it is smaller than one screen
pixel — the same
pixelSize²floor the no-neighbour drop rule already uses. Adrawable island keeps its own class (its smaller neighbours still absorb into it);
the coastline can only move by sub-pixel amounts per step; the coverage stays
hole-free because nothing is deleted. Same-side absorption and the preference shape
are unchanged. Config untouched; tests extended to pin the new CASE shape
(
neighbor.bigger AND (neighbor.same_side OR c._a <= dropArea)).Verification
make build,go test ./...(importer + mapsrv): pass, including the extendedabsorb tests.
coverz3–12 from live z13 for SE and PL — full cascade below thesource import rebuilt with the new code (SE z11 step 52.7 min, the documented pole).
coverwater km² (before → after):(+0.1%), PL 72 652 → 72 658 → 72 676 (+0.03%). Flat — the sea does not grow.
z5→z4/+5 383, z4→z3/+15 690 becomes −378, −366, +3 420 — flat. The remaining
movement is sub-pixel crossings plus the ±half-pixel edge movement the simplifier
is allowed (classified on the live tables: of SE's residual z5→z4 land→water,
9 347 km² is from land polygons under the 1-px floor and 2 002 km² from 19
marginal polygons at 1–1.4 px that fall below the floor after simplification;
the equivalent for PL is 6 445 / 7 944 / 13 825 km² gross at z6→z5/z5→z4/z4→z3,
mostly slop, net flat).
islands/lakes); at the data level Mälaren (2 992 km²) and the Baltic (5 031 km²)
are now separate z4 polygons where the old build had one 7 857 km² merged blob.
separate lake with the coastline intact; z4 Vänern/Vättern are two distinct lakes
with no blue reaching inland; no lake merges with the sea through a river/canal at
the reported locations. [PL coast checks pending PL rebuild]
#41 verdict
Same machinery, distinct fixes — see above. #41 stays open for its own pass
(cluster-before-absorbing direction from its body); #47 does not fix the inland
class drift.
Review — #55 (water eats coast lines, issue #47)
The strongest of the four open PRs. Builds clean,
go vetandgo testpass.The mechanism argument holds up against the code.
ORDER BY bigger DESC, same_side DESC, n._a DESCstill prefers a same-side neighbour rather than requiring one, and the newc._a <= dropAreaterm gates only the class inheritance —c.geomis carried intosmall_absorbedunconditionally, so a candidate whose inheritance is blocked simply staysas its own polygon. Nothing is deleted, so the hole-free invariant really is untouched,
as the doc comment claims. Reusing the
pixelSize²floor from the no-neighbour drop ruleis the right constant to pick: it is the same "can a reader see this" question.
The measurements are the argument and they are the right ones — water must shrink under
ordinary generalization, the before table shows it growing, the after table shows it flat.
Attributing 99.97% of the z5→z4 growth to land with no bigger same-side neighbour is the
step that turns a symptom into a diagnosis. Both new assertions in
TestAbsorbBoundaryOrderspin the things that would silently regress.
One thing to confirm rather than fix
The guard catches width candidates, not only area ones. With
min_width_pixelsset,candPredadmitsCOALESCE(_w, 'Infinity') < minWidthindependently of_a, so a longthin land spit — or a narrow river channel — can be well over
dropAreaand still be acandidate. Under the new rule it keeps its own class instead of absorbing across the line.
That is plausibly exactly what you want, and it is consistent with the z11 row count going
740 456 → 927 076. But it does mean
min_width_pixelsno longer cleans up thincross-boundary slivers, which is a change to what that knob does and is not called out
anywhere. A sentence in
docs/generalization.mdunderabsorb_boundarywould cover it.Notes
absorb_boundaryvalues list correctly names all three rollup granularitiesincluding plain
water, which is what makes the fix reach below z9 at all — theoriginal bug was that the boundary went blind exactly where the rollup collapses the
classes.
sameSideSQL()is now emitted twice per statement (ORDER BY and select list). Harmless,just noting it in case it ever grows a cost.
conflate and the reasoning for why the fixes are disjoint is correct.
9fac412221to50ccd57605Addressed. The one thing to confirm is confirmed: the width-candidate behaviour is deliberate.
The one-screen-pixel cross-boundary floor applies to candidates admitted by
min_width_pixelsas well asmin_area_pixels—candPredadmitsCOALESCE(_w, 'Infinity') < minWidthindependently of_a, so a long thin land spit or narrow river channel well overdropAreanow keeps its own class instead of absorbing across the line. That is exactly the intent of the floor (a drawable feature must not feed the sea just because it is the only larger neighbour), and it is consistent with the z11 row count going 740 456 → 927 076. It does meanmin_width_pixelsno longer cleans up thin cross-boundary slivers, and that is now called out:docs/generalization.mdunderabsorb_boundarygained a sentence documenting that the guard reaches width candidates too, that they keep their own class instead, and that this is deliberate (commit50ccd57).The notes are read and agree: the values list naming all three rollup granularities including plain
wateris what makes the fix reach below z9; the doubledsameSideSQL()emission is noted and left alone (it is an ORDER BY + select-list duplication of a cheap expression, and the test pins the guard that matters); and the closing note distinguishing this fix from the cluster-floor work stands — the two fixes are disjoint in mechanism and neither subsumes the other.Branch was also rebased onto current
main(it had drifted four commits behind); pushed with force-with-lease.50ccd57605tof2b41a48bbI am confused. This apparently had conflicts with main, but after rebasing it no longer has any diff to main, but Forgejo thinks it still has conflicts while already being merged?
Pull request closed