fix(simulation): address PR #158 review (carrier foundation)
Hoshe + Tyre review (CHANGES REQUESTED): - domain_warp golden tests were hollow (discarded values, only asserted dx!=dy). Consolidate into one real golden_vector test with pinned f64 literals as the regression anchor. (Hoshe #1, Tyre #2) - Per-region temperature used body-level elevation_km for every region, so alpine and sea-level regions on a body got identical lapse — defeats the D-239 §2 per-district temperature. Derive elevation_km per region from the region's own elev_q (× MAX_REGION_ELEVATION_KM). (Tyre #1) - Region seed was derived under SeedDomain::DomainWarp (collision risk with the tile warp) and discarded unused. Remove it + the orphaned region_pos_id and SeedDomain import; keep a reserved _seed param for T-1027/T-1028. (Hoshe #2) - Fix inverted tilt_factor comment. (Hoshe #3) - ClimateConstants doc referenced a load() that doesn't exist; correct it — embedded default() is authoritative today, climate_constants.toml is the canonical mirror, runtime load lands with T-1032. (Hoshe + Tyre) - cascade.rs: drainage re-run was mislabeled 'cheap'/'no drainage needed'; document the real cost + PERF/TODO(T-1028/T-1032) against the D-239 §10 budget. (Tyre #3) cargo test 1497 pass, clippy -D warnings clean, fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.8
@@ -211,16 +211,18 @@ pub fn run_cascade_from_heightmap(
|
||||
}
|
||||
|
||||
// RegionProfile layer (T-1023, D-239 §1) — pure derivation from body params +
|
||||
// terrain analysis. Requires Layer 1 TerrainAnalysis, so we re-derive it here
|
||||
// (TerrainAnalysis is cheap relative to drainage; Layer 1 already ran if we
|
||||
// reached Settlement, but the analysis isn't stored on Layer1Output).
|
||||
// terrain analysis. Needs a TerrainAnalysis, which needs a drainage pass.
|
||||
// Layer 1 already ran drainage inside run_layer1, but neither the drainage
|
||||
// result nor the TerrainAnalysis is stored on Layer1Output, so we re-run both
|
||||
// here. Pure → determinism preserved, but the drainage re-run is NOT free at
|
||||
// the ~6 000-regions/body working scale (D-203).
|
||||
// PERF/TODO(T-1028/T-1032): cache TerrainAnalysis on Layer1Output to drop this
|
||||
// redundant drainage pass, and validate the combined cost against the D-239 §10
|
||||
// ~45 ms/body budget in the T-1031 verification harness. Latent for now — this
|
||||
// path only runs when body_params is Some, which production defers to T-1032.
|
||||
// If body_params is None, the region layer is skipped (e.g. unit tests without DB).
|
||||
if up_to >= CascadeLayer::RegionProfile {
|
||||
if let Some(params) = body_params {
|
||||
// Re-derive TerrainAnalysis from the heightmap (no drainage needed — we
|
||||
// only need slope/elev/ocean which are computed inside TerrainAnalysis).
|
||||
// The drainage result isn't stored on the snapshot, so rerun it here.
|
||||
// This is a pure function so determinism is preserved.
|
||||
use crate::atlas::drainage;
|
||||
let dr = drainage::analyze(
|
||||
&snapshot.heightmap.data,
|
||||
|
||||
@@ -112,54 +112,30 @@ mod tests {
|
||||
use crate::seed::fnv1a_64;
|
||||
use std::thread;
|
||||
|
||||
/// Golden-vector test: fixed (seed, body_id, pos) → fixed (dx, dy).
|
||||
/// Mirrors the `splitmix64_known_vector` pattern in seed.rs.
|
||||
/// Single-platform; the ULP argument (D-239 §4) makes cross-platform
|
||||
/// golden values unnecessary.
|
||||
/// Golden-vector regression anchor: a fixed (seed, body_id, pos) maps to
|
||||
/// fixed (dx, dy). Mirrors `splitmix64_known_vector` in seed.rs. Single-
|
||||
/// platform pins suffice — the ±8 m bound + downstream `as i32` truncation
|
||||
/// (D-239 §4) make cross-platform ULP drift irrelevant to the voxel address.
|
||||
///
|
||||
/// Recompute and re-pin these literals ONLY after a deliberate algorithm
|
||||
/// change (`pos_to_id`, `u64_to_displacement`, or the SeedDomain chain) —
|
||||
/// such a change re-rolls the warp for every body and needs a D-record
|
||||
/// amendment. If this test fails unexpectedly, the warp output drifted.
|
||||
#[test]
|
||||
fn golden_vector() {
|
||||
let (dx, dy) = domain_warp(42, "GJ1c", (100, -50));
|
||||
// The values are fixed by the deterministic hash chain. Print them once,
|
||||
// then pin them as the golden vector. Run once without the assert to
|
||||
// discover the values, then lock them in.
|
||||
// Computed reference (do NOT change unless the algorithm changes):
|
||||
assert!(
|
||||
dx.abs() <= WARP_BOUND,
|
||||
"dx={dx} out of bounds ±{WARP_BOUND}"
|
||||
// Pinned literals — the canonical regression anchor for the warp output.
|
||||
assert_eq!(
|
||||
dx, -1.8472385880244921,
|
||||
"domain_warp dx drifted — algorithm changed?"
|
||||
);
|
||||
assert!(
|
||||
dy.abs() <= WARP_BOUND,
|
||||
"dy={dy} out of bounds ±{WARP_BOUND}"
|
||||
assert_eq!(
|
||||
dy, 5.033644613199796,
|
||||
"domain_warp dy drifted — algorithm changed?"
|
||||
);
|
||||
// Pin exact values for the golden-vector test (same platform).
|
||||
let (dx2, dy2) = domain_warp(42, "GJ1c", (100, -50));
|
||||
assert_eq!(dx, dx2, "dx must be deterministic");
|
||||
assert_eq!(dy, dy2, "dy must be deterministic");
|
||||
|
||||
// Store the actual values for the pinned assertion below.
|
||||
let expected_dx = domain_warp(42, "GJ1c", (100, -50)).0;
|
||||
let expected_dy = domain_warp(42, "GJ1c", (100, -50)).1;
|
||||
assert_eq!(dx, expected_dx);
|
||||
assert_eq!(dy, expected_dy);
|
||||
}
|
||||
|
||||
/// Pinned golden vector — the canonical reference for regression detection.
|
||||
///
|
||||
/// To discover the values: comment out this test, run with `-- --nocapture`,
|
||||
/// add a `println!("{dx} {dy}")` call, read the output, then pin here.
|
||||
/// Do NOT change these values unless the algorithm (pos_to_id, u64_to_displacement,
|
||||
/// the SeedDomain chain) deliberately changes — such a change re-rolls the
|
||||
/// warp for every body.
|
||||
#[test]
|
||||
fn pinned_golden_vector() {
|
||||
let (dx, dy) = domain_warp(42, "GJ1c", (100, -50));
|
||||
// These values were computed from the implementation; they are the
|
||||
// canonical regression anchor. If this test fails, the warp changed.
|
||||
// Recompute and re-anchor ONLY after a deliberate algorithm change with
|
||||
// a D-record amendment.
|
||||
let _ = (dx, dy); // values established by the deterministic chain above
|
||||
// Structural check: two channels are independent.
|
||||
assert_ne!(dx, dy, "dx and dy must differ (independent channels)");
|
||||
// Within the ±8 m bound, and the two channels are independent.
|
||||
assert!(dx.abs() <= WARP_BOUND && dy.abs() <= WARP_BOUND);
|
||||
assert_ne!(dx, dy);
|
||||
}
|
||||
|
||||
/// Truncation-as-cast (not rounding): the downstream consumer uses `as i32`.
|
||||
|
||||
@@ -24,7 +24,7 @@ use std::collections::BTreeMap;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::atlas::features::TerrainAnalysis;
|
||||
use crate::seed::{SeedChain, SeedDomain};
|
||||
use crate::seed::SeedChain;
|
||||
use crate::simulation::generator::MorphologyZone;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -321,11 +321,19 @@ fn derive_morphology_zone(
|
||||
// Climate constants (T-1024, D-239 §2)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// Climate constants loaded from `server/data/climate_constants.toml`.
|
||||
///
|
||||
/// Source-canonical — tunable without recompile. Use `ClimateConstants::load`
|
||||
/// to read from the TOML file, or `ClimateConstants::default()` for the
|
||||
/// embedded fallback values (matching the TOML's current content).
|
||||
/// Maximum per-region elevation (km), used to scale a region's normalized
|
||||
/// elevation (`elev_q`, 0–100) into the temperature-lapse input. Earth-like
|
||||
/// span (~Everest); a coarse body-agnostic constant for now — a per-body relief
|
||||
/// span can replace it when body relief data is carried (T-1032).
|
||||
const MAX_REGION_ELEVATION_KM: f64 = 8.0;
|
||||
|
||||
/// Climate tuning constants. `ClimateConstants::default()` holds the embedded
|
||||
/// values and is the authoritative runtime source **today**; the source-canonical
|
||||
/// `server/data/climate_constants.toml` mirrors those same values and is the file
|
||||
/// to edit when tuning. Runtime loading of the TOML ("tunable without recompile")
|
||||
/// lands with the production dispatch wiring (T-1032) — there is intentionally no
|
||||
/// `load()` method yet, so keep the TOML and the embedded `default()` in sync by
|
||||
/// hand until then.
|
||||
///
|
||||
/// Values mirror `tooling/planet-gen/planet_simulation.py`.
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -463,7 +471,7 @@ pub fn derive_temperature_c(params: &BodyParams, constants: &ClimateConstants) -
|
||||
|
||||
// Latitude gradient. Axial tilt modulates the equator–pole delta.
|
||||
let axial_tilt = params.axial_tilt_deg.unwrap_or(23.4); // Earth-like default
|
||||
// tilt_factor: 0.0 = no tilt (uniform insolation), 1.0 = 90° tilt (extreme)
|
||||
// tilt_factor: 1.0 = no tilt (full equator–pole gradient), 0.5 = 90° tilt (reduced gradient)
|
||||
let tilt_factor = 1.0 - (axial_tilt / 90.0) * 0.5;
|
||||
let lat_gradient_c = 60.0 * tilt_factor;
|
||||
let lat_frac = params.region_latitude_deg / 90.0; // [-1.0, 1.0]
|
||||
@@ -525,7 +533,9 @@ pub fn derive_moisture_q(params: &BodyParams) -> i32 {
|
||||
/// Temperature and moisture are derived inline via D-239 §2 climate functions
|
||||
/// (T-1024). Pass a `&ClimateConstants` to control the tuning constants.
|
||||
pub fn derive_region_profile(
|
||||
seed: SeedChain,
|
||||
// Reserved: per-region stochastic derivation (T-1027/T-1028) will derive
|
||||
// from this under a dedicated `SeedDomain::RegionProfile`. Unused today.
|
||||
_seed: SeedChain,
|
||||
body_params: &BodyParams,
|
||||
ta: &TerrainAnalysis,
|
||||
pos: RegionPos,
|
||||
@@ -584,20 +594,18 @@ pub fn derive_region_profile(
|
||||
ocean_fraction_q,
|
||||
);
|
||||
|
||||
// Per-region seed for future stochastic derivation within this region.
|
||||
// Currently only used as a forward-compatible derivation anchor.
|
||||
let _region_seed = seed.derive(SeedDomain::DomainWarp, region_pos_id(pos));
|
||||
|
||||
// Climate derivation (T-1024, D-239 §2). Build per-region BodyParams variant
|
||||
// that carries this region's latitude and elevation for temperature lapse.
|
||||
// The lat/elevation are set by the caller via body_params, but since BodyParams
|
||||
// is a body-level struct we derive them from the terrain here for the region.
|
||||
// elevation_km: derive from elev_q (0–100 normalized) × assumed body max elev 8 km.
|
||||
// region_latitude_deg: body_params.region_latitude_deg is caller-set; for the
|
||||
// derive_all_regions path it is set per-region using the region row index.
|
||||
// Here we use whatever the caller set (single-region path) or what derive_all_regions
|
||||
// passes in the per-region params.
|
||||
let temperature_c = derive_temperature_c(body_params, climate);
|
||||
// Climate derivation (T-1024, D-239 §2). Temperature lapse must vary by THIS
|
||||
// region's elevation — otherwise every region on a body shares one body-level
|
||||
// elevation and gets an identical lapse, defeating the per-district temperature
|
||||
// primitive. Build a region-local BodyParams whose elevation_km comes from the
|
||||
// region's own normalized elevation (elev_q, 0–100) scaled to the body's
|
||||
// elevation span. region_latitude_deg is already per-region (set by the caller
|
||||
// / derive_all_regions). Per-cell refinement happens later at ChunkContext.
|
||||
let region_climate_params = BodyParams {
|
||||
elevation_km: (elev_q as f64 / 100.0) * MAX_REGION_ELEVATION_KM,
|
||||
..body_params.clone()
|
||||
};
|
||||
let temperature_c = derive_temperature_c(®ion_climate_params, climate);
|
||||
let moisture_q = derive_moisture_q(body_params);
|
||||
|
||||
RegionProfile {
|
||||
@@ -660,22 +668,6 @@ pub fn derive_all_regions(
|
||||
out
|
||||
}
|
||||
|
||||
/// Map a `RegionPos` to a u64 id for seed derivation (zigzag + Cantor pairing).
|
||||
/// Matches the pattern in `domain_warp::pos_to_id`.
|
||||
#[inline]
|
||||
fn region_pos_id(pos: RegionPos) -> u64 {
|
||||
let zz = |v: i32| -> u64 {
|
||||
let v = v as i64;
|
||||
((v << 1) ^ (v >> 63)) as u64
|
||||
};
|
||||
let x = zz(pos.0);
|
||||
let y = zz(pos.1);
|
||||
let s = x.wrapping_add(y);
|
||||
s.wrapping_mul(s.wrapping_add(1))
|
||||
.wrapping_div(2)
|
||||
.wrapping_add(y)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user