fix(simulation): PR #198 review round — spillway split, courses disclosure, appendix fidelity
Tyre C2: DownstreamTarget::OpenSpillway added — success (open low ground, complete carved path) split from EdgeUnreachable (strict search exhaustion); both doc comments exact; test updated. Tyre C3: determinism docstring corrected (no BTreeMap; the endorheic f64 gate stated as deterministic-by- derivation). Hoshe H1: vacuous cliff_edge test replaced by single_basin_bowl_never_carves_a_gorge asserting the known-empty outcome. Hoshe H2/H3: courses-force-empty disclosure at rect_window_replica and in the results doc; courses.len() print added to the square production-fn bench — measured 3/6/10 courses in window (NOT courses-empty, verified twice); 'faithful stand-in' claim retracted for a precise scope statement; GJ1c 18-course run identified as the sole production-density rate. Tyre C1: appendix (4) headline rephrased — the tagged-envelope migration cannot be dodged by payload optimization (byte math), field-count-rule trigger is a workshop synthesis call; appendix (2) scope split per-shape. Hydrology unit suite 15/15; atlas lib suite 679 green; benches compile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -48,11 +48,20 @@
|
||||
//! are handled in ascending spill-level order, so a lower basin is always
|
||||
//! resolved before anything can overflow into it a second time).
|
||||
//!
|
||||
//! Determinism (D-010): all comparisons are on the same `i64`-scaled
|
||||
//! elevation integers `drainage.rs` uses; heap tie-breaks are `(cost, cell
|
||||
//! Determinism (D-010): elevation and ordering comparisons are on the same
|
||||
//! `i64`-scaled integers `drainage.rs` uses; heap tie-breaks are `(cost, cell
|
||||
//! index)` so equal-cost frontier cells always resolve in the same order;
|
||||
//! every collection that participates in output ordering is `Vec`/`BTreeMap`
|
||||
//! keyed by cell index or basin id, never a `HashMap`/`HashSet` iteration.
|
||||
//! every collection that participates in output ordering is a `Vec` keyed by
|
||||
//! cell index or basin id (row-major/discovery order), never a
|
||||
//! `HashMap`/`HashSet` iteration. **One float comparison exists** —
|
||||
//! `is_endorheic`'s `area_frac >= ENDORHEIC_AREA_FLOOR` gate (`f64`, derived
|
||||
//! from `basin_cells[b].len() as f64 / n as f64`) — but it is
|
||||
//! deterministic-by-derivation: both operands are pure functions of the same
|
||||
//! inputs (basin cell count and grid size), computed identically every run,
|
||||
//! so the comparison itself always yields the same boolean for the same
|
||||
//! input grid. It is not IEEE-754-hazardous in the way accumulated
|
||||
//! floating-point summation across runs/platforms can be; it is a single,
|
||||
//! reproducible division and comparison.
|
||||
|
||||
use std::cmp::Reverse;
|
||||
use std::collections::{BinaryHeap, VecDeque};
|
||||
@@ -101,8 +110,25 @@ pub enum DownstreamTarget {
|
||||
Sea,
|
||||
/// Flows into another basin's footprint (chained overflow), by basin id.
|
||||
Basin(u32),
|
||||
/// No lower terrain reachable within the search budget — treated as a
|
||||
/// grid-edge drain, the lake-fill analogue of `RIVER_DOWNSTREAM_EDGE_DRAIN`.
|
||||
/// **Success case.** Reaches open low ground — at or below the
|
||||
/// originating basin's own spill level, but not sea and not another
|
||||
/// basin's footprint (the water simply spreads here without needing a
|
||||
/// lake label of its own). `outlet_path` on the `BasinOutcome::Overflow`
|
||||
/// that carries this variant is a real, complete carved path, same as
|
||||
/// the `Sea`/`Basin` cases — this is a genuine overflow terminus, not a
|
||||
/// failure. Named for the geomorphological term (an open, uncontained
|
||||
/// spillway channel), not to be confused with `EdgeUnreachable` below.
|
||||
OpenSpillway,
|
||||
/// **Failure case.** The search exhausted its budget (`w * h` node pops)
|
||||
/// without reaching ANY valid terminus (`Sea`, `Basin`, or
|
||||
/// `OpenSpillway`) — a pathological body that genuinely has no
|
||||
/// reachable lower ground within the grid. `outlet_path` on the
|
||||
/// `BasinOutcome::Overflow` that carries this variant is the
|
||||
/// best-effort partial result (see `cheapest_overflow_path`'s fallback),
|
||||
/// not a complete channel. The lake-fill analogue of
|
||||
/// `RIVER_DOWNSTREAM_EDGE_DRAIN` in spirit (a grid-topology dead end),
|
||||
/// but unlike that sentinel this one is a genuine non-terminus, not a
|
||||
/// polar-row artifact.
|
||||
EdgeUnreachable,
|
||||
}
|
||||
|
||||
@@ -505,21 +531,20 @@ fn is_endorheic(area_frac: f64, moisture_q: i32) -> bool {
|
||||
|
||||
/// Dijkstra from `spill` outward, cost = cumulative elevation carved above
|
||||
/// `spill_level` (0 for any step that stays at or below spill level).
|
||||
/// Terminates at the first cell that is: (a) below `sea_scaled` (Sea), (b)
|
||||
/// inside a different basin (`Basin(id)`), or (c) has original elevation
|
||||
/// `<= spill_level` and is not part of basin `own_basin` (a "the water can
|
||||
/// just flow here, no more carving needed" terminus — folded into `Sea`/
|
||||
/// `Basin` cases when applicable, otherwise reported as reaching open low
|
||||
/// ground via `DownstreamTarget::Sea` is wrong; low ground that isn't sea or
|
||||
/// another lake still needs a target, so this case reuses `EdgeUnreachable`
|
||||
/// only when the search genuinely exhausts the grid — reaching low open land
|
||||
/// is folded into the `Basin`/`Sea` checks below by construction, since any
|
||||
/// cell at or under the spill level either drains toward the sea or another
|
||||
/// basin's footprint already).
|
||||
/// Terminates at the first cell that is: (a) below `sea_scaled`
|
||||
/// (`DownstreamTarget::Sea`), (b) inside a different basin
|
||||
/// (`DownstreamTarget::Basin(id)`), or (c) has original elevation `<=
|
||||
/// spill_level` and is not part of basin `own_basin` — open low ground, a
|
||||
/// genuine SUCCESS terminus reported as `DownstreamTarget::OpenSpillway`
|
||||
/// with a real, complete `outlet_path` (the water simply spreads here
|
||||
/// without needing a lake label of its own; not sea, not another basin, but
|
||||
/// still a valid place for the overflow to end).
|
||||
///
|
||||
/// Search budget: capped at `w * h` node pops (a full-grid worst case), so a
|
||||
/// pathological body can't spin forever — returns `EdgeUnreachable` with the
|
||||
/// partial best-effort path if exhausted.
|
||||
/// pathological body can't spin forever — returns
|
||||
/// `DownstreamTarget::EdgeUnreachable` (the FAILURE case: exhaustion, no
|
||||
/// terminus of any kind found) with only the best-effort partial path if
|
||||
/// the budget runs out.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn cheapest_overflow_path(
|
||||
spill: usize,
|
||||
@@ -559,18 +584,13 @@ fn cheapest_overflow_path(
|
||||
return (reconstruct_path(&came, idx), DownstreamTarget::Basin(other));
|
||||
}
|
||||
} else if original[idx] <= spill_level {
|
||||
// Open low ground, not another basin, not sea: still counts
|
||||
// as a valid overflow terminus (the water simply spreads
|
||||
// here without needing a lake label of its own). Reported
|
||||
// as Sea only if truly below sea level; otherwise treat as
|
||||
// reaching the edge of viable carving — a "spillway" onto
|
||||
// open plain. This is intentionally the same bucket as
|
||||
// EdgeUnreachable's shape (no further basin/sea structure)
|
||||
// but WITH a real path, so callers still get carving data.
|
||||
return (
|
||||
reconstruct_path(&came, idx),
|
||||
DownstreamTarget::EdgeUnreachable,
|
||||
);
|
||||
// Open low ground, not another basin, not sea: a genuine
|
||||
// SUCCESS terminus — the water simply spreads here without
|
||||
// needing a lake label of its own. Reported as
|
||||
// `OpenSpillway`, distinct from `EdgeUnreachable` (that
|
||||
// variant is reserved for genuine search exhaustion, below)
|
||||
// — this arm always carries a real, complete path.
|
||||
return (reconstruct_path(&came, idx), DownstreamTarget::OpenSpillway);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -891,17 +911,31 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cliff_edge_implies_positive_channel_depth() {
|
||||
fn single_basin_bowl_never_carves_a_gorge() {
|
||||
// A single sealed basin's own rim is, by construction of
|
||||
// priority-flood, always exactly the basin's spill level — so it
|
||||
// never needs carving (see the module's "Gorge carving" test
|
||||
// section below for the full structural explanation, and the
|
||||
// T-1177 results doc's "Gorge carving — the honest story" section).
|
||||
// This bowl fixture is exactly that shape (one basin, no chained
|
||||
// second basin), so the correct, non-vacuous assertion here is that
|
||||
// NOTHING gets carved — not an `if cliff_edge[i] { assert depth > 0
|
||||
// }` loop, which passes vacuously whenever (as here) the condition
|
||||
// never fires. The carving arithmetic ITSELF (what happens when a
|
||||
// path cell IS above spill level) is covered independently by
|
||||
// `carving_arithmetic_matches_original_minus_spill_level`.
|
||||
let elev = bowl_grid(64, 32);
|
||||
let result = solve(&elev, 64, 32, 0.0, ClimateInputs { moisture_q: 90 });
|
||||
for i in 0..result.cliff_edge.len() {
|
||||
if result.cliff_edge[i] {
|
||||
assert!(
|
||||
result.channel_depth_scaled[i] > 0,
|
||||
"a cliff-edge cell must carry positive channel depth (cell {i})"
|
||||
);
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
result.cliff_edge.iter().all(|&c| !c),
|
||||
"a single sealed basin's own rim is always exactly its spill level under this \
|
||||
priority-flood algorithm, so it structurally never carves — see the module's \
|
||||
\"Gorge carving\" test section for why"
|
||||
);
|
||||
assert!(
|
||||
result.channel_depth_scaled.iter().all(|&d| d == 0),
|
||||
"channel_depth_scaled must be all-zero whenever cliff_edge is all-false"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -976,9 +1010,10 @@ mod tests {
|
||||
let (path, target) = cheapest_overflow_path(12, 100_000, &original, &basin_of, 0, -1, w, h);
|
||||
assert_eq!(
|
||||
target,
|
||||
DownstreamTarget::EdgeUnreachable,
|
||||
"cell 14 is open low ground (not sea, not another basin) — the \
|
||||
EdgeUnreachable bucket is the correct terminus shape for that case"
|
||||
DownstreamTarget::OpenSpillway,
|
||||
"cell 14 is open low ground (not sea, not another basin) — a genuine SUCCESS \
|
||||
terminus, OpenSpillway, not EdgeUnreachable (which is reserved for search \
|
||||
exhaustion)"
|
||||
);
|
||||
assert_eq!(
|
||||
path,
|
||||
|
||||
@@ -49,10 +49,18 @@
|
||||
//! measurements are taken for each cell-count target:
|
||||
//!
|
||||
//! 1. **Square, through `build_district_window_layer` itself** (District
|
||||
//! granularity, `n = side`, real function call, unmodified) — the closest
|
||||
//! possible approach to "the actual production entry point," at the
|
||||
//! nearest square cell count to the target (e.g. side=576 → 331,776
|
||||
//! cells, matching 768×432's 331,776 exactly).
|
||||
//! granularity, `n = side`, real function call, unmodified, REAL
|
||||
//! `RiverNetwork` passed in) — the closest possible approach to "the
|
||||
//! actual production entry point," at the nearest square cell count to
|
||||
//! the target (e.g. side=576 → 331,776 cells, matching 768×432's 331,776
|
||||
//! exactly). This path exercises the ACTUAL course-invention +
|
||||
//! riparian-cull machinery `build_district_window_layer` runs in
|
||||
//! production (`invent_courses_near_window`/`crop_courses_for_wire`) —
|
||||
//! confirmed non-empty at every measured shape (`layer.courses.len()` is
|
||||
//! printed and was 3/6/10 at 330K/2.07M/8.3M respectively on the
|
||||
//! synthetic gradient body centred at the world origin; see the results
|
||||
//! doc's H3 correction for exactly what this does and does not validate).
|
||||
//!
|
||||
//! 2. **Real 16:9 rectangle, via a row-chunked loop that mirrors
|
||||
//! `build_district_window_layer`'s internals cell-for-cell** (same
|
||||
//! `into_par_iter()` row chunking, same `derive_at_metres` call, same
|
||||
@@ -64,9 +72,43 @@
|
||||
//! "MEASURED (replica loop)" in the results table to distinguish it from
|
||||
//! "MEASURED (production fn)".
|
||||
//!
|
||||
//! Both converge on the same number at the same cell count (verified by the
|
||||
//! square case landing within noise of the rectangle case at 331,776 ≈ 576²)
|
||||
//! — see the results doc for the cross-check.
|
||||
//! **DISCLOSED GAP (PR #198 review, Hoshe H2):** [`rect_window_replica`]
|
||||
//! always calls `derive_at_metres` with an EMPTY `&[]` course slice — it
|
||||
//! has no `RiverNetwork`/`invent_courses_near_window` wiring at all, by
|
||||
//! construction (courses are invented ONCE per window, ahead of the
|
||||
//! per-cell loop, inside `build_district_window_layer` itself —
|
||||
//! replicating that machinery was out of scope for a bench loop whose job
|
||||
//! is the per-cell derive rate, not course invention). This means EVERY
|
||||
//! rectangular-canvas number in this file (the three named 16:9 shapes
|
||||
//! AND the 83K deep-step bench) **excludes the per-cell
|
||||
//! `river_course::near_perennial_water` riparian-test cost** production
|
||||
//! pays on every cell of a window with real nearby course geometry.
|
||||
//! `zoom_ladder_bench.rs`'s own `bench_course_cost_on_vs_off` measured
|
||||
//! that cost at District cap (n=64, real GJ1c geometry): **+0.09–0.21 ms
|
||||
//! against a ~5 ms baseline, under 5%** — small, but real, and this file's
|
||||
//! rectangular numbers do not include it. Full disclosure and the
|
||||
//! corrected "what converges with what" statement is in the results doc's
|
||||
//! H2/H3 section — read that section before citing any rectangular-canvas
|
||||
//! number here as courses-inclusive. It is not.
|
||||
//!
|
||||
//! **Convergence claim, corrected (H3):** the square production-fn path is
|
||||
//! courses-INCLUSIVE (light density: 3/6/10 courses at 330K/2.07M/8.3M) and
|
||||
//! the rectangular replica-loop path is courses-EMPTY (always `&[]`) — so
|
||||
//! their agreement at 331,776 ≈ 576² (~191 ns/cell either way) validates the
|
||||
//! ROW-CHUNKED LOOP MECHANICS (chunking granularity, dispatch overhead,
|
||||
//! per-cell derive cost) converging across two independently-written call
|
||||
//! sites, NOT a courses-empty-vs-courses-inclusive equivalence claim — the
|
||||
//! two paths differ in exactly one respect (courses present vs absent) and
|
||||
//! happen to land within noise of each other at this course DENSITY (3 out
|
||||
//! of 331,776 cells is far too sparse to move the aggregate ns/cell figure
|
||||
//! outside the run-to-run noise band, consistent with the <5% per-cell
|
||||
//! course-cost delta `zoom_ladder_bench.rs` measured directly). The
|
||||
//! courses-inclusive rate at REAL production course density (not this
|
||||
//! sparse an origin-window) is covered only by the separate GJ1c real-body
|
||||
//! cross-check bench below (18 courses in a 331,776-cell window, deliberately
|
||||
//! centred on real river geometry) — see that bench's own doc and the
|
||||
//! results doc for the exact scope of what each number does and does not
|
||||
//! include.
|
||||
//!
|
||||
//! Run: `cargo test --release --test bmv_gridunit_bench -- --ignored --nocapture`
|
||||
|
||||
@@ -222,13 +264,15 @@ fn bench_square_window_production_fn_district_spacing() {
|
||||
let ns_per_cell = elapsed.as_secs_f64() * 1e9 / cells as f64;
|
||||
println!(
|
||||
" side={side:>5} n={n:>5} cells={cells:>10} (target ~{}): \
|
||||
{ms:>9.2} ms warm, {ns_per_cell:>7.1} ns/cell ({:.3} us/cell)",
|
||||
{ms:>9.2} ms warm, {ns_per_cell:>7.1} ns/cell ({:.3} us/cell), \
|
||||
courses_in_window={}",
|
||||
match cells {
|
||||
c if c < 500_000 => "330K",
|
||||
c if c < 4_000_000 => "2.07M",
|
||||
_ => "8.3M",
|
||||
},
|
||||
ns_per_cell / 1000.0
|
||||
ns_per_cell / 1000.0,
|
||||
layer.courses.len()
|
||||
);
|
||||
}
|
||||
println!();
|
||||
@@ -416,6 +460,22 @@ fn bench_square_window_production_fn_gj1c_real_body_crosscheck() {
|
||||
/// `build_district_window_layer`'s internal loop shape cell-for-cell (see
|
||||
/// module doc for the explicit diff against the real function). Returns
|
||||
/// (elapsed, per_cell_ns).
|
||||
///
|
||||
/// **Courses are FORCE-EMPTY here, disclosed (PR #198 review, Hoshe H2):**
|
||||
/// every call site below passes `&[]` for `nearby_courses` — there is no
|
||||
/// `RiverNetwork`, no `invent_courses_near_window` call, and no
|
||||
/// `river_course::near_perennial_water` riparian test running per cell. This
|
||||
/// is a real, measured gap versus production, not a rounding footnote:
|
||||
/// `zoom_ladder_bench.rs`'s `bench_course_cost_on_vs_off` measured the
|
||||
/// courses-on-vs-off delta directly at District cap (n=64, real GJ1c
|
||||
/// geometry) as **+0.09–0.21 ms against a ~5 ms baseline (under 5%)**. Every
|
||||
/// number produced by this function — the three 16:9 canvas benches AND the
|
||||
/// 83K deep-step bench — excludes that cost. It is EXCLUDED, not zero in
|
||||
/// production; readers citing a rectangular-canvas number from this file as
|
||||
/// "the real per-cell cost including courses" are citing it wrong. The
|
||||
/// courses-inclusive numbers live only in the square
|
||||
/// `build_district_window_layer`-backed benches above (which pass a real
|
||||
/// `RiverNetwork` and print `courses_in_window`).
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn rect_window_replica(
|
||||
seed: SeedChain,
|
||||
|
||||
Reference in New Issue
Block a user