From 69f3aa2d220582c2c02abca9c8e9d3c6a535b0ea Mon Sep 17 00:00:00 2001 From: Jonas Reith Date: Mon, 31 Aug 2026 19:41:25 +0200 Subject: [PATCH] Fix unbounded sea-level sinking on long runs Reported after a real long-running world (elapsedMy ~1340) showed seaLevel drifted to -2900m, with "land" at -2000m next to "ocean" at -4000m. Root cause: adjustSeaLevel()'s geographic land target (elevation above seaLevel, fixed 30%) and advect()'s crust-type land conservation (targetLand/landBand, held near whatever Phase-1 forming settled at for that seed) are two independent notions of "land" that aren't guaranteed to agree -- this seed's continental crust settled at only ~26%. With genuine land structurally short of the target, the controller had no lower bound and kept sinking seaLevel to misclassify progressively older, deeper oceanic crust as land -- and since that crust keeps ageing and deepening even at a fixed seaLevel, it was chasing a moving target with no way to ever settle. Added seaLevelMin/seaLevelMax config fields (default +-3000m) that clamp adjustSeaLevel()'s candidate nudge, so a world that can't reach the target land fraction settles at a plausible offset instead of an unbounded one. New test_sealevel.cpp reproduces the pathology with a continuous elevation distribution (not a synthetic cliff) and confirms the controller still engages but never crosses either bound. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TMfyZv91tonDnPJqbaTVJE --- CLAUDE.md | 36 ++++++++++++- CMakeLists.txt | 2 +- src/sim/PlanetErosion.cpp | 9 +++- src/sim/PlanetIO.cpp | 7 ++- src/sim/PlanetTypes.hpp | 11 ++++ test_sealevel.cpp | 103 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 test_sealevel.cpp diff --git a/CLAUDE.md b/CLAUDE.md index 9ee5d67..5ba0a6f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -552,6 +552,38 @@ on the Live World clock). **Steps 1–7 of the roadmap are done (plus a derived `src/sim` (`Planet.hpp`, `PlanetIO.cpp`) + `src/render` (`Viewer.{hpp,cpp}`, `ViewerInput.cpp`, `ViewerRender.cpp`, `Toolbar.cpp`, new `MainMenu.{hpp,cpp}`); `.gitignore`'s `planet.save` line became `*.save`. +- **Bugfix: sea level could sink without limit on a long run (`seaLevel` at -2900 m after ~1340 My, + "land" cells sitting at -2000 m next to "ocean" at -4000 m)** *(done — see `Planet::adjustSeaLevel()`, + `src/sim/PlanetErosion.cpp`)* — reported after a real long-running world. Root cause: two + independent notions of "land" (documented since Phase 2 inc. 2, see the Architecture section) drift + apart on a long run. `adjustSeaLevel()`'s **geographic** target (`landFractionTarget`, elevation + above `seaLevel`) is a fixed 30%, but the **crust-type** land conservation in `advect()` + (`targetLand`/`landBand`, Phase 2 inc. 1.5) only holds continental crust area near whatever + Phase-1 forming happened to settle at for that seed — nothing guarantees the two agree (this + world's crust settled at ~26%). With genuine continental land structurally short of the 30% + target, `adjustSeaLevel()` had **no lower bound**: every check found the error still outside the + deadband and kept nudging `seaLevel` down by `seaLevelStep`, misclassifying progressively older, + deeper **oceanic** crust as "geographic land" to make up the shortfall. Because oceanic crust keeps + *aging and deepening* (`oceanicBase(age)`, seafloor subsidence) even at a fixed `seaLevel`, cells + counted as land kept sinking out of range under the controller — a moving target with no floor, so + it never reached equilibrium and just kept sinking, confirmed on the user's save (`cfg.seaLevel` + -2900 m, continental crust only 26.2% of cells vs. 30% target, 9.4% of "geographic land" was + actually aging oceanic crust). Fixed with two new hard bounds, **`seaLevelMin`** (-3000 m) / + **`seaLevelMax`** (+3000 m): `adjustSeaLevel()` clamps its candidate nudge to this range, so once + pinned at a bound the clamped candidate equals the current value, the error doesn't improve, and + the existing "only nudge if it helps" check stops it there — a world that structurally can't reach + the target settles at a plausible coastal offset with an imperfect land fraction instead of an + unbounded, physically nonsensical one. New headless `test_sealevel.cpp`: two synthetic worlds + (crust short of / in excess of the target, continuous depth distributions so every step genuinely + helps, reproducing the real pathology rather than getting stuck on a synthetic cliff) confirm the + controller still engages and moves, but never crosses either bound; a third confirms a normal world + never approaches the bounds. `src/sim` only (`PlanetTypes.hpp`, `PlanetErosion.cpp`, `PlanetIO.cpp` + for the two new config fields + range checks); config is the self-describing text block (v6+), so + **no save-version bump**. Note for already-affected saves: the fix stops further sinking but + doesn't retroactively restore a sunk `seaLevel` on load (config-field edits, whether via + `planet.cfg`+`F2` or the Main Menu's Settings tab, only take effect through a regenerate — there is + currently no way to push a single global field like `seaLevel` into an already-running world + without starting over); the practical fix for an affected world is a New World. ## Current state @@ -1460,7 +1492,9 @@ triangles (plates are fixed in phase 1). goal; `seaLevelStep` (100 m) — fixed nudge per adjustment + `seaLevelTol` (0.02) — deadband where sea level rests (raise step or shrink tol for a tighter 30/70, but a big step can overshoot a flat "cliff"); `seaLevelEvery` (100) — iterations - between sea-level updates (higher = more gradual). All editable in `planet.cfg`. + between sea-level updates (higher = more gradual); `seaLevelMin` (-3000 m) / + `seaLevelMax` (+3000 m) — hard bounds on how far the controller may push + `seaLevel` (see the runaway-guard bugfix entry below). All editable in `planet.cfg`. - **Orogeny — taller mountains (PlanetConfig, Phase 2 inc. 4):** `collisionFactor` (1.8, continent-continent uplift, Himalaya) and `arcFactor` (1.4, continental subduction-arc uplift, Andes) — raise either to make ranges taller/more reliable diff --git a/CMakeLists.txt b/CMakeLists.txt index fac4276..1bbfa35 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -94,7 +94,7 @@ if(UNIX AND NOT APPLE) endif() enable_testing() -foreach(test_name logic biota ocean live weather volcano geography ecoregions civ nation culture conflict diplomacy trade colony cultevo edit) +foreach(test_name logic biota ocean live weather volcano geography ecoregions civ nation culture conflict diplomacy trade colony cultevo edit sealevel) add_executable(test_${test_name} test_${test_name}.cpp) target_link_libraries(test_${test_name} PRIVATE planetsim_sim) add_test(NAME ${test_name} COMMAND test_${test_name}) diff --git a/src/sim/PlanetErosion.cpp b/src/sim/PlanetErosion.cpp index a7d26be..8187bbf 100644 --- a/src/sim/PlanetErosion.cpp +++ b/src/sim/PlanetErosion.cpp @@ -47,7 +47,13 @@ void Planet::erode(double dtMy) { // coastline drifts slowly, not in a jump. A nudge is only taken if it actually // reduces the error -- otherwise (e.g. a big mass of cells at one elevation, where // a full step would overshoot) it rests at the closest a fixed step allows instead -// of oscillating back and forth across that "cliff". +// of oscillating back and forth across that "cliff". The candidate is clamped to +// [seaLevelMin, seaLevelMax] -- without a bound, a world whose buoyant crust area +// permanently falls short of landFractionTarget has no seaLevel that reaches the +// target, and the controller sinks forever chasing ageing (deepening) oceanic +// crust it keeps re-misclassifying as land (see the seaLevelMin/Max comment). +// Once seaLevel is pinned at a bound, the clamped candidate equals the current +// value, candErr == err, and the "only nudge if it helps" check below stops it. void Planet::adjustSeaLevel() { const int n = (int)cells.size(); if (n == 0) return; @@ -57,6 +63,7 @@ void Planet::adjustSeaLevel() { double err = landFracAt(cfg.seaLevel) - cfg.landFractionTarget; if (std::fabs(err) <= cfg.seaLevelTol) return; // within deadband: rest double cand = cfg.seaLevel + (err > 0 ? cfg.seaLevelStep : -cfg.seaLevelStep); + cand = std::clamp(cand, cfg.seaLevelMin, cfg.seaLevelMax); double candErr = landFracAt(cand) - cfg.landFractionTarget; if (std::fabs(candErr) < std::fabs(err)) cfg.seaLevel = cand; // nudge only if it helps } diff --git a/src/sim/PlanetIO.cpp b/src/sim/PlanetIO.cpp index 70e4939..0aed2d5 100644 --- a/src/sim/PlanetIO.cpp +++ b/src/sim/PlanetIO.cpp @@ -24,7 +24,7 @@ D(maxDriftSpeed) D(ridgeDepth) D(splitFraction) D(splitProbBase) D(splitProbSlope) \ D(stalemateEps) D(stalemateBoost) D(babyPromoteFrac) D(volcanicLandFrac) \ D(volcanicElev) D(landBand) D(erosionLandRate) D(erosionSeaRate) \ - D(landFractionTarget) D(seaLevelStep) D(seaLevelTol) \ + D(landFractionTarget) D(seaLevelStep) D(seaLevelTol) D(seaLevelMin) D(seaLevelMax) \ D(phase3AfterMy) D(phase3DtScale) D(rainfall) D(riverThreshold) D(riverIncision) \ D(riverDischargeExp) D(riverSlopeExp) D(riverTransport) D(depFrac) \ D(biomeEquatorTemp) D(biomePoleDrop) D(biomeLatExp) D(biomeElevLapse) \ @@ -156,6 +156,7 @@ static const struct { const char* name; const char* category; } kConfigFieldCate {"erosionLandRate", "Erosion & sea level"}, {"erosionSeaRate", "Erosion & sea level"}, {"landFractionTarget", "Erosion & sea level"}, {"seaLevelStep", "Erosion & sea level"}, {"seaLevelTol", "Erosion & sea level"}, {"seaLevelEvery", "Erosion & sea level"}, + {"seaLevelMin", "Erosion & sea level"}, {"seaLevelMax", "Erosion & sea level"}, {"phase3AfterMy", "Hydrology"}, {"phase3DtScale", "Hydrology"}, {"rainfall", "Hydrology"}, {"riverThreshold", "Hydrology"}, {"riverIncision", "Hydrology"}, @@ -379,6 +380,8 @@ std::string validateConfig(const PlanetConfig& cfg) { E(rng(cfg.landFractionTarget, 0.01, 0.99, "landFractionTarget")); E(rng(cfg.seaLevelStep, 1.0, 2000.0, "seaLevelStep")); E(rng(cfg.seaLevelTol, 0.001, 0.5, "seaLevelTol")); + E(rng(cfg.seaLevelMin, -11000.0, 0.0, "seaLevelMin")); + E(rng(cfg.seaLevelMax, 0.0, 11000.0, "seaLevelMax")); E(rng(cfg.phase3AfterMy, 0.0, 1.0e6, "phase3AfterMy")); E(rng(cfg.phase3DtScale, 0.001, 1.0, "phase3DtScale")); E(rng(cfg.rainfall, 0.0, 1.0e6, "rainfall")); @@ -598,6 +601,8 @@ std::string validateConfig(const PlanetConfig& cfg) { } if (cfg.oceanBase >= cfg.continentBase) bad.push_back("oceanBase >= continentBase (ocean floor must be below continents)"); + if (cfg.seaLevelMin >= cfg.seaLevelMax) + bad.push_back("seaLevelMin >= seaLevelMax (the sea-level controller needs a real band)"); if (cfg.peakSoftCapStart >= cfg.peakSoftCapEnd) bad.push_back("peakSoftCapStart >= peakSoftCapEnd (grow probability must span a band)"); if (cfg.volcanoDormantMinYears > cfg.volcanoDormantMaxYears) diff --git a/src/sim/PlanetTypes.hpp b/src/sim/PlanetTypes.hpp index 11a5e15..e983613 100644 --- a/src/sim/PlanetTypes.hpp +++ b/src/sim/PlanetTypes.hpp @@ -270,6 +270,17 @@ struct PlanetConfig { double seaLevelStep = 100.0; // m, fixed nudge per adjustment when off target double seaLevelTol = 0.02; // deadband (land-fraction) where sea level rests int seaLevelEvery = 100; // erode calls between sea-level adjustments + // Hard bounds on the controller: if a world's buoyant (continental) crust area + // permanently sits below landFractionTarget (crust generation is independent of + // this geographic target -- no guarantee they match), there is no seaLevel low + // enough to reach the target using real land, and ageing oceanic crust keeps + // deepening out of reach -- so an unbounded controller sinks seaLevel forever, + // eventually misclassifying deep aging seafloor as "geographic land". These + // clamp the candidate each adjustSeaLevel() step so it settles at a plausible + // coastal offset instead of running away (a slightly-off land fraction beats a + // physically nonsensical sea level). + double seaLevelMin = -3000.0; // m, lowest the controller may sink seaLevel + double seaLevelMax = 3000.0; // m, highest the controller may raise seaLevel // --- Spreading & volcanic (Phase 2 plate dynamics, kept here for binary compat) int babyMinCells = 4; // baby blobs smaller than this dissolve (noise) diff --git a/test_sealevel.cpp b/test_sealevel.cpp new file mode 100644 index 0000000..281889d --- /dev/null +++ b/test_sealevel.cpp @@ -0,0 +1,103 @@ +// Headless test for the sea-level controller's runaway guard (no display needed). +// +// g++ -std=c++17 -O2 -Isrc/sim test_sealevel.cpp src/sim/IcoSphere.cpp +// src/sim/Planet.cpp src/sim/PlanetTectonics.cpp src/sim/PlanetDrift.cpp +// src/sim/PlanetErosion.cpp src/sim/PlanetHydrology.cpp +// src/sim/PlanetBiomes.cpp src/sim/PlanetClimate.cpp src/sim/PlanetLive.cpp +// src/sim/PlanetOcean.cpp src/sim/PlanetBiota.cpp src/sim/PlanetFloraGen.cpp +// src/sim/PlanetFaunaGen.cpp src/sim/PlanetFungiGen.cpp src/sim/PlanetIO.cpp +// -o /tmp/ts && /tmp/ts +// +// Reproduces the bug reported after a long real run: on a world whose buoyant +// (continental) crust area permanently falls short of landFractionTarget (crust +// generation is independent of that geographic target -- nothing guarantees they +// match), the old unbounded adjustSeaLevel() had no way to reach the target using +// real land, so it kept sinking seaLevel to "poach" ageing oceanic crust as fake +// land -- and since that crust keeps deepening with age, the target kept slipping +// away, sinking seaLevel indefinitely (observed: -2900 m after ~1340 My, still +// falling). Verifies the seaLevelMin/seaLevelMax clamp stops the runaway in both +// directions, that it actually engages (proving the mechanism still adjusts, not +// just trivially idle), and that a normal, non-pathological world never comes +// close to the clamp. + +#include "Planet.hpp" +#include +#include + +static int failures = 0; +static void check(bool cond, const char* what) { + std::printf(" [%s] %s\n", cond ? "PASS" : "FAIL", what); + if (!cond) ++failures; +} + +int main() { + std::printf("Sea level: runaway guard\n"); + + // --- Scenario 1: continental crust structurally short of the target ------- + // 25% of cells continental near continentBase, 75% oceanic spread smoothly + // across a wide, continuous depth range (mimicking a real mix of young and + // long-ageing seafloor, not a cliff) -- deliberately short of the 30% default + // landFractionTarget. With a continuous distribution, each single seaLevelStep + // nudge always captures more cells and strictly reduces the error, so the + // pre-fix controller would keep taking that nudge forever, sinking without end. + { + PlanetConfig cfg; cfg.subdivisions = 3; cfg.seed = 99; + Planet planet; planet.generate(cfg); + const int n = (int)planet.cells.size(); + for (int i = 0; i < n; ++i) { + bool continental = (i % 4) == 0; // 25% < landFractionTarget (30%) + planet.setCrust(i, continental); + if (continental) planet.setElevation(i, planet.cfg.continentBase + (i % 7) * 10.0); + else planet.setElevation(i, -10.0 - 8000.0 * (double)i / n); // continuous -10..-8010 m + } + double startSeaLevel = planet.cfg.seaLevel; + // erode()'s transport is dtMy-scaled; dt=0 isolates the periodic + // adjustSeaLevel() call (still triggered by the iteration counter) from + // any elevation change, so the crafted distribution above stays intact. + for (int i = 0; i < planet.cfg.seaLevelEvery * 400; ++i) planet.erode(0.0); + + check(planet.cfg.seaLevel < startSeaLevel - 100.0, + "short-of-target land engages the controller (seaLevel moves down)"); + check(planet.cfg.seaLevel >= planet.cfg.seaLevelMin, + "seaLevel never sinks past seaLevelMin (the pre-fix bug: it had no floor)"); + check(planet.cfg.seaLevel <= planet.cfg.seaLevelMax, "seaLevel stays within the upper bound too"); + } + + // --- Scenario 2: mirror case, land far in excess of the target ------------ + { + PlanetConfig cfg; cfg.subdivisions = 3; cfg.seed = 100; + Planet planet; planet.generate(cfg); + const int n = (int)planet.cells.size(); + for (int i = 0; i < n; ++i) { + bool continental = (i % 4) != 0; // 75% >> landFractionTarget (30%) + planet.setCrust(i, continental); + if (continental) planet.setElevation(i, 10.0 + 8000.0 * (double)i / n); // continuous 10..8010 m + else planet.setElevation(i, planet.cfg.oceanBase + (i % 7) * 10.0); + } + double startSeaLevel = planet.cfg.seaLevel; + for (int i = 0; i < planet.cfg.seaLevelEvery * 400; ++i) planet.erode(0.0); + + check(planet.cfg.seaLevel > startSeaLevel + 100.0, + "excess land engages the controller (seaLevel moves up)"); + check(planet.cfg.seaLevel <= planet.cfg.seaLevelMax, + "seaLevel never climbs past seaLevelMax (mirror of the sinking bug)"); + check(planet.cfg.seaLevel >= planet.cfg.seaLevelMin, "seaLevel stays within the lower bound too"); + } + + // --- Scenario 3: a normal, freshly-generated world never approaches the --- + // clamp -- the guard is a safety rail for the pathological case, not + // something that interferes with ordinary sea-level settling. + { + PlanetConfig cfg; cfg.subdivisions = 4; cfg.seed = 42; + Planet planet; planet.generate(cfg); + for (int i = 0; i < 40; ++i) planet.step(); // settle Phase-1 relief first + planet.drifting = true; + for (int i = 0; i < planet.cfg.seaLevelEvery * 20; ++i) planet.erode(planet.cflDtMy()); + + check(planet.cfg.seaLevel > planet.cfg.seaLevelMin * 0.5 && planet.cfg.seaLevel < planet.cfg.seaLevelMax * 0.5, + "a normal world's sea level settles well clear of the clamp bounds"); + } + + std::printf(failures ? "\nFAILURES: %d\n" : "\nALL SEA LEVEL CHECKS PASSED\n", failures); + return failures ? 1 : 0; +}