From 9e0d60c28f9a8fc1b3df8ca2aa2b4a0a37ba2896 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 17:14:47 +0200 Subject: [PATCH 01/13] Increment 38 designed: threads and memory as settings (@architect) The first fix from the generality audit (rows D1, D2, D3, D5, D6, D7; section 6 item 1), as docs/increments/38-threads-and-memory.md plus the ROADMAP row. Design only; no production code. What it settles: - `--threads N` on mesh, catchment and station-catchments; 0 = every core; RASPUTIN_THREADS is the default when the option is absent (0 there means every core too); a count above the cores is honoured, not clamped (OpenMP and Rayon honour it; the benchmark sweeps to 20 on 10 cores). - One `Resources` model (threads, memory_budget), built once in cli.py from the options and os.environ, carried to open_dem and delineate; the leaf functions take the one integer they use. The C++ keeps a plain integer and its 0 default; no C++ file changes. - Pools default to one worker; resources.py is the only module that reads the machine (a test greps for it). The reprojector's cap of 8 goes; the reprojector inside the target grid's workers runs serial instead of opening a pool inside a pool. - `--memory-budget 8G` caps the mosaic canvas and the flood window; default half of min(physical, cgroup limit) with memory.max (v2) and memory/memory.limit_in_bytes (v1) read from a cgroup root the tests can point at a synthetic directory; cpu.max likewise for the core count. - Stats row "Threads used: N." for every DEM run; run record entry `threads`. - decompose.py and its test deleted; project_structure.md row swapped for resources.py. Measured on 5d2e706d: five _core entry points take threads and five call sites reach them, four passing nothing; three pools and four core-count reads plus one memory read (the greps and their output are in section 3). Estimate 140 added, 78 removed, net about 60 (the audit said 130 before counting call sites); worst case +39 % about 120 net. Prior art read on 2026-10-12: OpenMP 5.0 section 6.2, GNU nproc, Rayon's ThreadPoolBuilder, threadpoolctl, Python 3.13's os.process_cpu_count, the JDK's UseContainerSupport (JDK-8146115), the kernel's cgroup-v2 guide. Legacy greps against legacy-archive: nothing on threads; one `#include ` on memory. Nothing carried. Co-Authored-By: Claude Fable 5.1 --- ROADMAP.md | 1 + docs/increments/38-threads-and-memory.md | 553 +++++++++++++++++++++++ 2 files changed, 554 insertions(+) create mode 100644 docs/increments/38-threads-and-memory.md diff --git a/ROADMAP.md b/ROADMAP.md index 1f27f21f..ebeb2bc7 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -58,6 +58,7 @@ increment that most needs a picture to check against | 32 | Land-cover borders simplified within a band, each class keeping its area exactly (Ola, 2026-10-08: "we need to implement is a CORINE simplification, similar to what we do on the outline of the auto-catchments"): the land-cover coverage cut into borders between junctions, each border reduced by the area-preserving segment collapse of the catchment outline (`reduce_ring`), with junctions and the domain outline fixed, a crossing test and a side test across all borders, and a band no border moves past (two-sided, proved by anchors on the source border). `--features-tolerance` becomes that band, default 50 m (half CORINE's positional accuracy), replacing GEOS's coverage simplifier; start angle 15° when land cover is simplified (band above 0), 25° otherwise. Prototype on the German fused catchment: 5.7 times fewer triangles at 50 m vertical tolerance | **merged (#222, 2026-10-11)** (green `82c0a7d4`, 519 net production lines, one PR; mutation round `30f939b6`, one mutant recorded as not killed); Ola ruled all four questions (2026-10-08); code review round 1 asked for changes (comments, docstrings and `project_structure.md` the change made false), fixed by `4d4f0411` and the docs commit after it; code review rounds 1-3 done (round 3 approved); `@perf`'s short timing check found the runs about 70 % slower (the outline rule on borders clipped to the catchment) and the worst angle far lower (the simplifier placing points millimetres from junctions); fix built (section 15: outline rule first, a 1 m clearance in the simplifier; green `05c8f73a`, 573 counted lines in all; mutation round M7 to M13 killed); its quick check `b192a2c6`: Numedalslagen −11 %, Lagan −1.3 %, outline rule and moved area as before, triangles −42 %; questions 5 and 6 decided on default while Ola was away, reversible; fix code review round 1 asked for prose fixes and the test file's red-step staging removed; fix code review round 3 approved; next the push on Ola's yes | `docs/increments/32-landcover-simplify.md` | | 36 | GeoPackage output, so QGIS opens a mesh as it opens any vector layer: `rasputin mesh --out X.gpkg`, chosen by the suffix of `--out` as `.vtk` and `.ply` are (Ola, 2026-10-05: "Is the vtk_to_phkg.py the way to go, or as a direct output from rasputin? I think the latter, and it should go into the roadmap and eventually into the codebase."). Proposed content: a `triangles` layer, one PolygonZ per triangle in the run's CRS (z at each of the three corners, as the stopgap writes), with the attributes the run already knows: land-cover code and class name (the code system's names, as `rasputin palette` has them), mean z, plan area; a layer of the constraint polygons (the `--features` polygons clipped to the domain, with their code); optionally a QGIS style for land cover stored in the file (the `layer_styles` table), so the triangles open coloured. Land-cover colour: when the mesh carries CORINE classes, each triangle and each constraint polygon also gets its class's colour as a `#rrggbb` text column, so a reader can colour by the column without the stored style (in QGIS, a data-defined fill colour). The Graz file of 2026-10-05 carries `clc_color` on its `triangles` and `corine` layers; its stored styles categorise on `clc_code` and do not read the column. The colours are those of `rasputin palette` (`src_python/tin_engine/palettes.py`), the same 26 the Graz file carries: natural colours, as Ola chose over the official CLC legend in 16c's ruling 2 (that legend paints peat bogs blue and the sea almost white). **One attribute is not in the mesh yet**: the ID of the source polygon each triangle lies in (a sub-catchment cell, a parcel). Today only a class code labels triangles, one code system per mesh (16c, 16e's D1); the stopgap finds the cell by a centroid lookup outside rasputin. It needs a per-triangle array from a polygon source's ID property: its own small step, done just before this writer, because the `.vtk` and `.ply` outputs gain the attribute too (Ola, 2026-10-05, accepting the default: "defaults on the cell ID"). Written with the standard library's `sqlite3` beside the existing reader `src_python/tin_engine/io/geopackage.py`, which can read it back in the tests; no GDAL, OGR or Fiona (CLAUDE.md §2). Python only: the C++ core is unchanged and never sees the file (the I/O boundary). Prior art and working spec: Ola's stopgap `vtk_to_gpkg.py`, outside the repository (`rasputin_data/lands/scripts/`), which converts a mesh `.vtk` with `sqlite3`, NumPy, Shapely and PyProj, no GDAL, and its first use, the Mur above Graz (2 451 734 triangles, 10 m DEM, tolerance 10 m, 2026-10-05): **the GeoPackage was 677.9 MB against 110.6 MB for the binary `.vtk`**; the whole stopgap run took 39 s and 4.1 GB peak, and that includes the centroid cell lookup and the CORINE clipping, so it is not the cost of the GeoPackage encoding alone: the design measures the writer on its own. A GeoPackage row holds the triangle's whole ring (four corners, the first repeated) and a bounding box, and the file adds a spatial index, where `.vtk` stores each vertex once. Questions for the design: file size (keep the spatial index or not; at most an opt-in variant of 2D polygons with mean z as an attribute, proposed default: full PolygonZ; writing in chunks to bound memory); the CRS entry for a CRS with no EPSG code; where the run record (increment 25) goes; whether the constraint lines get a layer of their own | **merged (#231)** (branch `worktree-gpkg-out`: red `87f41c3d`, green `c59246e5`, fix-up `7085dddf`, 377 net production lines, mutation round `dddce8e8` ten of ten killed; `@perf`'s acceptance `57c25074` ACCEPTED: Numedalslagen 3.93 s / 203 MB, Romsdalen 19.4 s / 810 MB, every mesh byte-identical; code review round 1's one blocker, an empty land-cover layer crashing the write, fixed by `55cc3c3e` + `18442b27`, 378 net lines; code review round 2 APPROVED; merged 2026-10-12). Designed by `@architect`, 2026-10-11, on `81a551b3`; proposed by Ola 2026-10-05, "yes, roadmap row now, build after PR D"; PR D merged as #205, so the writer joins the encoders under `io/`). Measured on this design's probe: the quick check's Numedalslagen mesh (653,039 triangles) written in 4.0 s to 203 MB against 0.28 s and 29 MB for the binary `.vtk`, the stopgap 5.1 s; the Romsdalen slope mesh (3.1 million) in 21.5 s to 922 MB at 2.1 GB peak, the stopgap 28.7 s at 4.1 GB; the R-tree index is 60 % of the time; the same run writes the same bytes. Layers `triangles` (PolygonZ with code, name, colour, z_min, z_max, z_mean, area_m2, slope_deg), `lines`, `land_cover`, `land_cover_classes`, a `run` table with the run record, and the stored QGIS style; about 305 net lines, one PR; questions 1-5 answered with their defaults while Ola is away | `docs/increments/36-geopackage-output.md` | | 37 | An MCP (Model Context Protocol) server over the rasputin commands, so an external agent (Codex, Claude Desktop, ChatGPT, Mistral) discovers and calls rasputin as tools instead of reading its help text: `rasputin mcp` serves over stdio, one tool per command with a JSON schema derived from the Typer command's own parameters (one source of truth, so schema and CLI cannot drift), each call run as a child `python -m tin_engine ` so a cancel kills it and a crash ends only the child, with the exit code, output paths, run record and `--stats` returned as data and a refusal returned as a structured error carrying the CLI's plain words and exit code (Ola, 2026-10-11: "my working name for what's we're doing is intent-driven triangulation. This means accessing the tools using agents and intent"; "Add an MCP server to the ideas file after GeoPackage"; 2026-10-12: "Rule the mcp SDK: yes, use it"). Built on the official `mcp` Python SDK 2.3 as an optional extra `rasputin[mcp]`; stdio first, Streamable HTTP later or never; no Tasks extension (the SDK has none server-side); a jobs cap, default 1; the GeoPackage output (36) is the mesh an agent reads | **built and review-approved** (branch `worktree-mcp-server`: design `b990e38c` approved in three review rounds, red `ee5b5d52` 62 tests, green `851592ba`, three code review rounds with a README section, an `error`-field fix and a help-text escape, master merged in as `030fd32b`, 328 net production lines against 260; `dev` pulls the `mcp` extra; next the push on Ola's yes). Designed (`@architect`, 2026-10-12, on `b4a81c0d`): about 250 counted lines, one PR; nine decisions with defaults for Ola (section 6); measured by the design's probe: a tool call costs 0.25 s over the command itself (the child's import of `tin_engine.cli`), the 1 m benchmark mesh 0.72 s through the tool, a cancelled child dead within 0.5 s over stdio and in process, unknown and mistyped arguments refused before any child runs; no refine or mesh code, so no `@perf` run | `docs/increments/37-mcp-server.md` | +| 38 | Threads and memory as settings, the first fix from the generality audit (Ola, 2026-10-12: "the number of cores should of course not be hardcoded"): `--threads N` on `mesh`, `catchment` and `station-catchments` (0 = every core; `RASPUTIN_THREADS` is the default when the option is absent; neither set = every core, a container's CPU quota counting), carried as one `Resources` model built once in `cli.py` to every `_core` call that takes `threads` (five: `refine`, `refine_strip`, `refine_points`, `SlopeTolerance`, `class_areas`) and every Python pool (three: the reprojector, whose fixed cap of 8 goes, the target grid's resample and check points, the block decoder); the stats file and run record print the count used; `--memory-budget 8G` caps the mosaic and flood windows, default half the machine's memory with a container's cgroup limit (`memory.max`, v1 `memory.limit_in_bytes`) counting as the machine's; the unused `decompose.py` deleted with its test. Python only, no C++ change, about 60 net lines | designed (`docs/increments/38-threads-and-memory.md`) | `docs/increments/38-threads-and-memory.md` | | — | A missing station number refused in plain words. In a reference-polygon file (`rasputin station-catchments --reference`), a feature whose `station` is null becomes the text "None" (`src_python/tin_engine/io/station_set.py@ed12512:89`). No station can match it, since a station number must look like `2.11.0` (`src_python/tin_engine/hydrography.py@ed12512:40`), so the polygon is dropped without a word and the station it belonged to is written with no comparison (`src_python/tin_engine/catchment_batch.py@ed12512:220`); two such polygons are refused as "station None appears twice". In a stations file a null number is refused, but in Pydantic's validation text rather than a sentence naming the file (`src_python/tin_engine/io/station_set.py@ed12512:79`). NVE's files have a number on every station, so only hand-made or third-party files are hit. The fix: both readers refuse a null `station`, naming the file; one or two lines. Plainer GeoJSON error messages in general are not planned until other people feed rasputin their own files | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | A GeoTIFF whose coordinate-system GeoKeys hold garbage refused in plain words. The reader turns each code key into a whole number with `int()`, and a key stored as a double or as text can hold NaN, infinity or letters. Probed 2026-10-06 on hand-made files: NaN or text in `ProjectedCSTypeGeoKey` (3072) stops `rasputin mesh --dem` with Python's conversion message (for NaN, "Invalid value for --dem: cannot convert float NaN to integer"), which names neither the key nor the file, and infinity ends in a Python traceback (`OverflowError`) (`src_python/tin_engine/io/geotiff.py@ed12512:171`). The same holds for every other code key the reader reads: 1025, 4099, 3076, 3074, 3075, 2048 and 2054 (`src_python/tin_engine/io/geotiff.py@ed12512:310`, `src_python/tin_engine/io/geotiff.py@ed12512:176`, `src_python/tin_engine/io/geotiff.py@ed12512:339`, `src_python/tin_engine/io/geotiff.py@ed12512:352`, `src_python/tin_engine/io/geotiff.py@ed12512:362`, `src_python/tin_engine/io/geotiff.py@ed12512:499`, `src_python/tin_engine/io/geotiff.py@ed12512:423`). Worse, a fractional value is cut to a whole number first: 2.5 in 4099 is refused as 2, and 32633.5 in 3072 is read as EPSG:32633 without a word (`src_python/tin_engine/io/geotiff.py@ed12512:321`); 1.5 in 1025, 9001.7 in 4099 and 9001.5 in 3076 are likewise accepted as 1 and 9001 (probed by code review round 1 on this row's branch, exit 0). The parameter keys are already checked (`_one_number`, `src_python/tin_engine/io/geotiff.py@ed12512:454`). The fix: one check for every code key, a whole number or a refusal naming the key and its value. No real file is known to do this | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | tifffile's warning about the NoData tag silenced. On a float32 DEM whose `GDAL_NODATA` (42113) is the lowest float32 written in 17 digits (`-3.4028234663852886e+38`), tifffile prints "parsing GDAL_NODATA tag raised ValueError: ... is not castable to float32" to stderr when the file is opened (`src_python/tin_engine/io/geotiff.py@ed12512:146`). It is noise: rasputin reads the tag itself and accepts the value, since it is that float32 exactly (`src_python/tin_engine/io/geotiff.py@ed12512:535`). The fix: silence that one tifffile message while the reader opens the file, leaving every other tifffile warning visible | approved by Ola 2026-10-06; not designed (a small PR) | - | diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md new file mode 100644 index 00000000..90c02d25 --- /dev/null +++ b/docs/increments/38-threads-and-memory.md @@ -0,0 +1,553 @@ +# Increment 38: threads and memory as settings + +**Status: designed** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +`worktree-threads`). The first fix from the generality audit +(`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, +D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds +`--threads` (0 = every core, an environment variable as the default) to `mesh` +and `catchment`, passes it to every C++ call and Python pool, drops the fixed +cap of 8 threads on reprojection, prints the count used in the stats file, adds +`--memory-budget` honouring a container's limit, and deletes the unused +`decompose.py`. One PR, Python only, no C++ change. Full path (the diff is over +50 lines), no `@perf` acceptance run by the rule (no refine or mesh code is +touched), the quick check as the speed statement (section 9). + +## 1. What Ola asked + +Ola, 2026-10-12: "the number of cores should of course not be hardcoded. I'm a +bit shocked about the 'abstraction neglect' I've seen." Today every run uses +every core, one pool stops at 8 whatever the machine, the stats file prints +the core count as if it were the thread count, and the two memory caps read +the host's memory inside a container. Nothing lets a user, a shared server, a +laptop on battery, a CI job or an agent running several meshes at once say how +much of the machine a run may take. + +## 2. Prior art: legacy and literature + +**Legacy.** The legacy tree was serial and read no machine limit; nothing is +carried. The greps, run on this tree against the `legacy-archive` tag: + +``` +$ git grep -l -i -E 'cpu_count|hardware_concurrency|omp_get|num_threads|ThreadPool|multiprocessing' legacy-archive -- legacy +(nothing; exit 1) +$ git grep -l -i -E 'memory|rlimit|sysconf' legacy-archive -- legacy +legacy-archive:legacy/bindings.cpp # one hit: `#include `, not a limit +``` + +**Literature.** No novelty is claimed: "how many threads" is an engineering +convention, and the design follows the ones in use. What they do, each read +from its own documentation on 2026-10-12: + +- *OpenMP* (specification 5.0, section 6.2, `OMP_NUM_THREADS`): "The value of + this environment variable must be a list of positive integer values"; a + value that "is not a positive integer", or more threads than the + implementation supports, is "implementation defined". So 0 has no meaning in + OpenMP, and a count above the machine is left to the implementation (GCC and + LLVM honour it). +- *GNU coreutils `nproc`*: prints "the number of processing units available to + the current process, which may be less than the number of online + processors", so a `taskset` affinity mask is honoured; `OMP_NUM_THREADS` and + `OMP_THREAD_LIMIT` "determine the minimum and maximum returned value". +- *Rayon* (`ThreadPoolBuilder::num_threads`): "If `num_threads` is 0, or you do + not call this function, then the Rayon runtime will select the number of + threads automatically"; the automatic count "is based on the + `RAYON_NUM_THREADS` environment variable (if set), or the number of logical + CPUs (otherwise)". Rayon's 0 means "choose for me", which is what the C++ + core already does (`include/terrain/parallel_util/chunks.hpp@5d2e706d:17`, + "threads == 0 means hardware_concurrency"), and what `--threads 0` means here. +- *threadpoolctl* (`threadpool_limits`): "If None, this function does not do + anything"; an integer "sets the maximum number of threads"; no zero + convention; it reads no environment variable itself. Nothing to borrow + beyond "None means leave it". +- *Python 3.13* (`os.process_cpu_count()`): "the number of logical CPU cores + usable by the calling thread of the current process", so affinity counts; + both it and `os.cpu_count()` "can be overridden through the new environment + variable `PYTHON_CPU_COUNT` or the new command-line option `-X cpu_count`", + "useful for users who need to limit CPU resources of a container system". + The design uses it where present (this tree runs 3.14; `pyproject.toml` + allows 3.12, where it falls back to `os.cpu_count()`). +- *The JDK* (JDK-8146115, `-XX:+UseContainerSupport`, on by default since + OpenJDK 8u191 and 11): the CPU count and the memory limit are read from the + cgroup file system, `-XX:ActiveProcessorCount` overrides the count, + `-XX:MaxRAMPercentage` sizes the heap as a share of the container's memory. + That is the shape here: a container-aware default, a user override, a share + (half) as the memory default. +- *cgroup v2* (kernel `admin-guide/cgroup-v2`): `memory.max` is "a read-write + single value file which exists on non-root cgroups. The default is 'max'", + amounts in bytes; `cpu.max` is "$MAX $PERIOD", default "max 100000". In + cgroup v1 the memory limit is `memory/memory.limit_in_bytes`, which reads as + a very large number (about 2^63) when there is none. + +What differs from all of them: nothing. The one choice they do not settle, a +count above the core count, is honoured (section 4.2), as OpenMP's +implementations and Rayon honour it. + +## 3. Measured on this tree (`5d2e706d`) + +The three inventories, by grep, so the reader can re-run them: + +**Machine reads and pools.** `grep -rn -E 'cpu_count|ThreadPoolExecutor\(|sysconf' src_python/tin_engine`: + +``` +src_python/tin_engine/dem_input.py:218: source, threads = TileWindows(mosaic.tile), os.cpu_count() or 1 +src_python/tin_engine/target_grid.py:156: with ThreadPoolExecutor(max_workers=max(1, threads)) as pool: +src_python/tin_engine/crs.py:39:WORKERS = min(8, os.cpu_count() or 1) +src_python/tin_engine/crs.py:73: with ThreadPoolExecutor(len(chunks)) as pool: +src_python/tin_engine/mosaic.py:57: return os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") +src_python/tin_engine/cli.py:1590: threads=os.cpu_count() if sizes.start_vertices is not None else None, +src_python/tin_engine/io/cog.py:124: `threads` decoding workers, the machine's cores (`os.cpu_count() or 1`) +src_python/tin_engine/io/cog.py:203: with ThreadPoolExecutor(threads if threads is not None else os.cpu_count() or 1) as pool: +``` + +Three pools: the reprojector (`crs.py:73`, up to `WORKERS` = min(8, cores)), +the target grid's `_pool` (`target_grid.py:156`, serving `resample` at :200 +and `check_point_blocks` at :258, with the count from `dem_input.py:218`), and +the block decoder (`io/cog.py:203`, serving `decode_window` :141 and +`decode_class_window` :169, called without a count from +`src_python/tin_engine/io/repository.py@5d2e706d:148`, `:281` and `:285`). Four core-count reads +(`crs.py:39`, `dem_input.py:218`, `cog.py:203`, `cli.py:1590`) and one memory +read (`mosaic.py:57`), used as a cap at `src_python/tin_engine/mosaic.py@5d2e706d:187` and +`src_python/tin_engine/catchment.py@5d2e706d:263`. + +**`_core` entry points that take `threads`.** `grep -n 'threads' src_python/tin_engine/_core.pyi` +less the docstring lines: five, at `src_python/tin_engine/_core.pyi@5d2e706d:341` +(`SlopeTolerance.__init__`), `:450` (`refine`), `:562` (`refine_strip`), `:584` +(`refine_points`), `:764` (`class_areas`). Their call sites, +`grep -rn -E '\b(refine|refine_points|refine_strip|SlopeTolerance|class_areas)\(' src_python/tin_engine` +less the stub and the definitions: + +``` +src_python/tin_engine/cli.py:2137: slope = SlopeTolerance(to_core(tile), *ramp) +src_python/tin_engine/cli.py:2139: out = refine( +src_python/tin_engine/edge_strip.py:48: out = refine_strip( +src_python/tin_engine/final_check.py:60: out = refine_points( +src_python/tin_engine/land_cover_fractions.py:110: parts.append(_core.class_areas(local, classes.array, row_areas(classes), threads=threads)) +``` + +Five call sites. Four pass no `threads` (the C++ default 0 = every core); the +fifth passes `assign`'s `threads=0` default +(`src_python/tin_engine/land_cover_fractions.py@5d2e706d:94`), which no caller sets +(`src_python/tin_engine/cli.py@5d2e706d:1447`). The only route to another count today is the +benchmark's monkeypatch of `refine` (`tools/bench.py@5d2e706d:788-790`). + +**Reprojector call sites.** `grep -rn 'reprojector(' src_python/tin_engine` +less the definition: 14. Three move arrays that can reach the pool's threshold +of 500 000 points in a real run: every feature vertex +(`src_python/tin_engine/feature_input.py@5d2e706d:339`, inside `_Tally._take`), every mesh vertex +for the class fractions (`src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`), and the +target grid's blocks (`src_python/tin_engine/target_grid.py@5d2e706d:181` and `:239`, called from +inside `_pool`'s workers, so a block over the threshold today opens a second +pool of up to 8 threads inside each of the first pool's workers). The other +ten move a point, a domain outline, a lake, a seed or a station list. + +**This machine**, for the fixture figures below: 10 cores (`sysctl hw.ncpu`; +8 performance, 2 efficiency), 32 GiB (`hw.memsize` 34359738368), no cgroup; +`os.cpu_count()` and `os.process_cpu_count()` both 10 (Python 3.14.7). + +**`decompose.py`**: 93 lines, about 55 counted (blank, comment and docstring +lines dropped by hand with the counter's rule); its three commits are 23c-1's +green steps; nothing imports it (`grep -rn decompose src_python tools`: the +module and one docstring, `src_python/tin_engine/io/mesh_index.py@5d2e706d:37`); its only importer is +`tests/python/test_decompose_partition.py` (263 lines). + +## 4. The design + +### 4.1 One place reads the machine + +A new module `src_python/tin_engine/resources.py`, Pydantic and `os` only, +no `_core`, no typer. It is the one module allowed to read the machine; a test +greps for it (section 8, T1). + +```python +class Resources(BaseModel): + """What a run may use of the machine, resolved: never 0, never None.""" + model_config = ConfigDict(frozen=True) + threads: int = Field(ge=1) # the count every pool and _core call receives + memory_budget: int = Field(ge=1) # bytes one DEM window may take, flood or mosaic + + @classmethod + def fallback(cls) -> Resources: ... # one thread, half the machine's memory + +def machine_cores(cgroup: Path = CGROUP) -> int: ... +def machine_memory(cgroup: Path = CGROUP) -> int: ... +def parse_bytes(text: str) -> int: ... +def resources(threads: int | None, memory_budget: str | None, environ: Mapping[str, str]) -> Resources: ... +``` + +- `machine_cores()`: `os.process_cpu_count()` where Python has it (3.13+; + affinity and `PYTHON_CPU_COUNT` honoured), else `os.cpu_count()`, else 1; + then the smaller of that and the cgroup v2 CPU quota, `ceil(MAX / PERIOD)` + from `/cpu.max` when the file exists and its first word is not + `max`. Never under 1. +- `machine_memory()`: `os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE")` + (what `mosaic.physical_memory` does today, which moves here), then the + smaller of that and the cgroup limit: `/memory.max` (v2), else + `/memory/memory.limit_in_bytes` (v1). A file that is absent, + unreadable, reads `max`, or does not hold a positive whole number is no + limit; v1's "no limit" value (about 2^63) loses to the physical figure by + the `min` and needs no special case. +- `CGROUP = Path("/sys/fs/cgroup")`: the root as a container runtime mounts + the container's own cgroup. A limit set on an ancestor cgroup of a process + on a bare host (a systemd slice) is not read; stated as a limit in the + module docstring. The scale assumed: one container, one cgroup root; checked + against synthetic files only (T4), there being no container on this machine. +- `parse_bytes("8G")`: a whole number with an optional suffix K, M, G or T + (either case), 1024-based; anything else, or 0, is a `ValueError` naming the + text. +- `resources(threads, memory_budget, environ)`: the resolution rule of 4.2 and + 4.3; raises `ValueError` with plain words, which `cli.py` turns into a usage + error with `param_hint` `--threads`, `RASPUTIN_THREADS` or `--memory-budget`. + Pure: `environ` is passed in, as `cli.cache_root(option, environ)` already + does for `RASPUTIN_DATA` (`src_python/tin_engine/cli.py@5d2e706d:1756`). + +### 4.2 Threads: the rule + +| Given | Count used | +|---|---| +| `--threads N`, N ≥ 1 | N, even above the core count | +| `--threads 0` | `machine_cores()` | +| no option, `RASPUTIN_THREADS=N` | as the option: N ≥ 1 is N, 0 is every core | +| no option, `RASPUTIN_THREADS` unset or empty | `machine_cores()` | +| `--threads -1` | refused by Typer (`min=0`) | +| `RASPUTIN_THREADS=abc` or `-1` | usage error: "RASPUTIN_THREADS=abc is not a whole number of threads (0 means every core)" | + +Settled: + +- **A count above the core count is honoured, not clamped.** Reasons: OpenMP + and Rayon honour it; the C++ honours it today + (`include/terrain/parallel_util/chunks.hpp@5d2e706d:53` takes `min(threads, blocks)` workers, no core + check); the benchmark's thread sweep runs to 20 on this 10-core machine + (`tools/bench.py@5d2e706d:675`) and must keep meaning "20 threads"; and a + user asking for more than the cores is making a measurement, not a mistake. + The stats file shows the count, so the choice is visible. +- **`RASPUTIN_THREADS=0` means every core**, the same as `--threads 0`: one + rule for both sources, Rayon's convention, and the C++'s own. +- **The C++ keeps taking a plain integer.** Resources is a Python model; the + bindings and their default `threads = 0` are untouched (no C++ file in the + diff). Python always passes a resolved count of 1 or more, so the C++ never + sees 0 from the CLI; its 0 stays for direct callers and the C++ tests. +- **The pools' library defaults become one worker.** `reprojector(src, dst, *, + threads=1)`, `decode_window(..., threads=1)`, `decode_class_window(..., + threads=1)`, `_decode_into(..., threads)`: no module but `resources.py` + reads the core count. A library caller that passes nothing gets a serial + pool; the CLI passes its count everywhere a run's large arrays go (4.4). The + reprojector's `WORKERS` constant goes; `CHUNK_POINTS` stays (a chunk of + work, not a machine figure; the "about 60 ms" comment is reworded as one + machine's figure). +- **No clamp on the chunked reprojector either**: with `threads=9` and + 2 250 000 points (9 chunks of 250 000), nine workers, where today's cap gives + eight. With fewer chunks than threads, as many workers as chunks (today's + rule, kept: `min(threads, len(points) // CHUNK_POINTS)`). + +### 4.3 Memory: the rule + +| Given | Budget | +|---|---| +| `--memory-budget 8G` (or `512M`, `1T`, a plain number of bytes) | that many bytes | +| no option | `machine_memory() // 2`: half the machine's memory, the container's limit counting as the machine's | +| `--memory-budget 0`, `8GB`, `-1`, `abc` | usage error naming the text | + +The budget caps both DEM windows a run assembles: the mosaic canvas +(`src_python/tin_engine/mosaic.py@5d2e706d:187`, today "half the physical memory") and the flood +window (`src_python/tin_engine/catchment.py@5d2e706d:263`, the same words). `plan_mosaic` gains a +keyword `memory_budget: int | None = None` (None: `Resources.fallback()`'s); +the refusal reads "the window is R x C nodes, N bytes to flood, over the +memory budget of B bytes (half the machine's memory; --memory-budget changes +it)". `mosaic.physical_memory` is deleted; its one test (M5, +`tests/python/test_mosaic.py@5d2e706d:658-666`) moves to `resources`. + +Settled: **`--memory-budget` goes on `mesh` as well as `catchment` and +`station-catchments`**, because `mesh`'s mosaic cap reads the host's memory in +a container too (the same D6 defect, cited in the audit's D6 row), and the +model is built once per command. Default yes; question 1 in section 10. + +### 4.4 Where the value travels + +``` +cli.py mesh / catchment / station-catchments + │ threads: int | None = None (--threads, min=0) + │ memory_budget: str | None = None (--memory-budget) + │ res = resources(threads, memory_budget, os.environ) ← built once, here only + │ + ├─ _open_dem(..., res) ─ open_dem(request, resources=res) + │ ├─ repository_for(sources, nodata, cached, threads=res.threads) + │ │ TiffDemRepository / CacheRepository keep `threads`, pass it to + │ │ decode_window / decode_class_window (pool: io/cog.py) + │ ├─ plan_mosaic(..., memory_budget=res.memory_budget) (cap: mosaic.py) + │ └─ _open_reprojected(..., res): resample(grid, source, res.threads), + │ check_point_blocks(grid, source, domain, res.threads) (pool: target_grid.py) + │ and, inside those workers, reprojector(..., threads=1) (no pool in a pool) + ├─ _open_features(...): FeatureRequest(threads=res.threads) → _Tally → reprojector(threads) + ├─ _dem_mesh(..., res): + │ SlopeTolerance(view, *ramp, threads=res.threads) (_core) + │ refine(..., threads=res.threads) (_core) + │ edge_strip.run(..., threads=res.threads) → refine_strip(threads=) (_core) + │ final_check.run(..., threads=res.threads) → refine_points(threads=) (_core) + ├─ _class_fractions(..., res.threads): CacheRepository(threads=), assign(threads=) + │ → index_space(..., threads) → reprojector(threads); class_areas(threads=) (_core) + ├─ record: threads=res.threads → "Threads used" row in --record and --stats + └─ _write_report(..., threads=res.threads) → Report.threads → "Threads used: N." + +cli.py catchment / station-catchments + ├─ repository_for(dem, threads=res.threads) + ├─ delineate(request, repository, resources=res) + │ └─ _grow(..., budget=res.memory_budget): _plan → plan_mosaic(memory_budget=); + │ `need > budget` refuses before the flood (today's check, the budget in place of half the memory) + └─ run_batch(..., resources=res) → delineate(..., resources=res) per station +``` + +`delineate` and `open_dem` take `resources: Resources | None = None`; None is +`Resources.fallback()` (one thread, half the machine's memory), so every test +and library caller that passes nothing keeps working and reads no core count. +`Resources` travels whole through these two so a later field (say, an I/O +concurrency for `fetch`) changes no signature; the leaf functions take the one +integer they use. + +The stats line changes from "Threads: 10 (hardware concurrency)." +(`src_python/tin_engine/stats.py@5d2e706d:242`) to "Threads used: 3." and is printed for every DEM +run, not only refined ones (`src_python/tin_engine/cli.py@5d2e706d:1590`'s condition goes). The run +record gains the entry `threads` with the wording "Threads used", an Inputs +row (`run_record.py`'s `WORDING` and `INPUTS`), set by the refined and the +stride records; the gallery's flat record has no DEM and no row. + +### 4.5 What `--threads` does not reach, and why + +- **`fetch` and `fetch-stations`**: I/O-bound; `fetch` already has + `--connections` for its HTTP concurrency (`src_python/tin_engine/fetch/run.py@5d2e706d:95`) and + decodes no block (it stores them; `tifffile` is used for page headers + only). No option. +- **`draw`, `palette`, `version`, `mcp`**: no pool, no `_core` call that + takes threads. +- **The MCP server (increment 37)**: its schemas are derived from the Typer + parameters (`src_python/tin_engine/mcp/schema.py@5d2e706d:61-77`), so `threads` and + `memory_budget` appear in the `mesh`, `catchment` and `station_catchments` + tools with no server change, and an agent sets them per call; the child + inherits the server's environment (`src_python/tin_engine/mcp/server.py@5d2e706d:111`, + `env={**os.environ, **CHILD_ENV}`), so `RASPUTIN_THREADS` set where the + server starts is the default for every call. One test pins the first (T9). +- **`tools/bench.py`**: its child still monkeypatches `refine`'s `threads` + keyword (`tools/bench.py@5d2e706d:788-790`), which overrides the CLI's value, + so the benchmark is unchanged; switching it to `--threads` is a tooling idea + for later (section 11). + +### 4.6 `decompose.py` goes + +`src_python/tin_engine/decompose.py` and `tests/python/test_decompose_partition.py` +are deleted; `src_python/tin_engine/io/mesh_index.py@5d2e706d:37`'s docstring says "as the partition +rule of increment 23c gave them" instead of naming the module; +`project_structure.md`'s `decompose.py` row (`project_structure.md@5d2e706d:170`) +goes and a `resources.py` row comes ("the one module that reads the machine: +cores, memory, cgroup limits; Pydantic and os only"). `PartitionRecord` in +`mesh_index.py` keeps its `memory_budget` field: it records what a cut run was +given, and is read by nothing that needs the deleted rule. Ola's Q4 in the +audit, default delete, taken. + +### 4.7 Documentation in the same PR + +README's "Quick example" paragraph gets one sentence on `--threads` and +`RASPUTIN_THREADS`; INSTALL's environment paragraph (`INSTALL.md@5d2e706d:188`, +where `RASPUTIN_DATA` is introduced) names `RASPUTIN_THREADS` beside it and +`--memory-budget` with the container rule. Not counted lines. + +## 5. Boundaries checked + +- *Data versus execution*: `Resources` is data, resolved once at the CLI; the + algorithms take integers. The C++ sees a plain `unsigned`; nothing of the + model, the environment or the cgroup crosses the binding (I/O boundary + intact; no path reaches C++). +- *State*: no module-level machine read remains (`crs.WORKERS` at import time + was one); `resources.py` has no state, only functions and a frozen model. +- *Dependency gravity*: none added; `os`, `pathlib`, `re`, Pydantic. +- *Async-readiness*: `catchment_batch` already runs `delineate` in + `asyncio.to_thread`; a `Resources` per call lets an API worker give each job + its share. +- *Testability*: `machine_cores(cgroup)` and `machine_memory(cgroup)` take the + cgroup root, so a `tmp_path` with a synthetic `memory.max` tests the read + without a container; `resources(...)` takes `environ`; every pool takes its + count; no monkeypatch of `os` is needed anywhere in the new suite, and the + catchment cap test (`tests/python/test_catchment.py@5d2e706d:296-304`) drops + its two monkeypatches for a `Resources(memory_budget=...)`. + +## 6. Degenerate inputs and refusals + +- `--threads 0` on a machine whose count cannot be read (`os.cpu_count()` + None): 1. +- `cpu.max` reading `max 100000`: no quota. `cpu.max` reading `50000 100000`: + a quota of 0.5 CPU, `ceil` gives 1. `150000 100000`: 2. +- `memory.max` reading `max`: no limit; reading `0` or `-1` or text: ignored + (no limit), never a budget of 0. +- `--memory-budget 1` (one byte): accepted by the parser; every window is then + refused with the window's size and the budget in the message, which is the + honest answer to that request. +- A window exactly at the budget is accepted (`need > budget` refuses, as + today's `>`). +- `RASPUTIN_THREADS=" 4 "`: stripped, 4. `RASPUTIN_THREADS=""`: as unset. + +## 7. Left out, with reasons + +- A cgroup v1 CPU quota (`cpu/cpu.cfs_quota_us` and `cpu.cfs_period_us`): two + files and a second format for hosts on the old hierarchy; the memory limit + is read on both because the task names both and it is one file each. +- Ancestor cgroups on a bare host (walking `/proc/self/cgroup`): the + container case, which is D6's, mounts the container's cgroup at the root. +- A per-phase thread count (decode on N, refine on M): one number is what Ola + asked for; `Resources` can grow a field. +- `--threads` on `fetch` (4.5) and on the C++ default (the C++ is untouched). +- Reporting the machine's core count beside the count used ("3 of 10"): one + more `Report` field for a figure the user can get from `nproc`; left out to + keep the row plain. +- The quick check's `cases.toml` `threads` lists and `bench.py`'s child keep + their own `--threads` plumbing (4.5). + +## 8. Tests (`@tester`), red first + +Fixtures: the micro DEMs of `tests/python/cli_driver.py` (`rough_dem(16)`, a +16 x 16-node GeoTIFF; the geographic one of `test_cli_mesh_geographic.py`, +`tests/fixtures/velhas/anadem_velhas.tif`, for the resampled path) and the +land-cover fraction fixture of `test_cli_mesh_land_cover_fractions.py`. Pools +are observed as `tests/python/test_io_cog.py@5d2e706d:268-299` observes one: the pool class +monkeypatched in the module under test, recording `max_workers`. `_core` calls +are observed through a spy that wraps the real function in the module that +calls it (`cli.refine`, `cli.SlopeTolerance`, `edge_strip.refine_strip`, +`final_check.refine_points`, `land_cover_fractions._core.class_areas`), records +`kwargs["threads"]`, and calls through, so the run still produces its mesh. +Figures on this machine (10 cores, 32 GiB) are given where a test's expectation +depends on the machine; the test computes them from `resources` itself, never +from a literal. + +- **T1 one module reads the machine** (`tests/python/test_resources.py`): + `grep`-equivalent in Python over `src_python/tin_engine/**/*.py`: the + tokens `cpu_count`, `process_cpu_count`, `sched_getaffinity`, `sysconf` + and `/sys/fs/cgroup` occur in `resources.py` only. Red today on + `crs.py:39`, `dem_input.py:218`, `cog.py:203`, `cli.py:1590`, + `mosaic.py:57`. +- **T2 the threads rule** (section 4.2's table, one case each): `resources(None, + None, {})` is `machine_cores()`; `(0, None, {})` likewise; `(3, None, + {"RASPUTIN_THREADS": "7"})` is 3; `(None, None, {"RASPUTIN_THREADS": "7"})` + is 7; `"0"` is `machine_cores()`; `" 4 "` is 4; `""` is as unset; `"abc"` + and `"-1"` raise `ValueError` naming `RASPUTIN_THREADS` and the text; 64 on + a 10-core machine is 64. +- **T3 `machine_cores` with a synthetic `cpu.max`**: `tmp_path/cpu.max` = + `"200000 100000\n"` gives `min(2, count)` (2 here); `"max 100000"` and an + absent file give the plain count (10 here); `"50000 100000"` gives 1. +- **T4 `machine_memory` with synthetic files**: `tmp_path/memory.max` = + `"1073741824\n"` gives 1 GiB on this 32 GiB machine; `"max"` and an absent + file give `os.sysconf` physical memory (34359738368 here); v1 + `tmp_path/memory/memory.limit_in_bytes` = `"9223372036854771712"` gives the + physical figure, `"536870912"` gives 512 MiB; v2 wins when both exist. +- **T5 `parse_bytes`**: `"8G"` = 8589934592, `"512m"` = 536870912, `"1T"`, + `"4096"`; `"0"`, `"8GB"`, `"-1"`, `"abc"`, `""` raise naming the text. The + default budget is `machine_memory() // 2` (17179869184 here). +- **T6 the CLI option and the environment** (`test_cli_mesh_stats.py` or a new + `test_cli_threads.py`): `mesh --dem rough16 --tolerance 1 --threads 3 --stats -` + prints "Threads used: 3."; with `RASPUTIN_THREADS=5` in the runner's env and + no option, "Threads used: 5."; with both, the option's 3; `--threads -1` is + a usage error naming `--threads`; `RASPUTIN_THREADS=abc` is a usage error + naming `RASPUTIN_THREADS`; a stride run (no `--tolerance`) prints the row + too; `--record` carries `"threads": 3` with the wording "Threads used" among + the inputs. `catchment --threads 3` and `station-catchments --threads 3` + accept the option (the help lists it). +- **T7 every `_core` call site receives the value** (five tests, one per + site, each a spy): with `--threads 3`, `refine` and `refine_strip` get + `threads=3` on `rough_dem(16) --tolerance 1`; `refine_points` on the + resampled Velhas run (`--out-crs` path, which is the `final_check` route); + `SlopeTolerance` on a `--tolerance-slope` run; `class_areas` on the + land-cover fractions fixture. Each test asserts the mesh is still written + (the spy calls through). +- **T8 every pool receives the value** (three tests plus two defaults): the + block decoder's pool has `max_workers=3` on a `--threads 3` cached-source + run (`test_cli_mesh_cache.py`'s fixture) and on a tile run; the target + grid's `_pool` has `max_workers=3` on the Velhas `--out-crs` run, and the + reprojector called inside its workers has `threads=1` (spy on + `target_grid.reprojector`); the reprojector's pool has `max_workers=9` for + `reprojector(a, b, threads=9)` over 2 250 000 points (36 MB of float64 + pairs; 9 chunks of `CHUNK_POINTS`), where today's `WORKERS` gives 8, and + `max_workers=2` with `threads=9` over 500 000 points (two chunks). + Defaults: `reprojector(a, b)` over 2 250 000 points opens no pool + (`threads=1`: `one()` is called once); `decode_window` without `threads` + builds its pool with 1 worker (replacing + `tests/python/test_io_cog.py@5d2e706d:257-262`'s "the machine's cores"). + `tests/python/test_crs.py@5d2e706d:207-213`'s `WORKERS` assertion goes. +- **T9 the MCP schema carries the options**: the `mesh` entry of `derive(app)` has a model with + fields `threads` (int or None, default None) and `memory_budget` (str or + None), with no change to `mcp/`; `argv({"threads": 3})` ends with + `--threads 3`. +- **T10 the memory budget refuses a window above it**: `delineate(request, + repository, resources=Resources(threads=1, memory_budget=8192))` on + `tests/python/test_catchment.py@5d2e706d:296`'s 48 x 48 fixture (48 x 48 x 6 bytes = + 13 824 bytes to flood, over 8192) raises `CatchmentError` naming the window, + the bytes and the budget; with `memory_budget=13824` it floods (exactly at + the budget is accepted). The CLI: `catchment ... --memory-budget 8K` is the + same refusal as a usage error, `--memory-budget 8KB` names the text. The + mosaic cap: `plan_mosaic(footprints, memory_budget=1)` on `test_mosaic.py`'s + 2 x 2 lattice refuses naming `--memory-budget`; `tests/python/test_mosaic.py@5d2e706d:658-666` + (M5, `physical_memory`) becomes T4's physical case in `test_resources.py`. +- **T11 `decompose` is gone**: `importlib.util.find_spec("tin_engine.decompose")` + is None; `project_structure.md` has no line containing `decompose.py` and + one containing `resources.py`; `tests/python/test_decompose_partition.py` + is deleted in the red commit. +- **T12 wording**: `stats.render` with `threads=3` prints "Threads used: 3." + and never "hardware concurrency" (`tests/python/test_stats.py@5d2e706d:246` and `:370` + updated). + +No mutation round: the invariant-critical surface is five keyword arguments +and two file reads, whose failure mode is a typo T7, T8 and T4 catch. + +## 9. Estimate, speed, acceptance + +**Lines** (production, `CLAUDE.md` section 2's count; added in the new +revision, removed in the old): + +| Where | Added | Removed | +|---|---|---| +| `resources.py` (model, three readers, parser, resolution) | 55 | 0 | +| `cli.py` (two `Annotated` option aliases, three signatures, `resources()` once per command, threading through `_open_dem`, `_dem_mesh`, `_class_fractions`, `_write_report`, `catchment`, `station_catchments`, the record entry) | 32 | 4 | +| `dem_input.py`, `io/repository.py` (`resources` keyword, `threads` kept by two repositories, `repository_for`) | 16 | 2 | +| `io/cog.py`, `crs.py`, `target_grid.py` (defaults of 1, `WORKERS` gone, inner reprojector serial) | 8 | 6 | +| `feature_input.py`, `land_cover_fractions.py`, `edge_strip.py`, `final_check.py` | 12 | 2 | +| `mosaic.py`, `catchment.py`, `catchment_batch.py` (budget keyword, `physical_memory` gone, refusal wording) | 14 | 8 | +| `stats.py`, `run_record.py` | 3 | 1 | +| `decompose.py` deleted | 0 | 55 | +| **Total** | **140** | **78** | + +Net about **60**; the audit said about 130 (210 added, 80 removed) before the +call sites were counted. Worst case at +39 % on the additions: 195 added, net +about 120. Both far under 700. One PR. + +**Speed, one line:** the refine path is touched only by passing a value it +already takes (today's implicit 0 resolves to the same `hardware_concurrency` +count that `machine_cores()` returns on this machine, 10 = 10), so every mesh +is bit-identical and no timing change is expected, and `python3 +tools/bench_quick.py run` on the branch head against the stored baseline +confirms it; the one deliberate change in work scheduling, the reprojector +inside the target grid's workers running serial instead of opening a second +pool, can only shorten the `resample` phase of the Lagan GLO-30 case, and the +quick check's `decode`/`resample` rows are where it would show. + +**Acceptance by the rule:** no file under `include/terrain/refinement/`, +`include/terrain/mesh/` or `bindings/` changes, so no `@perf` acceptance run; +the quick check is the gate. + +## 10. Questions for Ola, with defaults + +1. Should `mesh` get `--memory-budget` too (its mosaic cap reads the host's + memory in a container as `catchment`'s does), or only `catchment` and + `station-catchments` as the audit worded it? Default: `mesh` too. +2. A thread count above the machine's cores: honour it (the benchmark's sweep + to 20 on 10 cores, OpenMP's and Rayon's practice) or clamp it to the cores? + Default: honour. +3. The container's CPU quota (`cpu.max`) counting toward "every core", as its + memory limit counts toward the memory default? Default: yes (three lines, + one synthetic-file test). +4. The stats row reads "Threads used: 3." Fine, or add the machine's count + ("3 of 10 cores")? Default: the short row. + +## 11. The ideas file + +Entries this design takes up or overlaps: *Product review (b), generality +audit* (this is its first fix); *MCP server over the rasputin commands* (an +agent now sets `threads` per call through the derived schema, and +`RASPUTIN_THREADS` in the server's environment is the default; nothing in +`mcp/` changes). Every other entry is untouched. New idea from this design, +for the handback: `tools/bench.py`'s child could pass `--threads` to the CLI +and drop its monkeypatch of `refine` (tooling, not counted). From 3ff484358bd071aed0e8d31621cb4623662b2890 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 17:29:07 +0200 Subject: [PATCH 02/13] 38 design review round 1 CHANGES REQUESTED recorded, by the main session Co-Authored-By: Claude Fable 5.1 --- docs/increments/38-threads-and-memory.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index 90c02d25..687e5384 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed, review round 1 CHANGES REQUESTED (five blockers, Review section), fixes next;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds @@ -551,3 +551,7 @@ agent now sets `threads` per call through the derived schema, and `mcp/` changes). Every other entry is untouched. New idea from this design, for the handback: `tools/bench.py`'s child could pass `--threads` to the CLI and drop its monkeypatch of `refine` (tooling, not counted). + +## Review + +**Design review, round 1, 2026-10-12.** Range `5d2e706d..9e0d60c2`, 0 production lines, estimate 60 net with worst case 120, @reviewer: CHANGES REQUESTED, five blockers. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/docs/increments/38-threads-and-memory.md@9e0d60c2:476-481: T10's figures are wrong; probed on the fixture, the first window is 49 x 49, the mosaic cap 9,604 bytes and the flood 14,406, so a budget of 8,192 is refused by the mosaic cap (`/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/catchment.py@5d2e706d:394`) naming 9,604, not the flood, and 13,824 also refuses; fix: a budget in 9,604 to 14,405 for the flood case, 14,406 for "exactly at", the `8K` CLI case restated. (2) @9e0d60c2:485-488: T11 misses `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/tests/python/test_layering.py@5d2e706d:44` (the `decompose` row), whose line 152 fails on a gone module; the red commit drops that row and adds `resources`. (3) @9e0d60c2:457-470: T8 spies no reprojector at `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/feature_input.py@5d2e706d:339` or `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`, two of the three large sites; with the library default of one worker a dropped `threads` serialises them unseen; one spy per site. (4) @9e0d60c2:133-141: 15 reprojector callers, not 14 (`target_grid.py:136`, `tolerance_field.py:83`, `feature_input.py:183` missing; "other ten" is twelve); `target_grid.py:239`'s blocks are 512 x 512 = 262,144 points, under the 500,000 threshold, so only line 181 nests a pool. (5) @9e0d60c2:177-181: `cpu.max` with a non-numeric first word or a zero period is unspecified; apply the memory rule (no quota). Suggestions: the refusal text "half the machine's memory" (:254-256) is false when a budget was given; section 4.5 should say `bench.py` children's stats row still shows the CLI's count; T9's `argv` is `DerivedCommand.argv(model)` (`src_python/tin_engine/mcp/schema.py@5d2e706d:108`). Verified: all 37 pinned cites; the legacy greps; the I/O boundary, the pinned assumptions and the MCP claims; `check_citations` 46 at-risk lines, all `ROADMAP.md` rows 27 and 29, unmoved. Lesson: T10's figures came from a test comment, not a run; one probe script settled it. From 3018192a7d283d03b0479496b579d0805ea05d40 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 18:10:32 +0200 Subject: [PATCH 03/13] Increment 38 design: review round 1's five blockers fixed, Ola's four rulings recorded (@architect) Design review round 1 (recorded at 3ff48435) asked for five fixes and three wordings; all are in docs/increments/38-threads-and-memory.md, cites pinned at 5d2e706d with directory paths. python3 tools/check_citations.py: exit 0, 46 at-risk lines, all ROADMAP.md rows, none in this file. (1) T10: the figures now come from a run, not the test's comment. Probe: delineate on tests/python/test_catchment.py:296-304's fixture with physical_memory faked to twice each budget. The run floods three windows, 49 x 49, 113 x 95 and 224 x 175 nodes; after each flood the growth check plans a probe window at the base margin (catchment.py:279), judged by the mosaic cap. Measured: 8192 refused by the mosaic cap ("49 x 49, 9604 bytes at float32"); 9604, 13824 and 14405 refused by the flood cap ("14406 bytes to flood"); 14406 floods the first window and is refused on the probe window ("73 x 55, 16060 bytes"); 235199 refused on the last window ("224 x 175, 235200 bytes to flood"); 235200 floods. The tests pin 12000 (flood cap), 14406 (names 73 x 55, so the first window passed exactly at its figure), 235199 and 235200 (exactly at the budget, three windows); the 8K CLI case is the mosaic refusal naming 49 x 49, 9604 and 8192. The reviewer's "14406 for exactly at" would still raise, on the second window; hence the extra pair at 235200. (2) T11: tests/python/test_layering.py:44's "decompose" row goes and a "resources" row (L1, no first-party import) comes in the red commit; :153 fails on the stale row, :152 on the missing one. (3) T8: one spy per large reprojector site: feature_input.reprojector on test_cli_mesh_features.py:328-338 (the EPSG:4326 GeoJSON over the EPSG:25833 DEM, the one fixture whose feature CRS differs), asserting threads=3 on the call whose source CRS is the file's; land_cover_fractions.reprojector on the fractions fixture (built on every run, the class raster being EPSG:4326). Five tests plus two defaults. (4) Reprojector callers: 15, the grep pasted with its file list. Four sites move large arrays at three places; target_grid.py:239 moves a block's rim and then at most 512 x 512 = 262 144 valid nodes (:32, :238, :252), under the 500 000 threshold (crs.py:70-73), so only :181 nests a pool today. The other eleven listed by line with what each moves. (5) cpu.max: the memory rule. A quota counts only when the file holds two words, both positive whole numbers (max is no quota; a zero or missing period is no quota, never a division); section 6 and T3 list the malformed files, each giving the plain count, never an exception. Wordings: the two refusals read "(set with --memory-budget; half the machine's memory when it is not set)", true with or without a budget; section 4.5 says bench.py's child prints "Threads used: 10." whatever the sweep's count, since its monkeypatch overrides refine only; T9 names DerivedCommand.argv(model) (mcp/schema.py:108). Section 10 is now "Ola's rulings": "Yes to all", 2026-10-12, taking each default (mesh gets --memory-budget; a count above the cores is honoured; a container's CPU quota counts toward every core; the row reads "Threads used: 3."), with sections 4.1-4.4 and 7 pointing at it. Status: designed, review round 1 fixes in; next round 2. Estimate unmoved at about 60 net. Co-Authored-By: Claude Fable 5.1 --- docs/increments/38-threads-and-memory.md | 249 +++++++++++++++++------ 1 file changed, 185 insertions(+), 64 deletions(-) diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index 687e5384..9c9bce5d 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed, review round 1 CHANGES REQUESTED (five blockers, Review section), fixes next;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed, review round 1 fixes in; next round 2;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds @@ -99,14 +99,20 @@ src_python/tin_engine/io/cog.py:124: `threads` decoding workers, the machine' src_python/tin_engine/io/cog.py:203: with ThreadPoolExecutor(threads if threads is not None else os.cpu_count() or 1) as pool: ``` -Three pools: the reprojector (`crs.py:73`, up to `WORKERS` = min(8, cores)), -the target grid's `_pool` (`target_grid.py:156`, serving `resample` at :200 -and `check_point_blocks` at :258, with the count from `dem_input.py:218`), and -the block decoder (`io/cog.py:203`, serving `decode_window` :141 and -`decode_class_window` :169, called without a count from +Three pools: the reprojector (`src_python/tin_engine/crs.py@5d2e706d:73`, up +to `WORKERS` = min(8, cores)), the target grid's `_pool` +(`src_python/tin_engine/target_grid.py@5d2e706d:156`, serving `resample` at +:200 and `check_point_blocks` at :258, with the count from +`src_python/tin_engine/dem_input.py@5d2e706d:218`), and the block decoder +(`src_python/tin_engine/io/cog.py@5d2e706d:203`, serving `decode_window` :141 +and `decode_class_window` :169, called without a count from `src_python/tin_engine/io/repository.py@5d2e706d:148`, `:281` and `:285`). Four core-count reads -(`crs.py:39`, `dem_input.py:218`, `cog.py:203`, `cli.py:1590`) and one memory -read (`mosaic.py:57`), used as a cap at `src_python/tin_engine/mosaic.py@5d2e706d:187` and +(`src_python/tin_engine/crs.py@5d2e706d:39`, +`src_python/tin_engine/dem_input.py@5d2e706d:218`, +`src_python/tin_engine/io/cog.py@5d2e706d:203`, +`src_python/tin_engine/cli.py@5d2e706d:1590`) and one memory read +(`src_python/tin_engine/mosaic.py@5d2e706d:57`), used as a cap at +`src_python/tin_engine/mosaic.py@5d2e706d:187` and `src_python/tin_engine/catchment.py@5d2e706d:263`. **`_core` entry points that take `threads`.** `grep -n 'threads' src_python/tin_engine/_core.pyi` @@ -131,14 +137,50 @@ fifth passes `assign`'s `threads=0` default benchmark's monkeypatch of `refine` (`tools/bench.py@5d2e706d:788-790`). **Reprojector call sites.** `grep -rn 'reprojector(' src_python/tin_engine` -less the definition: 14. Three move arrays that can reach the pool's threshold -of 500 000 points in a real run: every feature vertex -(`src_python/tin_engine/feature_input.py@5d2e706d:339`, inside `_Tally._take`), every mesh vertex -for the class fractions (`src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`), and the -target grid's blocks (`src_python/tin_engine/target_grid.py@5d2e706d:181` and `:239`, called from -inside `_pool`'s workers, so a block over the threshold today opens a second -pool of up to 8 threads inside each of the first pool's workers). The other -ten move a point, a domain outline, a lake, a seed or a station list. +less the definition: 15, the grep as run on `5d2e706d`: + +``` +src_python/tin_engine/land_cover_fractions.py:67: lonlat = reprojector(mesh_crs, meta.crs)(xy) +src_python/tin_engine/catchment_batch.py:180: move = reprojector(stations_crs, segments_crs) +src_python/tin_engine/domain.py:49: move = reprojector(source, target) +src_python/tin_engine/target_grid.py:136: back = reprojector(grid.crs, meta.crs) +src_python/tin_engine/target_grid.py:181: lonlat = reprojector(grid.crs, m.crs)(grid.xy(r0, r1)) +src_python/tin_engine/target_grid.py:239: move = reprojector(m.crs, grid.crs) +src_python/tin_engine/dem_input.py:212: ((ax, ay),) = reprojector(target, meta.crs)([(c.x, c.y)]) +src_python/tin_engine/catchment.py:222: ((x, y),) = reprojector(request.seed_crs, dem_crs)([request.seed]) +src_python/tin_engine/catchment.py:373: ((px, py),) = reprojector(request.seed_crs, request.lakes_crs)([request.seed]) +src_python/tin_engine/catchment.py:381: move = reprojector(request.lakes_crs, dem_crs) +src_python/tin_engine/tolerance_field.py:83: to_mesh = reprojector(found.crs, mesh_crs) +src_python/tin_engine/feature_input.py:183: xy = reprojector(dem_crs, source_crs)(xy) +src_python/tin_engine/feature_input.py:339: move = None if same_crs(src, self.dem) else reprojector(src, self.dem) +src_python/tin_engine/feature_input.py:563: ((x, y),) = reprojector(point_crs, own)([point]) +src_python/tin_engine/cli.py:2282: ((x, y),) = reprojector(seed_crs, crs)([seed]) +``` + +The pool opens at 500 000 points (`2 * CHUNK_POINTS`, +`src_python/tin_engine/crs.py@5d2e706d:70-73`). Three sites move arrays that +reach it in a real run: every feature vertex +(`src_python/tin_engine/feature_input.py@5d2e706d:339`, inside `_Tally._take`, +built only when the source's CRS differs from the DEM's), every mesh vertex +for the class fractions (`src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`), +and the target grid's resample blocks +(`src_python/tin_engine/target_grid.py@5d2e706d:181`, up to `BLOCK_NODES` = +1 048 576 nodes a block, `src_python/tin_engine/target_grid.py@5d2e706d:36` +and `:163`, called inside `_pool`'s workers, so a block over the threshold +today opens a second pool of up to 8 threads inside each of the first pool's +workers). A fourth site runs inside `_pool`'s workers too, +`src_python/tin_engine/target_grid.py@5d2e706d:239` (`check_point_blocks`), +but moves a block's rim and then its valid nodes, at most `BLOCK` x `BLOCK` = +512 x 512 = 262 144 points (`:32`, `:238`, `:252`), under the threshold, so it +never nests a pool; it takes `threads=1` all the same (4.4). The other eleven +move a point (`dem_input.py:212`, `catchment.py:222`, `:373`, +`feature_input.py:563`, `cli.py:2282`), a station list +(`catchment_batch.py:180`), a domain outline (`domain.py:49`), the target +grid's rim (`target_grid.py:136`), the lakes (`catchment.py:381`), the +feature hull ring densified to 1 km (`feature_input.py:183`) or one tolerance +line's vertices at a time (`tolerance_field.py:83`, one `shapely.get_parts` +part per call): none reaches 500 000 points in a run, and all become serial by +the library default of 4.2 with no change in behaviour. **This machine**, for the fixture figures below: 10 cores (`sysctl hw.ncpu`; 8 performance, 2 efficiency), 32 GiB (`hw.memsize` 34359738368), no cgroup; @@ -177,8 +219,11 @@ def resources(threads: int | None, memory_budget: str | None, environ: Mapping[s - `machine_cores()`: `os.process_cpu_count()` where Python has it (3.13+; affinity and `PYTHON_CPU_COUNT` honoured), else `os.cpu_count()`, else 1; then the smaller of that and the cgroup v2 CPU quota, `ceil(MAX / PERIOD)` - from `/cpu.max` when the file exists and its first word is not - `max`. Never under 1. + from `/cpu.max`. The memory rule applies to the file: a quota + counts only when the file is readable and holds two words, the first a + positive whole number (`max` is no quota) and the second a positive whole + number (a zero or missing period is no quota, never a division); anything + else is no quota. Never under 1. - `machine_memory()`: `os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE")` (what `mosaic.physical_memory` does today, which moves here), then the smaller of that and the cgroup limit: `/memory.max` (v2), else @@ -211,7 +256,7 @@ def resources(threads: int | None, memory_budget: str | None, environ: Mapping[s | `--threads -1` | refused by Typer (`min=0`) | | `RASPUTIN_THREADS=abc` or `-1` | usage error: "RASPUTIN_THREADS=abc is not a whole number of threads (0 means every core)" | -Settled: +Settled (the first two ruled by Ola, 2026-10-12, section 10): - **A count above the core count is honoured, not clamped.** Reasons: OpenMP and Rayon honour it; the C++ honours it today @@ -252,14 +297,19 @@ The budget caps both DEM windows a run assembles: the mosaic canvas window (`src_python/tin_engine/catchment.py@5d2e706d:263`, the same words). `plan_mosaic` gains a keyword `memory_budget: int | None = None` (None: `Resources.fallback()`'s); the refusal reads "the window is R x C nodes, N bytes to flood, over the -memory budget of B bytes (half the machine's memory; --memory-budget changes -it)". `mosaic.physical_memory` is deleted; its one test (M5, -`tests/python/test_mosaic.py@5d2e706d:658-666`) moves to `resources`. - -Settled: **`--memory-budget` goes on `mesh` as well as `catchment` and -`station-catchments`**, because `mesh`'s mosaic cap reads the host's memory in -a container too (the same D6 defect, cited in the audit's D6 row), and the -model is built once per command. Default yes; question 1 in section 10. +memory budget of B bytes (set with --memory-budget; half the machine's memory +when it is not set)", and the mosaic's "the request is R x C nodes, N bytes +at float32, over the memory budget of B bytes (set with --memory-budget; half +the machine's memory when it is not set); narrow it with --bbox". Both +sentences are true whether or not a budget was given, which the earlier +wording ("half the machine's memory") was not. `mosaic.physical_memory` is +deleted; its one test (M5, `tests/python/test_mosaic.py@5d2e706d:658-666`) +moves to `resources`. + +Ruled by Ola, 2026-10-12 (section 10): **`--memory-budget` goes on `mesh` as +well as `catchment` and `station-catchments`**, because `mesh`'s mosaic cap +reads the host's memory in a container too (the same D6 defect, cited in the +audit's D6 row), and the model is built once per command. ### 4.4 Where the value travels @@ -326,9 +376,14 @@ stride records; the gallery's flat record has no DEM and no row. `env={**os.environ, **CHILD_ENV}`), so `RASPUTIN_THREADS` set where the server starts is the default for every call. One test pins the first (T9). - **`tools/bench.py`**: its child still monkeypatches `refine`'s `threads` - keyword (`tools/bench.py@5d2e706d:788-790`), which overrides the CLI's value, - so the benchmark is unchanged; switching it to `--threads` is a tooling idea - for later (section 11). + keyword (`tools/bench.py@5d2e706d:788-790`), which overrides the CLI's value + for `refine` only, so the benchmark's timings are unchanged. Two things + follow and are stated so nobody reads them as defects: the child's stats + row reads "Threads used: 10." (the CLI's resolved count, every core on this + machine) whatever the sweep's count, because the row reports what the CLI + resolved, not what the monkeypatch passed; and the child's other `_core` + calls and pools run on the CLI's count. Switching the child to `--threads` + is a tooling idea for later (section 11). ### 4.6 `decompose.py` goes @@ -373,7 +428,10 @@ where `RASPUTIN_DATA` is introduced) names `RASPUTIN_THREADS` beside it and - `--threads 0` on a machine whose count cannot be read (`os.cpu_count()` None): 1. - `cpu.max` reading `max 100000`: no quota. `cpu.max` reading `50000 100000`: - a quota of 0.5 CPU, `ceil` gives 1. `150000 100000`: 2. + a quota of 0.5 CPU, `ceil` gives 1. `150000 100000`: 2. Malformed, by the + memory rule, all no quota: `abc 100000` (first word not a number), + `200000 0` (zero period), `200000` (one word), `200000 abc`, an empty file, + an unreadable file. - `memory.max` reading `max`: no limit; reading `0` or `-1` or text: ignored (no limit), never a budget of 0. - `--memory-budget 1` (one byte): accepted by the parser; every window is then @@ -395,7 +453,7 @@ where `RASPUTIN_DATA` is introduced) names `RASPUTIN_THREADS` beside it and - `--threads` on `fetch` (4.5) and on the C++ default (the C++ is untouched). - Reporting the machine's core count beside the count used ("3 of 10"): one more `Report` field for a figure the user can get from `nproc`; left out to - keep the row plain. + keep the row plain (Ola's ruling, section 10). - The quick check's `cases.toml` `threads` lists and `bench.py`'s child keep their own `--threads` plumbing (4.5). @@ -419,8 +477,11 @@ from a literal. `grep`-equivalent in Python over `src_python/tin_engine/**/*.py`: the tokens `cpu_count`, `process_cpu_count`, `sched_getaffinity`, `sysconf` and `/sys/fs/cgroup` occur in `resources.py` only. Red today on - `crs.py:39`, `dem_input.py:218`, `cog.py:203`, `cli.py:1590`, - `mosaic.py:57`. + `src_python/tin_engine/crs.py@5d2e706d:39`, + `src_python/tin_engine/dem_input.py@5d2e706d:218`, + `src_python/tin_engine/io/cog.py@5d2e706d:203`, + `src_python/tin_engine/cli.py@5d2e706d:1590` and + `src_python/tin_engine/mosaic.py@5d2e706d:57`. - **T2 the threads rule** (section 4.2's table, one case each): `resources(None, None, {})` is `machine_cores()`; `(0, None, {})` likewise; `(3, None, {"RASPUTIN_THREADS": "7"})` is 3; `(None, None, {"RASPUTIN_THREADS": "7"})` @@ -429,7 +490,10 @@ from a literal. a 10-core machine is 64. - **T3 `machine_cores` with a synthetic `cpu.max`**: `tmp_path/cpu.max` = `"200000 100000\n"` gives `min(2, count)` (2 here); `"max 100000"` and an - absent file give the plain count (10 here); `"50000 100000"` gives 1. + absent file give the plain count (10 here); `"50000 100000"` gives 1; the + malformed files of section 6 (`"abc 100000"`, `"200000 0"`, `"200000"`, + `"200000 abc"`, an empty file) each give the plain count, never an + exception. - **T4 `machine_memory` with synthetic files**: `tmp_path/memory.max` = `"1073741824\n"` gives 1 GiB on this 32 GiB machine; `"max"` and an absent file give `os.sysconf` physical memory (34359738368 here); v1 @@ -454,12 +518,27 @@ from a literal. `SlopeTolerance` on a `--tolerance-slope` run; `class_areas` on the land-cover fractions fixture. Each test asserts the mesh is still written (the spy calls through). -- **T8 every pool receives the value** (three tests plus two defaults): the +- **T8 every pool receives the value** (five tests plus two defaults, one + spy per large reprojector site of section 3): the block decoder's pool has `max_workers=3` on a `--threads 3` cached-source run (`test_cli_mesh_cache.py`'s fixture) and on a tile run; the target grid's `_pool` has `max_workers=3` on the Velhas `--out-crs` run, and the reprojector called inside its workers has `threads=1` (spy on - `target_grid.reprojector`); the reprojector's pool has `max_workers=9` for + `target_grid.reprojector`, which sees both the `:181` and the `:239` + call); the feature reprojector built in `_Tally._take` + (`src_python/tin_engine/feature_input.py@5d2e706d:339`) is called with + `threads=3` on a `--threads 3` run of the reprojected-source case + (`tests/python/test_cli_mesh_features.py@5d2e706d:328-338`, a GeoJSON in + EPSG:4326 over the EPSG:25833 DEM, the one feature fixture whose CRS + differs from the DEM's; spy on `feature_input.reprojector`, which also sees + the hull call at `:183`, so the assertion picks the call whose source CRS + is the feature file's); the class-fraction reprojector + (`src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`) is called + with `threads=3` on a `--threads 3` run of the land-cover fractions fixture + (spy on `land_cover_fractions.reprojector`; it is built on every fractions + run, the class raster being EPSG:4326). These two are the sites a dropped + `threads` would serialise unseen under the library default of one worker, + which is why each has its own spy. The reprojector's pool has `max_workers=9` for `reprojector(a, b, threads=9)` over 2 250 000 points (36 MB of float64 pairs; 9 chunks of `CHUNK_POINTS`), where today's `WORKERS` gives 8, and `max_workers=2` with `threads=9` over 500 000 points (two chunks). @@ -470,22 +549,59 @@ from a literal. `tests/python/test_crs.py@5d2e706d:207-213`'s `WORKERS` assertion goes. - **T9 the MCP schema carries the options**: the `mesh` entry of `derive(app)` has a model with fields `threads` (int or None, default None) and `memory_budget` (str or - None), with no change to `mcp/`; `argv({"threads": 3})` ends with - `--threads 3`. -- **T10 the memory budget refuses a window above it**: `delineate(request, - repository, resources=Resources(threads=1, memory_budget=8192))` on - `tests/python/test_catchment.py@5d2e706d:296`'s 48 x 48 fixture (48 x 48 x 6 bytes = - 13 824 bytes to flood, over 8192) raises `CatchmentError` naming the window, - the bytes and the budget; with `memory_budget=13824` it floods (exactly at - the budget is accepted). The CLI: `catchment ... --memory-budget 8K` is the - same refusal as a usage error, `--memory-budget 8KB` names the text. The - mosaic cap: `plan_mosaic(footprints, memory_budget=1)` on `test_mosaic.py`'s - 2 x 2 lattice refuses naming `--memory-budget`; `tests/python/test_mosaic.py@5d2e706d:658-666` - (M5, `physical_memory`) becomes T4's physical case in `test_resources.py`. + None), with no change to `mcp/`; `DerivedCommand.argv(model)` + (`src_python/tin_engine/mcp/schema.py@5d2e706d:108`) with the entry's model + built from `{"threads": 3}` plus the required arguments ends with + `--threads 3`, and with `{"memory_budget": "8G"}` ends with + `--memory-budget 8G`. +- **T10 the memory budget refuses a window above it**, on + `tests/python/test_catchment.py@5d2e706d:296-304`'s fixture (`bowl()` + through `repository_of`, `request(api)`). Its figures, measured by running + `delineate` on this tree with `physical_memory` faked to twice each budget + (so the cap is the budget), not read from the test's comment, which says + 48 x 48: the run floods three windows, 49 x 49, 113 x 95 and 224 x 175 + nodes (float32, 4 bytes a node for the mosaic, 6 to flood); after each + flood the growth check plans a probe window at the base margin + (`src_python/tin_engine/catchment.py@5d2e706d:279`), which `plan_mosaic`'s + cap judges too. Measured: + + ``` + budget 8192 refused by the mosaic cap: "the request is 49 x 49 nodes, 9604 bytes at float32, ..." + budget 9604 refused by the flood cap: "the window is 49 x 49 nodes, 14406 bytes to flood, ..." + budget 13824 refused by the flood cap: the same 14406 + budget 14405 refused by the flood cap: the same 14406 + budget 14406 first window floods; refused by the mosaic cap on the probe window: "73 x 55 nodes, 16060 bytes" + budget 235199 refused by the flood cap: "the window is 224 x 175 nodes, 235200 bytes to flood, ..." + budget 235200 floods, three windows + ``` + + The tests: `delineate(request, repository, resources=Resources(threads=1, + memory_budget=12000))` raises `CatchmentError` whose text names + "49 x 49", "14406 bytes to flood" and "12000" (the flood cap, not the + mosaic cap, which 12 000 passes); `memory_budget=14406` raises naming + "73 x 55" (so the first window was accepted at exactly its flood figure); + `memory_budget=235199` raises naming "224 x 175" and "235200"; + `memory_budget=235200` floods and returns three windows (exactly at the + budget is accepted). The test keeps the budgets as literals with the + measured figures beside them, since they are the fixture's, not the + machine's. The CLI: `catchment ... --memory-budget 8K` (8 192 bytes) is the + mosaic refusal naming "49 x 49", "9604" and "8192" as a usage error; + `--memory-budget 8KB` names the text. The mosaic cap on its own: + `plan_mosaic(footprints, memory_budget=1)` on `test_mosaic.py`'s 2 x 2 + lattice refuses naming `--memory-budget`; + `tests/python/test_mosaic.py@5d2e706d:658-666` (M5, `physical_memory`) + becomes T4's physical case in `test_resources.py`. - **T11 `decompose` is gone**: `importlib.util.find_spec("tin_engine.decompose")` is None; `project_structure.md` has no line containing `decompose.py` and one containing `resources.py`; `tests/python/test_decompose_partition.py` - is deleted in the red commit. + is deleted in the red commit. The layering table + (`tests/python/test_layering.py@5d2e706d:44`) has a `"decompose": ""` row + in L1, and its `test_the_table_names_every_module` (`:152-153`) fails on a + row for a gone module (`:153`, "rows for modules that are gone"); the red + commit drops that row and adds `"resources": ""` to L1 (`os` and Pydantic + only, no first-party import), so the table test is red on `resources` + being absent (`:152`, "modules with no row") and green once the module + exists. - **T12 wording**: `stats.render` with `threads=3` prints "Threads used: 3." and never "hardware concurrency" (`tests/python/test_stats.py@5d2e706d:246` and `:370` updated). @@ -512,7 +628,10 @@ revision, removed in the old): Net about **60**; the audit said about 130 (210 added, 80 removed) before the call sites were counted. Worst case at +39 % on the additions: 195 added, net -about 120. Both far under 700. One PR. +about 120. Both far under 700. One PR. Unmoved by review round 1: the +`cpu.max` rule (4.1) is one more condition in a reader already counted in +`resources.py`'s 55, the wording of two refusals changes no line count, and +the rest of the round's fixes are tests and prose. **Speed, one line:** the refine path is touched only by passing a value it already takes (today's implicit 0 resolves to the same `hardware_concurrency` @@ -528,19 +647,21 @@ quick check's `decode`/`resample` rows are where it would show. `include/terrain/mesh/` or `bindings/` changes, so no `@perf` acceptance run; the quick check is the gate. -## 10. Questions for Ola, with defaults - -1. Should `mesh` get `--memory-budget` too (its mosaic cap reads the host's - memory in a container as `catchment`'s does), or only `catchment` and - `station-catchments` as the audit worded it? Default: `mesh` too. -2. A thread count above the machine's cores: honour it (the benchmark's sweep - to 20 on 10 cores, OpenMP's and Rayon's practice) or clamp it to the cores? - Default: honour. -3. The container's CPU quota (`cpu.max`) counting toward "every core", as its - memory limit counts toward the memory default? Default: yes (three lines, - one synthetic-file test). -4. The stats row reads "Threads used: 3." Fine, or add the machine's count - ("3 of 10 cores")? Default: the short row. +## 10. Ola's rulings + +Four questions were put to Ola with defaults; he ruled "Yes to all" on +2026-10-12, taking each default. No question is open. + +1. **`mesh` gets `--memory-budget` too**, beside `catchment` and + `station-catchments` (its mosaic cap reads the host's memory in a + container as `catchment`'s does). Section 4.3. +2. **A thread count above the machine's cores is honoured, not clamped** + (the benchmark's sweep to 20 on 10 cores, OpenMP's and Rayon's practice). + Section 4.2. +3. **A container's CPU quota (`cpu.max`) counts toward "every core"**, as its + memory limit counts toward the memory default. Section 4.1. +4. **The stats row reads "Threads used: 3."**, without the machine's count. + Section 4.4; the longer row stays left out (section 7). ## 11. The ideas file From 1d9f0c7cb44d37f24091a6e6a2e1c904766e1d45 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 18:14:42 +0200 Subject: [PATCH 04/13] 38 design review round 2 APPROVED recorded, by the main session Co-Authored-By: Claude Fable 5.1 --- docs/increments/38-threads-and-memory.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index 9c9bce5d..7c16a428 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed, review round 1 fixes in; next round 2;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed and review-approved after two rounds; next `@tester`'s red suite (section 8);** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds @@ -676,3 +676,5 @@ and drop its monkeypatch of `refine` (tooling, not counted). ## Review **Design review, round 1, 2026-10-12.** Range `5d2e706d..9e0d60c2`, 0 production lines, estimate 60 net with worst case 120, @reviewer: CHANGES REQUESTED, five blockers. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/docs/increments/38-threads-and-memory.md@9e0d60c2:476-481: T10's figures are wrong; probed on the fixture, the first window is 49 x 49, the mosaic cap 9,604 bytes and the flood 14,406, so a budget of 8,192 is refused by the mosaic cap (`/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/catchment.py@5d2e706d:394`) naming 9,604, not the flood, and 13,824 also refuses; fix: a budget in 9,604 to 14,405 for the flood case, 14,406 for "exactly at", the `8K` CLI case restated. (2) @9e0d60c2:485-488: T11 misses `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/tests/python/test_layering.py@5d2e706d:44` (the `decompose` row), whose line 152 fails on a gone module; the red commit drops that row and adds `resources`. (3) @9e0d60c2:457-470: T8 spies no reprojector at `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/feature_input.py@5d2e706d:339` or `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`, two of the three large sites; with the library default of one worker a dropped `threads` serialises them unseen; one spy per site. (4) @9e0d60c2:133-141: 15 reprojector callers, not 14 (`target_grid.py:136`, `tolerance_field.py:83`, `feature_input.py:183` missing; "other ten" is twelve); `target_grid.py:239`'s blocks are 512 x 512 = 262,144 points, under the 500,000 threshold, so only line 181 nests a pool. (5) @9e0d60c2:177-181: `cpu.max` with a non-numeric first word or a zero period is unspecified; apply the memory rule (no quota). Suggestions: the refusal text "half the machine's memory" (:254-256) is false when a budget was given; section 4.5 should say `bench.py` children's stats row still shows the CLI's count; T9's `argv` is `DerivedCommand.argv(model)` (`src_python/tin_engine/mcp/schema.py@5d2e706d:108`). Verified: all 37 pinned cites; the legacy greps; the I/O boundary, the pinned assumptions and the MCP claims; `check_citations` 46 at-risk lines, all `ROADMAP.md` rows 27 and 29, unmoved. Lesson: T10's figures came from a test comment, not a run; one probe script settled it. + +**Design review, round 2, 2026-10-12.** Range `3ff48435..3018192a`, 0 production lines, estimate 60 net, @reviewer: APPROVED. Verified by running: T10's figures with the main venv (8,192 refused by the mosaic cap at 9,604; 9,604 to 14,405 by the flood cap at 14,406; 14,406 refused on the probe window 73 x 55 at 16,060; 235,199 refused naming 224 x 175; 235,200 floods three windows), every figure in the table matching; the `decompose` layering row and its two assertions; the three T8 spy sites and the EPSG:4326-over-25833 case; the caller grep reproducing 15 lines, `target_grid.py` under the 500,000 threshold; the `cpu.max` rule in 4.1 with its cases; section 10's four rulings stated as rulings with date and quote; check_citations exit 0. Idea, non-blocking: T3 omits section 6's unreadable-file case (chmod 000 in `tmp_path`, skipped as root). From 4b9c0dfe00dbcd80453b6174bef3b4d4b4f55a27 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 19:10:39 +0200 Subject: [PATCH 05/13] Increment 38 red: threads and memory as settings, T1 to T12 (@tester) The failing suite of docs/increments/38-threads-and-memory.md, section 8, before any production code. 110 tests are red, 85 of them in two new files; every one fails on a missing module, option, keyword or wording, none on a typo (the red run's distinct reasons: 65 "No module named tin_engine.resources", 19 "No such option: --threads" or "--memory-budget", 7 "plan_mosaic() got an unexpected keyword argument 'memory_budget'", 2 "reprojector() got an unexpected keyword argument 'threads'", the rest the old "Threads: 10 (hardware concurrency)" wording, the cap of 8 workers, the block decoder's default of 10 workers, crs.WORKERS still present, the layering table's missing row, decompose still importable, and the five modules that still read the machine). The 606 other tests in the touched files pass; the full Python suite passes apart from the red tests. New files: - tests/python/test_resources.py (T1 to T5, T11): the grep-equivalent that only resources.py holds cpu_count, sched_getaffinity, sysconf or /sys/fs/cgroup; the Resources model (frozen, never 0; fallback() is one thread and half the memory); the threads rule of section 4.2, one case each, with 1.5 and 0x10 added to abc and -1 as non-whole numbers; machine_cores on synthetic cpu.max files (2 CPUs, 0.5 rounds up to 1, 1.5 to 2, and every malformed case of section 6 plus an unreadable file, skipped as root); machine_memory on synthetic memory.max and v1 memory.limit_in_bytes (1 GiB and 512 MiB, under the machine as asserted; v1's 2^63 loses to the physical figure; v2 wins over v1; max, 0, -1, text and an empty file are no limit); parse_bytes and the default budget; and find_spec("tin_engine.decompose") is None. Every machine figure is computed from the module's own readers, never a literal. - tests/python/test_cli_threads.py (T6 to T8, T10's command line): the stats row "Threads used: N." from --threads, from RASPUTIN_THREADS, from both, from 0 and from nothing (every core, through machine_cores()), on a stride run too; the --record entry threads and the Inputs row "Threads used"; --threads -1 and RASPUTIN_THREADS=abc or -1 refused in the CLI's words; the help of mesh, catchment and station-catchments lists both options; a catchment run with --threads 3. One spy per _core site (cli.refine, edge_strip.refine_strip, final_check.refine_points on the Velhas --out-crs run, cli.SlopeTolerance on a --tolerance-slope run, land_cover_fractions._core.class_areas on the fractions fixture), each asserting the mesh is still written. One spy per pool (io/cog.py on a cached source and on a tile, target_grid.py's two pools on the Velhas run) and per large reprojector site (target_grid.reprojector with threads=1 inside the workers, feature_input.reprojector on the EPSG:4326 gallery over the EPSG:25833 DEM, picking the call from the file's CRS to the DEM's, land_cover_fractions.reprojector on the fractions fixture). --memory-budget 8K on the catchment fixture refused by the mosaic cap naming 49 x 49, 9604 and 8192; 8KB names the text; mesh --memory-budget 1 refused naming 17 x 21, 1428 and the option, 8G passes. The 8K and 12000 figures were re-measured through the CLI on test_cli_catchment's dem_dir with the cap faked to twice the budget before being written down. Changed: - test_catchment.py: the monkeypatched cap test becomes TestTheMemoryBudget through delineate(..., resources=Resources(threads=1, memory_budget=)): 12000 names 49 x 49, 14406 bytes to flood and 12000; 14406 names 73 x 55, 16060; 235199 names 224 x 175, 235200; 235200 floods three windows of the measured shapes; resources=None still floods. Resources is imported inside the helper so only these tests fail on its absence. - test_mosaic.py: M5 on plan_mosaic(..., memory_budget=) (the plan fixture passes the keyword only when given); the refusal names --memory-budget; physical_memory's test moves to test_resources.py as T4's plain case. - test_dem_input.py: the cap through open_dem(..., resources=). - test_io_cog.py: the default thread count is one worker (was the cores). - test_crs.py: crs.WORKERS is gone; the pool takes 9 workers for 9 chunks with threads=9 and 2 for 2 chunks; without a count no pool opens. - test_stats.py, test_cli_mesh_stats.py: "Threads used: 10.", printed on every DEM run and never on a fixture run (T12). - test_mcp_schema.py: T9, threads and memory_budget in the three tools' schemas and in argv. - test_layering.py: the decompose row goes, resources joins L1. - test_decompose_partition.py deleted (T11). Left out, by rule: T11's assertion on project_structure.md, which is a prose file under tools/ci_changes.py and so may not be read by a test (conftest.py, h11 T5); @reviewer reads its two rows instead. Co-Authored-By: Claude Fable 5.1 --- tests/python/test_catchment.py | 55 +++- tests/python/test_cli_mesh_stats.py | 7 +- tests/python/test_cli_threads.py | 403 +++++++++++++++++++++++ tests/python/test_crs.py | 54 ++- tests/python/test_decompose_partition.py | 263 --------------- tests/python/test_dem_input.py | 14 +- tests/python/test_io_cog.py | 29 +- tests/python/test_layering.py | 3 +- tests/python/test_mcp_schema.py | 20 ++ tests/python/test_mosaic.py | 81 +++-- tests/python/test_resources.py | 288 ++++++++++++++++ tests/python/test_stats.py | 20 +- 12 files changed, 889 insertions(+), 348 deletions(-) create mode 100644 tests/python/test_cli_threads.py delete mode 100644 tests/python/test_decompose_partition.py create mode 100644 tests/python/test_resources.py diff --git a/tests/python/test_catchment.py b/tests/python/test_catchment.py index 9be3cc47..88e6ac70 100644 --- a/tests/python/test_catchment.py +++ b/tests/python/test_catchment.py @@ -293,16 +293,51 @@ def test_a_catchment_cut_by_the_sentinel_is_refused(api: Any) -> None: api.delineate(request(api), repository_of(z, nodata=-32767.0)) -def test_the_memory_cap_refuses_before_the_flood(api: Any, monkeypatch: pytest.MonkeyPatch) -> None: - # The first window is the lake's box (7 cells) plus 2 x 20 cells: about 48 - # x 48 nodes. With physical memory 10 x that, 15a's cap (4 bytes a node - # against half of it) passes and the flood's (4 + 2 bytes a node) refuses. - first = 48 * 48 - fake = 10 * first - monkeypatch.setattr(mosaic, "physical_memory", lambda: fake) - monkeypatch.setattr(api, "physical_memory", lambda: fake, raising=False) - with pytest.raises(api.CatchmentError, match=r"(?i)memory"): - api.delineate(request(api), repository_of(bowl())) +def budget(bytes_: int) -> Any: + """Increment 38's `Resources`, one thread and this budget; imported here + so its absence fails these tests alone.""" + from tin_engine.resources import Resources + + return Resources(threads=1, memory_budget=bytes_) + + +class TestTheMemoryBudget: + """Increment 38, T10: `delineate(..., resources=)` caps both windows by the + budget, the mosaic canvas (4 bytes a node) and the flood (4 + 2). The + figures are this fixture's, measured on `5d2e706d` with the cap faked to + twice each budget (the design's table, checked in review round 2): the run + floods 49 x 49, 113 x 95 and 224 x 175 nodes, and after each flood plans + a probe window at the base margin, 73 x 55 after the first, which the + mosaic cap judges too. A budget of 12 000 passes the first mosaic cap + (9 604) and fails the first flood (14 406); exactly 14 406 floods the first + window and fails on the probe window; 235 199 fails the last flood + (235 200), which 235 200 passes: exactly at the budget is accepted.""" + + @pytest.mark.parametrize( + ("bytes_", "names"), + [ + pytest.param(12_000, ("49 x 49", "14406 bytes to flood", "12000"), id="first flood"), + pytest.param(14_406, ("73 x 55", "16060", "14406"), id="probe window after it"), + pytest.param(235_199, ("224 x 175", "235200", "235199"), id="last flood"), + ], + ) + def test_a_window_over_the_budget_is_refused_naming_it( + self, api: Any, bytes_: int, names: tuple[str, ...] + ) -> None: + with pytest.raises(api.CatchmentError) as info: + api.delineate(request(api), repository_of(bowl()), resources=budget(bytes_)) + message = str(info.value) + for name in (*names, "--memory-budget"): + assert name in message, f"{name!r} not in {message!r}" + + def test_a_window_exactly_at_the_budget_floods(self, api: Any) -> None: + result = api.delineate(request(api), repository_of(bowl()), resources=budget(235_200)) + assert [(w.rows, w.cols) for w in result.windows] == [(49, 49), (113, 95), (224, 175)] + + def test_without_resources_the_fallback_reads_no_core_count(self, api: Any) -> None: + """`resources=None` is `Resources.fallback()`: half the machine's + memory, so the fixture floods as before.""" + assert len(api.delineate(request(api), repository_of(bowl())).windows) == 3 def test_the_loop_stops_early_far_from_the_data_edge(api: Any) -> None: diff --git a/tests/python/test_cli_mesh_stats.py b/tests/python/test_cli_mesh_stats.py index 612e000b..75825370 100644 --- a/tests/python/test_cli_mesh_stats.py +++ b/tests/python/test_cli_mesh_stats.py @@ -292,6 +292,11 @@ def test_a_dem_without_tolerance(self, report_of: ReportOf) -> None: phases = seconds(report) assert "decode" in phases and "sample" in phases and "trim" in phases assert not any(name.startswith("refine") for name in phases) + assert "Threads used:" in section(report, "Timings") # 38: every DEM run, not only refined + + def test_a_fixture_run_has_no_threads_row(self, report_of: ReportOf) -> None: + report, _, _ = report_of("fixture") + assert "Threads" not in section(report, "Timings") def test_a_tolerance_run(self, report_of: ReportOf, monkeypatch: pytest.MonkeyPatch) -> None: outcomes: list[Any] = [] @@ -315,7 +320,7 @@ def spy(*args: Any, **kwargs: Any) -> Any: assert stats_row(report, "points_inserted_on_nodata") == "0" # no NoData in bumpy assert stats_row(report, "dem_nodes_outside_mesh") == "0" assert not re.search(r"\d+ rounds, \d+ points inserted", result.stderr), result.stderr - assert "Threads:" in section(report, "Timings") + assert "Threads used:" in section(report, "Timings") phases = seconds(report) for name in ( "refine", diff --git a/tests/python/test_cli_threads.py b/tests/python/test_cli_threads.py new file mode 100644 index 00000000..8c0a5d74 --- /dev/null +++ b/tests/python/test_cli_threads.py @@ -0,0 +1,403 @@ +"""`rasputin mesh|catchment|station-catchments --threads N --memory-budget B`: +increment 38, T6 to T8 and T10's command-line cases. + +`docs/increments/38-threads-and-memory.md`, sections 4.2 to 4.4 and 8. The +count a run resolves (`--threads`, else `RASPUTIN_THREADS`, else every core) +must reach every `_core` call that takes `threads` and every Python pool, and +be printed as "Threads used: N." Each site has its own spy, because a dropped +keyword falls back to the C++ default (every core) or the library default +(one worker) and the mesh comes out the same: only the spy sees it. + +Spies wrap the real function in the module that calls it and call through, +so every run still writes its mesh, which each test asserts. Pools are +observed as `test_io_cog.py` observes one: the pool class monkeypatched in +the module under test, recording `max_workers`. + +Nothing here times anything: the machine's load is not a test's business. + +T10's figures are the fixture's (`test_cli_catchment.dem_dir`, the bowl in +four tiles, through the lake's box plus the margin): the first window is 49 x +49 nodes, 9 604 bytes at float32 for the mosaic cap, 14 406 to flood. They +were measured on this tree (`5d2e706d`) with the cap faked to twice each +budget, through the CLI as well as `delineate`, and match the design's +table (review round 2). +""" + +from __future__ import annotations + +import importlib +import json +import re +from pathlib import Path +from typing import Any + +import pytest + +import tin_engine.cli as cli +import tin_engine.edge_strip as edge_strip +import tin_engine.feature_input as feature_input +import tin_engine.final_check as final_check +import tin_engine.io.cog as cog +import tin_engine.land_cover_fractions as land_cover_fractions +import tin_engine.target_grid as target_grid +from cli_driver import USAGE, UTM33, invoke, plain, polygon_file, ran, refused, rough_dem, runner +from cog_fixtures import write_cache +from feature_fixtures import Feat, moved, write_geojson +from fraction_fixtures import write_class_cache, write_dem, write_domain +from recordread import stats_row +from test_cli_catchment import dem_dir, lakes, utm_seed +from test_cli_mesh_cache import KEY as DEM_KEY +from test_cli_mesh_cache import catalogue, data, sources, tif +from test_cli_mesh_features import GALLERY, SQUARE +from test_cli_mesh_geographic import TARGET, VELHAS +from test_cli_mesh_land_cover_fractions import classes, covered, offline +from test_cli_mesh_stats import section +from test_cli_tolerance_slope import STEEP, box, v2 +from tin_engine.cli import app +from tin_engine.crs import same_crs +from vtkread import read_vtk + +__all__ = [ # fixtures, used by name + "box", "catalogue", "classes", "data", "dem_dir", "lakes", "offline", "sources", "tif", "v2", +] # fmt: skip + +bumpy = rough_dem(16) +plain_square = polygon_file(SQUARE) + +#: The three commands that take the two options. +TAKERS = ("mesh", "catchment", "station-catchments") +VELHAS_RUN = ( + *("--dem", str(VELHAS / "anadem_velhas.tif")), + *("--domain", str(VELHAS / "catchment.geojson")), + *("--out-crs", TARGET, "--tolerance", "5"), +) + + +@pytest.fixture(autouse=True) +def no_threads_variable(monkeypatch: pytest.MonkeyPatch) -> None: + """The developer's shell is not the test's default.""" + monkeypatch.delenv("RASPUTIN_THREADS", raising=False) + + +@pytest.fixture +def dem_cache(tmp_path: Path, data: bytes) -> Path: + """`test_cli_mesh_cache`'s cached DEM, under its own name here.""" + write_cache(tmp_path / "dem-cache", DEM_KEY, {"dem": data}) + return tmp_path / "dem-cache" + + +@pytest.fixture +def class_scene(tmp_path: Path) -> tuple[Path, Path, Path]: + """The land-cover fractions scene: (dem, domain, cache).""" + return ( + write_dem(tmp_path / "dem.tif"), + write_domain(tmp_path / "domain.geojson"), + write_class_cache(tmp_path / "class-cache"), + ) + + +class Calls: + """Every call of one function as a module uses it: the positional and + keyword arguments, and what it returned; calls through.""" + + def __init__(self, monkeypatch: pytest.MonkeyPatch, module: Any, name: str) -> None: + self.args: list[tuple[Any, ...]] = [] + self.kwargs: list[dict[str, Any]] = [] + self.out: list[Any] = [] + real = getattr(module, name) + + def spy(*args: Any, **kwargs: Any) -> Any: + self.args.append(args) + self.kwargs.append(kwargs) + self.out.append(real(*args, **kwargs)) + return self.out[-1] + + monkeypatch.setattr(module, name, spy) + + def threads(self) -> list[Any]: + """The `threads` keyword of each call, None where it was not given.""" + return [kw.get("threads") for kw in self.kwargs] + + +class Pools: + """The worker count of every `ThreadPoolExecutor` one module builds.""" + + def __init__(self, monkeypatch: pytest.MonkeyPatch, module: Any) -> None: + self.workers: list[int | None] = [] + real = module.ThreadPoolExecutor + + def spy(*args: Any, **kwargs: Any) -> Any: + self.workers.append(kwargs.get("max_workers", args[0] if args else None)) + return real(*args, **kwargs) + + monkeypatch.setattr(module, "ThreadPoolExecutor", spy) + + +def mesh(*args: str, env: dict[str, str] | None = None) -> Any: + """`rasputin mesh `, required to succeed; the runner's result.""" + result = runner.invoke(app, ["mesh", *args], env=env) + assert result.exit_code == 0, result.output + return result + + +def written(out: Path) -> None: + """The run wrote a mesh with triangles: the spies called through.""" + assert out.is_file(), out + assert len(read_vtk(out.read_bytes()).polygons) > 0 + + +def machine_cores() -> int: + return int(importlib.import_module("tin_engine.resources").machine_cores()) + + +# ---------------------------------------------------------------- T6 + + +class TestT6TheOptionAndTheVariable: + def test_the_option_sets_the_stats_row(self, tmp_path: Path, bumpy: Path) -> None: + out = tmp_path / "x.vtk" + args = ("--dem", str(bumpy), "--tolerance", "1", "--threads", "3", "--out", str(out)) + result = mesh(*args, "--stats", "-") + assert "Threads used: 3." in result.stdout + assert "hardware concurrency" not in result.stdout + + def test_the_variable_is_the_default(self, tmp_path: Path, bumpy: Path) -> None: + out = tmp_path / "x.vtk" + args = ("--dem", str(bumpy), "--tolerance", "1", "--out", str(out), "--stats", "-") + result = mesh(*args, env={"RASPUTIN_THREADS": "5"}) + assert "Threads used: 5." in result.stdout + + def test_the_option_beats_the_variable(self, tmp_path: Path, bumpy: Path) -> None: + out = tmp_path / "x.vtk" + args = ("--dem", str(bumpy), "--tolerance", "1", "--threads", "3", "--out", str(out)) + result = mesh(*args, "--stats", "-", env={"RASPUTIN_THREADS": "5"}) + assert "Threads used: 3." in result.stdout + + @pytest.mark.parametrize("how", ["--threads 0", "RASPUTIN_THREADS=0", "nothing"]) + def test_zero_or_nothing_is_every_core(self, tmp_path: Path, bumpy: Path, how: str) -> None: + out = tmp_path / "x.vtk" + args = ["--dem", str(bumpy), "--tolerance", "1", "--out", str(out), "--stats", "-"] + env = {"RASPUTIN_THREADS": "0"} if how == "RASPUTIN_THREADS=0" else None + if how == "--threads 0": + args += ["--threads", "0"] + result = mesh(*args, env=env) + assert f"Threads used: {machine_cores()}." in result.stdout + + def test_a_stride_run_prints_the_row_too(self, tmp_path: Path, bumpy: Path) -> None: + out = tmp_path / "x.vtk" + result = mesh("--dem", str(bumpy), "--threads", "3", "--out", str(out), "--stats", "-") + assert "Threads used: 3." in result.stdout + + def test_the_record_and_the_inputs_table_carry_the_count( + self, tmp_path: Path, bumpy: Path + ) -> None: + out, md, rec = tmp_path / "x.vtk", tmp_path / "x.md", tmp_path / "x.json" + mesh(*("--dem", str(bumpy), "--tolerance", "1", "--threads", "3", "--out", str(out)), + "--stats", str(md), "--record", str(rec)) # fmt: skip + record = json.loads(rec.read_text(encoding="ascii")) + assert record["threads"] == 3 and type(record["threads"]) is int + report = md.read_text(encoding="utf-8") + assert stats_row(report, "threads") == "3" + inputs = section(report, "Inputs") + assert re.search(r"^\| Threads used \| 3 \| `?threads`? \|$", inputs, re.M), inputs + vtk = read_vtk(out.read_bytes()) + assert "threads" not in vtk.field_data, "how a run was scheduled is not a mesh field" + + def test_a_negative_count_is_refused_naming_the_option( + self, tmp_path: Path, bumpy: Path + ) -> None: + args = ("--dem", str(bumpy), "--tolerance", "1", "--threads", "-1") + refused(tmp_path, "mesh", *args, says=("--threads",)) + + @pytest.mark.parametrize("text", ["abc", "-1"]) + def test_a_variable_that_is_not_a_count_is_refused_naming_it( + self, tmp_path: Path, bumpy: Path, text: str + ) -> None: + out = tmp_path / "x.vtk" + args = ["mesh", "--dem", str(bumpy), "--tolerance", "1", "--out", str(out)] + result = runner.invoke(app, args, env={"RASPUTIN_THREADS": text}) + output = plain(result.output) + assert result.exit_code == USAGE, output + assert "Traceback" not in output + assert "RASPUTIN_THREADS" in output and text in output, output + assert "0 means every core" in output + assert not out.exists() + + @pytest.mark.parametrize("command", TAKERS) + def test_the_help_lists_both_options(self, command: str) -> None: + text = ran(command, "--help") + assert "--threads" in text and "--memory-budget" in text + assert "RASPUTIN_THREADS" in text + + def test_catchment_runs_with_a_count(self, tmp_path: Path, dem_dir: Path, lakes: Path) -> None: + out = tmp_path / "c.geojson" + args = ("--dem", str(dem_dir), *utm_seed(), "--lakes", str(lakes), "--threads", "3") + ran("catchment", *args, "--out", str(out)) + assert out.is_file() + + +# ---------------------------------------------------------------- T7 + + +class TestT7EveryCoreCallGetsTheCount: + """Five `_core` entry points take `threads`; each call site passes the count.""" + + def test_refine_and_refine_strip( + self, tmp_path: Path, bumpy: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + phase = Calls(monkeypatch, cli, "refine") + strip = Calls(monkeypatch, edge_strip, "refine_strip") + out = tmp_path / "x.vtk" + mesh("--dem", str(bumpy), "--tolerance", "1", "--threads", "3", "--out", str(out)) + assert phase.threads() == [3] + assert strip.threads() == [3] + written(out) + + def test_refine_points_on_the_resampled_path( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + points = Calls(monkeypatch, final_check, "refine_points") + out = tmp_path / "x.vtk" + mesh(*VELHAS_RUN, "--threads", "3", "--out", str(out)) + assert points.threads() == [3] + written(out) + + def test_slope_tolerance( + self, tmp_path: Path, v2: Path, box: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + slope = Calls(monkeypatch, cli, "SlopeTolerance") + out = tmp_path / "x.vtk" + args = ("--dem", str(v2), "--domain", str(box), "--tolerance", "10", *STEEP) + mesh(*args, "--threads", "3", "--out", str(out)) + assert slope.threads() == [3] + written(out) + + def test_class_areas( + self, + classes: Any, + offline: None, + tmp_path: Path, + class_scene: tuple[Path, Path, Path], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + areas = Calls(monkeypatch, land_cover_fractions._core, "class_areas") + run = covered(tmp_path, *class_scene, "--threads", "3") + assert areas.threads() and set(areas.threads()) == {3} + assert len(run.vtk.polygons) > 0 + + +# ---------------------------------------------------------------- T8 + + +class TestT8EveryPoolGetsTheCount: + """Three pools, and the three reprojector sites whose arrays reach one.""" + + def test_the_block_decoder_on_a_cached_source( + self, catalogue: Any, dem_cache: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + pools = Pools(monkeypatch, cog) + out = tmp_path / "x.vtk" + args = ("--dem", DEM_KEY, "--cache", str(dem_cache), "--tolerance", "1", "--threads", "3") + mesh(*args, "--out", str(out)) + assert pools.workers and set(pools.workers) == {3}, pools.workers + written(out) + + def test_the_block_decoder_on_a_tile( + self, tif: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + pools = Pools(monkeypatch, cog) + out = tmp_path / "x.vtk" + mesh("--dem", str(tif), "--tolerance", "1", "--threads", "3", "--out", str(out)) + assert pools.workers and set(pools.workers) == {3}, pools.workers + written(out) + + def test_the_target_grids_pool_and_the_serial_reprojector_inside_it( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`resample` and `check_point_blocks` each open a pool of the count; + the reprojector their workers call takes `threads=1` (no pool in a + pool). The spy also sees the rim's call (`source_region`), which + passes nothing and so is serial by the library default.""" + pools = Pools(monkeypatch, target_grid) + moves = Calls(monkeypatch, target_grid, "reprojector") + out = tmp_path / "x.vtk" + mesh(*VELHAS_RUN, "--threads", "3", "--out", str(out)) + assert len(pools.workers) >= 2 and set(pools.workers) == {3}, pools.workers + explicit = [kw["threads"] for kw in moves.kwargs if "threads" in kw] + assert len(explicit) >= 2 and set(explicit) == {1}, moves.kwargs + assert all(kw.get("threads", 1) == 1 for kw in moves.kwargs) + written(out) + + def test_the_feature_reprojector( + self, tmp_path: Path, bumpy: Path, plain_square: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """`feature_input.py`'s `_Tally._take` builds a reprojector from the + file's CRS to the DEM's when they differ (the gallery in EPSG:4326 + over the EPSG:25833 DEM); that call carries the count. The hull's + call runs the other way (DEM to file) and is not the one asserted.""" + moves = Calls(monkeypatch, feature_input, "reprojector") + lonlat = [Feat(f.fid, moved(f.geometry, UTM33, "EPSG:4326"), f.properties) for f in GALLERY] # type: ignore[arg-type] + path = write_geojson(tmp_path / "wgs.geojson", lonlat, crs=None) + out = tmp_path / "x.vtk" + args = ("--dem", str(bumpy), "--domain", str(plain_square), "--tolerance", "1") + mesh(*args, "--features", str(path), "--threads", "3", "--out", str(out)) + from_the_file = [ + kw + for args_, kw in zip(moves.args, moves.kwargs, strict=True) + if same_crs(args_[0], "EPSG:4326") and same_crs(args_[1], UTM33) + ] + assert from_the_file, moves.args + assert all(kw.get("threads") == 3 for kw in from_the_file), from_the_file + written(out) + + def test_the_class_fraction_reprojector( + self, + classes: Any, + offline: None, + tmp_path: Path, + class_scene: tuple[Path, Path, Path], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """`land_cover_fractions.index_space` moves every mesh vertex to the + class raster's longitude and latitude: the one site in the module.""" + moves = Calls(monkeypatch, land_cover_fractions, "reprojector") + run = covered(tmp_path, *class_scene, "--threads", "3") + assert moves.threads() and set(moves.threads()) == {3}, moves.kwargs + assert len(run.vtk.polygons) > 0 + + +# ---------------------------------------------------------------- T10, the command line + + +class TestT10TheMemoryBudgetOnTheCommandLine: + def test_8k_is_refused_by_the_mosaic_cap_naming_the_figures( + self, tmp_path: Path, dem_dir: Path, lakes: Path + ) -> None: + out = tmp_path / "c.geojson" + args = ("--dem", str(dem_dir), *utm_seed(), "--lakes", str(lakes), "--memory-budget", "8K") + code, output = invoke("catchment", *args, "--out", str(out)) + assert code == USAGE, output + assert "No such option" not in output and "Traceback" not in output + for word in ("49 x 49", "9604", "8192", "--memory-budget"): + assert word in output, f"{word!r} not in {output!r}" + assert not out.exists() + + def test_a_wrong_suffix_is_refused_naming_the_text( + self, tmp_path: Path, dem_dir: Path, lakes: Path + ) -> None: + out = tmp_path / "c.geojson" + args = ("--dem", str(dem_dir), *utm_seed(), "--lakes", str(lakes), "--memory-budget", "8KB") + code, output = invoke("catchment", *args, "--out", str(out)) + assert code == USAGE, output + assert "No such option" not in output and "Traceback" not in output + assert "8KB" in output and "--memory-budget" in output, output + assert not out.exists() + + def test_mesh_takes_the_budget_too(self, tmp_path: Path, bumpy: Path) -> None: + """Ola's ruling 1: `mesh`'s mosaic cap is the budget. `rough_dem` is + 17 x 21 nodes, 1 428 bytes at float32; one byte refuses, 8G passes.""" + args = ("--dem", str(bumpy), "--tolerance", "1", "--memory-budget", "1") + refused(tmp_path, "mesh", *args, says=("17 x 21", "1428", "--memory-budget")) + out = tmp_path / "x.vtk" + mesh("--dem", str(bumpy), "--tolerance", "1", "--memory-budget", "8G", "--out", str(out)) + written(out) diff --git a/tests/python/test_crs.py b/tests/python/test_crs.py index f4e87b78..0c0eb51c 100644 --- a/tests/python/test_crs.py +++ b/tests/python/test_crs.py @@ -49,7 +49,6 @@ import ast import importlib -import os from pathlib import Path from types import ModuleType, SimpleNamespace from typing import Any @@ -190,6 +189,19 @@ def test_no_vertex_is_added_or_dropped(self, crs: ModuleType) -> None: LAEA, SWEREF = "EPSG:3035", "EPSG:3006" +def pool_spy(crs: ModuleType, monkeypatch: pytest.MonkeyPatch) -> list[int | None]: + """The worker count of every pool `crs` builds; the real pool is built.""" + built: list[int | None] = [] + real = crs.ThreadPoolExecutor + + def spy(*args: Any, **kwargs: Any) -> Any: + built.append(kwargs.get("max_workers", args[0] if args else None)) + return real(*args, **kwargs) + + monkeypatch.setattr(crs, "ThreadPoolExecutor", spy) + return built + + def southern_sweden(n: int) -> np.ndarray: rng = np.random.default_rng(35) return np.column_stack([rng.uniform(4.3e6, 4.9e6, n), rng.uniform(3.5e6, 4.1e6, n)]) @@ -207,10 +219,44 @@ class TestChunkedReprojector: cover, 2, 4 and 8 threads) in the design.""" def test_the_constants_are_the_designs(self, crs: ModuleType) -> None: - """Section 3.1 (b): a 250 000-point chunk is about 60 ms of PROJ - work; at most 8 threads, fewer on a smaller machine.""" + """Section 3.1 (b): a 250 000-point chunk is a chunk of work (about + 60 ms of PROJ on one machine). Increment 38 (section 4.2) drops the + cap of 8 workers: `reprojector` takes `threads` from its caller, and + no module but `resources.py` reads the core count.""" assert crs.CHUNK_POINTS == 250_000 - assert min(8, os.cpu_count() or 1) == crs.WORKERS + assert not hasattr(crs, "WORKERS") + + @pytest.mark.parametrize( + ("n", "threads", "workers"), + [ + pytest.param(2_250_000, 9, 9, id="nine-chunks-nine-workers-where-8-was-the-cap"), + pytest.param(500_000, 9, 2, id="two-chunks-two-workers"), + ], + ) + def test_the_pool_takes_the_count_given_up_to_one_worker_a_chunk( + self, crs: ModuleType, monkeypatch: pytest.MonkeyPatch, n: int, threads: int, workers: int + ) -> None: + """Increment 38, T8: `min(threads, len(points) // CHUNK_POINTS)` + workers, no clamp to the machine. 2 250 000 points are 36 MB of + float64 pairs, 9 chunks.""" + built = pool_spy(crs, monkeypatch) + xy = southern_sweden(n) + out = crs.reprojector(LAEA, SWEREF, threads=threads)(xy) + assert built == [workers] + x, y = Transformer.from_crs(LAEA, SWEREF, always_xy=True).transform(xy[:, 0], xy[:, 1]) + assert np.array_equal(out, np.column_stack([x, y])) + + def test_without_a_count_no_pool_opens( + self, crs: ModuleType, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The library default is one worker: a 2 250 000-point array is + mapped in one call, bit for bit the same.""" + built = pool_spy(crs, monkeypatch) + xy = southern_sweden(2_250_000) + out = crs.reprojector(LAEA, SWEREF)(xy) + assert built == [] + x, y = Transformer.from_crs(LAEA, SWEREF, always_xy=True).transform(xy[:, 0], xy[:, 1]) + assert np.array_equal(out, np.column_stack([x, y])) @pytest.mark.parametrize( "n", diff --git a/tests/python/test_decompose_partition.py b/tests/python/test_decompose_partition.py deleted file mode 100644 index 9b1ff7a4..00000000 --- a/tests/python/test_decompose_partition.py +++ /dev/null @@ -1,263 +0,0 @@ -"""DC0: the partition rule of increment 23c, pure (no DEM, no mesh). - -`docs/increments/23-basin-scale.md`, "The partition (23c; B2, B3 and B14 as -ruled)" and "The memory estimate and the defaults (B14 as ruled)"; the -figures are the ones `docs/increments/23-probes/partition.py` prints, and the -cell count approximates the request and is not a floor (the review of B13 and -B14, round 1). - -The surface, pinned at the red step and confirmed under "Settled after 23c's -red step (65e3990)", item 7: `tin_engine.decompose` holds - -- `DEFAULT_MEMORY_BUDGET`, 16 GiB in bytes (2**34); -- `bytes_per_node(tolerance) -> int`, the `b(T)` table, linear between its - columns and rounded up, 19 above 50 m; -- `partition(cols, rows, tolerance, *, pieces=1, memory_budget=DEFAULT_MEMORY_BUDGET)` - returning an object with integer `nx`, `ny`, `dx`, `dy` (cells across and - down, and their sides in whole lattice nodes). - -Two refusals the design does not state, pinned as `ValueError`: `pieces < 1`, -and a budget under one node's bytes (`memory_budget < b(T)`), for which step -3's loop would never end (no cell of one node fits). - -Tolerances in the interpolation cases are binary fractions (0.25, 0.5, 0.75, -1.5, 7.5), so the rounding up is of an exact value and a correct -implementation cannot land one above it by floating-point noise. -""" - -from __future__ import annotations - -import importlib -import itertools -import math -import os -from types import ModuleType - -import numpy as np -import pytest - -GIB = 1 << 30 - -#: The design's table, restated here from the record (not imported). -TABLE = ((0.0, 637), (1.0, 267), (2.0, 155), (5.0, 65), (10.0, 37), (20.0, 25), (50.0, 19)) - - -@pytest.fixture -def dec() -> ModuleType: - return importlib.import_module("tin_engine.decompose") - - -def cells(p: object) -> tuple[int, int, int, int]: - nx, ny, dx, dy = (getattr(p, k) for k in ("nx", "ny", "dx", "dy")) - for v in (nx, ny, dx, dy): - assert isinstance(v, int) and not isinstance(v, bool), (nx, ny, dx, dy) - return nx, ny, dx, dy - - -# ------------------------------------------------------------------ b(T) - - -class TestBytesPerNode: - @pytest.mark.parametrize(("tolerance", "expected"), TABLE) - def test_the_tables_columns(self, dec: ModuleType, tolerance: float, expected: int) -> None: - assert dec.bytes_per_node(tolerance) == expected - - @pytest.mark.parametrize( - ("tolerance", "expected"), - [(0.5, 452), (0.25, 545), (0.75, 360), (1.5, 211), (7.5, 51)], - ) - def test_linear_between_columns_rounded_up( - self, dec: ModuleType, tolerance: float, expected: int - ) -> None: - """0.5 m gives 452 (the design's own example); 0.25 m is 544.5 exact, - so 545 is the rounding up, not to nearest-even.""" - assert dec.bytes_per_node(tolerance) == expected - - @pytest.mark.parametrize("tolerance", [50.0, 50.5, 100.0, 1e6]) - def test_nineteen_from_fifty_metres_up(self, dec: ModuleType, tolerance: float) -> None: - assert dec.bytes_per_node(tolerance) == 19 - - def test_never_increases_with_the_tolerance(self, dec: ModuleType) -> None: - values = [dec.bytes_per_node(t) for t in np.linspace(0.0, 60.0, 601)] - assert all(a >= b for a, b in itertools.pairwise(values)) - - def test_the_default_budget_is_sixteen_gib(self, dec: ModuleType) -> None: - assert dec.DEFAULT_MEMORY_BUDGET == 16 * GIB == 2**34 - - -# ------------------------------------------------------------------ the figures - - -class TestThePartitionProbesFigures: - """`partition.py`'s rows under "What was measured for this design".""" - - def test_velhas_at_one_metre_with_pieces_64(self, dec: ModuleType) -> None: - assert cells(dec.partition(4208, 7347, 1.0, pieces=64)) == (6, 11, 702, 668) - - def test_velhas_at_one_metre_with_pieces_16_gives_fifteen_cells(self, dec: ModuleType) -> None: - """The count approximates the request and is not a floor: 3 x 5 = 15.""" - nx, ny, _, _ = cells(dec.partition(4208, 7347, 1.0, pieces=16)) - assert (nx, ny) == (3, 5) - assert nx * ny == 15 < 16 - - def test_velhas_is_one_piece_at_half_a_metre_and_up(self, dec: ModuleType) -> None: - for tolerance in (0.5, 1.0, 5.0, 50.0): - assert cells(dec.partition(4208, 7347, tolerance)) == (1, 1, 4208, 7347) - - def test_velhas_at_zero_is_cut_in_two(self, dec: ModuleType) -> None: - assert cells(dec.partition(4208, 7347, 0.0))[:2] == (1, 2) - - def test_the_basin_at_one_metre(self, dec: ModuleType) -> None: - assert cells(dec.partition(41332, 50297, 1.0)) == (5, 7, 8267, 7186) - - def test_the_basin_at_fifty_metres(self, dec: ModuleType) -> None: - assert cells(dec.partition(41332, 50297, 50.0))[:2] == (2, 2) - - -# ------------------------------------------------------------------ the boundary - - -class TestTheBudgetBoundary: - @pytest.mark.parametrize("tolerance", [0.0, 1.0, 10.0, 50.0]) - def test_one_piece_at_n_b_equal_to_the_budget(self, dec: ModuleType, tolerance: float) -> None: - cols, rows = 300, 200 - budget = cols * rows * dec.bytes_per_node(tolerance) - assert cells(dec.partition(cols, rows, tolerance, memory_budget=budget)) == ( - 1, - 1, - cols, - rows, - ) - - @pytest.mark.parametrize("tolerance", [0.0, 1.0, 10.0, 50.0]) - def test_a_cut_at_n_b_one_byte_over_the_budget(self, dec: ModuleType, tolerance: float) -> None: - cols, rows = 300, 200 - b = dec.bytes_per_node(tolerance) - budget = cols * rows * b - 1 - nx, ny, dx, dy = cells(dec.partition(cols, rows, tolerance, memory_budget=budget)) - assert nx * ny > 1 - assert dx * dy * b <= budget - - def test_a_large_budget_gives_one_piece_above_every_former_cap(self, dec: ModuleType) -> None: - """B14: no N_MAX (2**22 nodes) and no per-piece cap; the basin at 0 m - is one piece under a budget that holds it.""" - cols, rows = 41332, 50297 - budget = cols * rows * 637 - assert cols * rows > 2**22 - assert cells(dec.partition(cols, rows, 0.0, memory_budget=budget)) == (1, 1, cols, rows) - - def test_pieces_one_is_no_request(self, dec: ModuleType) -> None: - assert cells(dec.partition(64, 64, 1.0, pieces=1)) == (1, 1, 64, 64) - - def test_a_request_cuts_a_small_window(self, dec: ModuleType) -> None: - """`--pieces` asks for more pieces than the budget needs: 4 on a - square window is 2 x 2.""" - assert cells(dec.partition(41, 41, 1.0, pieces=4)) == (2, 2, 21, 21) - - -class TestRefusals: - @pytest.mark.parametrize("pieces", [0, -1]) - def test_pieces_under_one(self, dec: ModuleType, pieces: int) -> None: - with pytest.raises(ValueError, match="pieces"): - dec.partition(64, 64, 1.0, pieces=pieces) - - @pytest.mark.parametrize( - ("cols", "rows"), - [(0, 64), (64, 0), (0, 0), (-1, 64), (64, -3)], - ids=["no-columns", "no-rows", "neither", "negative-cols", "negative-rows"], - ) - @pytest.mark.parametrize("pieces", [1, 4]) - def test_an_empty_window(self, dec: ModuleType, cols: int, rows: int, pieces: int) -> None: - """Point 4 of "Three points from 23c-1's green (3ffae13)": `cols < 1` - or `rows < 1` is a `ValueError` naming the cause, not a - `ZeroDivisionError` from step 2 nor a partition of nothing.""" - with pytest.raises(ValueError, match=r"cols|rows|window"): - dec.partition(cols, rows, 1.0, pieces=pieces) - - def test_a_budget_under_one_node(self, dec: ModuleType) -> None: - """No cell of one node fits, so step 3 cannot end: refused, not looped.""" - b = dec.bytes_per_node(1.0) - with pytest.raises(ValueError, match="budget"): - dec.partition(64, 64, 1.0, memory_budget=b - 1) - - def test_a_budget_of_one_node_is_every_node_a_cell(self, dec: ModuleType) -> None: - b = dec.bytes_per_node(1.0) - assert cells(dec.partition(5, 4, 1.0, memory_budget=b)) == (5, 4, 1, 1) - - -# ------------------------------------------------------------------ the invariants - - -def random_cases(seed: int, n: int) -> list[tuple[int, int, float, int, int]]: - rng = np.random.default_rng(seed) - out = [] - for _ in range(n): - cols = int(rng.integers(2, 60_000)) - rows = int(rng.integers(2, 60_000)) - tolerance = float(rng.choice([0.0, 0.25, 0.5, 1.0, 2.0, 3.0, 5.0, 10.0, 33.0, 80.0])) - pieces = int(rng.choice([1, 1, 2, 3, 4, 7, 16, 64, 100, 1000])) - # Budgets from a few nodes' worth to far above the window. - budget = int(637 * 2 ** rng.uniform(0, 42)) - out.append((cols, rows, tolerance, pieces, budget)) - return out - - -class TestInvariantsOverRandomWindows: - """For windows, tolerances, `--pieces` and budgets drawn at random: no - cell over the budget, no empty row or column of cells, whole-node sides, - and the count at least what the budget needs.""" - - def test_seeded_draws(self, dec: ModuleType) -> None: - checked = 0 - for cols, rows, tolerance, pieces, budget in random_cases(23, 3000): - b = dec.bytes_per_node(tolerance) - if budget < b: - continue - nx, ny, dx, dy = cells( - dec.partition(cols, rows, tolerance, pieces=pieces, memory_budget=budget) - ) - case = (cols, rows, tolerance, pieces, budget, nx, ny, dx, dy) - assert dx * dy * b <= budget, case - assert 1 <= dx <= cols and 1 <= dy <= rows, case - # No empty column or row of cells: the last starts inside the window. - assert (nx - 1) * dx < cols <= nx * dx, case - assert (ny - 1) * dy < rows <= ny * dy, case - if nx * ny == 1: - assert (dx, dy) == (cols, rows), case - assert max(pieces, math.ceil(cols * rows * b / budget)) <= 1, case - checked += 1 - assert checked > 2500 - - def test_one_piece_exactly_when_neither_the_request_nor_the_budget_asks( - self, dec: ModuleType - ) -> None: - for cols, rows, tolerance, pieces, budget in random_cases(29, 2000): - b = dec.bytes_per_node(tolerance) - if budget < b: - continue - nx, ny, _, _ = cells( - dec.partition(cols, rows, tolerance, pieces=pieces, memory_budget=budget) - ) - wanted = max(pieces, -(-cols * rows * b // budget)) - assert (nx * ny == 1) == (wanted <= 1), (cols, rows, tolerance, pieces, budget) - - -class TestNotTheMachines: - """K5: neither the count nor the cut is the machine's.""" - - @pytest.mark.parametrize( - ("cores", "memory"), [(1, 1 * GIB), (512, 4096 * GIB), (None, 2 * GIB)] - ) - def test_the_same_partition_on_any_machine( - self, dec: ModuleType, monkeypatch: pytest.MonkeyPatch, cores: int | None, memory: int - ) -> None: - cases = [(4208, 7347, 1.0, 64), (41332, 50297, 1.0, 1), (41332, 50297, 0.0, 1)] - before = [cells(dec.partition(c, r, t, pieces=p)) for c, r, t, p in cases] - import tin_engine.mosaic as mosaic - - monkeypatch.setattr(os, "cpu_count", lambda: cores) - monkeypatch.setattr(mosaic, "physical_memory", lambda: memory, raising=False) - if hasattr(os, "sched_getaffinity"): - monkeypatch.setattr(os, "sched_getaffinity", lambda pid: set(range(cores or 1))) - after = [cells(dec.partition(c, r, t, pieces=p)) for c, r, t, p in cases] - assert after == before diff --git a/tests/python/test_dem_input.py b/tests/python/test_dem_input.py index 295ae607..12456e9a 100644 --- a/tests/python/test_dem_input.py +++ b/tests/python/test_dem_input.py @@ -155,15 +155,19 @@ def test_a_caller_nodata_applies_to_every_tile(self, di: ModuleType, mosaic_dir: assert (opened.tile.meta.nodata, opened.tile.meta.nodata_source) == (-9999.0, "caller") def test_the_cap_refuses_before_any_pixel_is_read( - self, di: ModuleType, mz: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + self, di: ModuleType, mz: ModuleType, tmp_path: Path ) -> None: """M5 through a file: its strip is cut, so a load would raise - `GeoTiffError`; the refusal must be the cap's `MosaicError` instead.""" + `GeoTiffError`; the refusal must be the cap's `MosaicError` instead. + The cap is increment 38's budget, through `open_dem(..., resources=)`: + 8 bytes, which 3 x 4 float32 nodes exceed.""" + from tin_engine.resources import Resources + path = tmp_path / "cut.tif" path.write_bytes(truncated_strip().getvalue()) - monkeypatch.setattr(mz, "physical_memory", lambda: 8) # cap 4 bytes: 3 x 4 nodes exceed it - with pytest.raises(mz.MosaicError, match="--bbox"): - di.open_dem(request(di, path)) + with pytest.raises(mz.MosaicError, match="--bbox") as info: + di.open_dem(request(di, path), resources=Resources(threads=1, memory_budget=8)) + assert "--memory-budget" in str(info.value) class TestDemRequest: diff --git a/tests/python/test_io_cog.py b/tests/python/test_io_cog.py index db3f6fb0..a37a42a7 100644 --- a/tests/python/test_io_cog.py +++ b/tests/python/test_io_cog.py @@ -21,7 +21,6 @@ import importlib import io -import os import re from pathlib import Path from types import ModuleType @@ -254,8 +253,8 @@ def test_one_thread_and_eight_give_the_same_bytes( def test_p2_the_default_thread_count_changes_no_byte( self, cog: ModuleType, geotiff: ModuleType, window: Any, variant: Variant ) -> None: - """30c P2: a call without `threads` (the machine's cores since 30c, - 4 before) gives `threads=1`'s bytes.""" + """30c P2: a call without `threads` (one worker since increment 38, + the machine's cores from 30c, 4 before) gives `threads=1`'s bytes.""" data = build(variant) w = window(row0=5, col0=9, rows=40, cols=55) one = decoded(cog, geotiff, BytesBlocks(data), data, w, threads=1) @@ -263,28 +262,27 @@ def test_p2_the_default_thread_count_changes_no_byte( assert np.asarray(default.array).tobytes() == np.asarray(one.array).tobytes() -class TestR1DefaultThreadsAreTheMachines: - """30c R1 (`docs/increments/30c-dem-read-speed.md`, section 3.1): without - `threads`, `decode_window` builds its pool with `os.cpu_count() or 1` - workers; a given `threads` is used as given. The pool is observed through - a spy in place of `cog.ThreadPoolExecutor` that records the worker count - and builds the real pool, so the decode itself still runs.""" +class TestR1DefaultThreadsIsOne: + """Increment 38, section 4.2 (replacing 30c R1): without `threads`, + `decode_window` builds its pool with one worker, the library default; no + module but `resources.py` reads the core count. A given `threads` is used + as given. The pool is observed through a spy in place of + `cog.ThreadPoolExecutor` that records the worker count and builds the real + pool, so the decode itself still runs.""" @pytest.mark.parametrize( - ("cores", "kwargs", "workers"), + ("kwargs", "workers"), [ - pytest.param(7, {}, 7, id="seven_cores"), - pytest.param(None, {}, 1, id="cores_unknown"), - pytest.param(7, {"threads": 9}, 9, id="threads_given"), + pytest.param({}, 1, id="default_is_one"), + pytest.param({"threads": 9}, 9, id="threads_given"), ], ) - def test_the_pool_is_built_with_the_machines_core_count( + def test_the_pool_is_built_with_the_count_given_or_one( self, cog: ModuleType, geotiff: ModuleType, window: Any, monkeypatch: pytest.MonkeyPatch, - cores: int | None, kwargs: dict[str, int], workers: int, ) -> None: @@ -296,7 +294,6 @@ def spy(*args: Any, **kw: Any) -> Any: return real(*args, **kw) monkeypatch.setattr(cog, "ThreadPoolExecutor", spy) - monkeypatch.setattr(os, "cpu_count", lambda: cores) data = build(VARIANTS[0]) w = window(row0=5, col0=9, rows=40, cols=55) tile = decoded(cog, geotiff, BytesBlocks(data), data, w, **kwargs) diff --git a/tests/python/test_layering.py b/tests/python/test_layering.py index 30a435cf..450ced00 100644 --- a/tests/python/test_layering.py +++ b/tests/python/test_layering.py @@ -41,8 +41,9 @@ "grid_domain": "io.models", "domain": "crs io.models", "chains": "domain features", - "elevation": "", "outline": "", "sensitivity": "", "landcover": "", "decompose": "", + "elevation": "", "outline": "", "sensitivity": "", "landcover": "", "grow": "", # 30d: the outline grown, shapely and NumPy only + "resources": "", # 38: the one module that reads the machine; os and Pydantic only "burn": "gauge io.models", "gauge": "hydrography", "reference": "io.models", diff --git a/tests/python/test_mcp_schema.py b/tests/python/test_mcp_schema.py index 7be92b2b..d4c4ea8c 100644 --- a/tests/python/test_mcp_schema.py +++ b/tests/python/test_mcp_schema.py @@ -317,6 +317,26 @@ def test_an_integer_range_carries_its_minimum(self) -> None: "type": "integer", "minimum": 1, "default": 8, } # fmt: skip + def test_t9_threads_and_memory_budget_derive_from_the_cli(self) -> None: + """Increment 38, T9: the two options reach the `mesh`, `catchment` and + `station_catchments` tools through the derived schema, with no change + to `mcp/`; `argv` puts them on the command line as the CLI's words.""" + tools = derived(app) + for name in ("mesh", "catchment", "station_catchments"): + properties = tools[name].tool().input_schema["properties"] + threads, budget = properties["threads"], properties["memory_budget"] + assert threads["default"] is None, name + assert {"type": "integer", "minimum": 0} in threads["anyOf"], name + assert {"type": "null"} in threads["anyOf"], name + assert budget["default"] is None and {"type": "string"} in budget["anyOf"], name + mesh = tools["mesh"] + with_threads = mesh.model.model_validate({"out": "x.vtk", "threads": 3}) + assert mesh.argv(with_threads)[-2:] == ["--threads", "3"] + with_budget = mesh.model.model_validate({"out": "x.vtk", "memory_budget": "8G"}) + assert mesh.argv(with_budget)[-2:] == ["--memory-budget", "8G"] + with pytest.raises(ValidationError): + mesh.model.model_validate({"out": "x.vtk", "threads": -1}) + def test_the_whole_tool_list_stays_small(self) -> None: # 16,575 bytes measured (section 3); the bound leaves room for help text to grow. size = sum(len(json.dumps(d.tool().input_schema)) for d in mcp_schema.derive(app)) diff --git a/tests/python/test_mosaic.py b/tests/python/test_mosaic.py index e3a4fb45..c1f112f8 100644 --- a/tests/python/test_mosaic.py +++ b/tests/python/test_mosaic.py @@ -88,13 +88,18 @@ def footprint() -> Any: @pytest.fixture def plan(mz: ModuleType, footprint: Any) -> Any: - """`plan(tiles, box=None, needed=None)`: plan a dict of tiles; `box` is a 4-tuple.""" + """`plan(tiles, box=None, needed=None, memory_budget=None)`: plan a dict of + tiles; `box` is a 4-tuple; the budget is passed only when given (38).""" def run( - tiles: dict[str, DemTile], box: tuple[float, ...] | None = None, needed: Any = None + tiles: dict[str, DemTile], + box: tuple[float, ...] | None = None, + needed: Any = None, + memory_budget: int | None = None, ) -> Any: bounds = None if box is None else bounds_of(mz, box) - return mz.plan_mosaic(footprints(footprint, tiles), bounds, needed) + extra = {} if memory_budget is None else {"memory_budget": memory_budget} + return mz.plan_mosaic(footprints(footprint, tiles), bounds, needed, **extra) return run @@ -655,51 +660,43 @@ def test_a_box_leaving_one_column_at_the_edge_is_refused( class TestM5MemoryCap: - """M5, changed for 15a (R7): the cap is half of physical memory, from a function.""" + """M5, changed for 15a (R7) and again for increment 38 (section 4.3): the + cap is the memory budget, `plan_mosaic(..., memory_budget=)`, None being + half the machine's memory (`Resources.fallback()`; the machine read is + `test_resources.py`'s). The refusal names `--memory-budget` and `--bbox`.""" NODES = 9 * 13 - def test_physical_memory_is_the_machines(self, mz: ModuleType) -> None: - import os - - pages = os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") - assert mz.physical_memory() == pages - - @pytest.mark.parametrize( - ("memory", "accepted"), [(2 * NODES * 4, True), (2 * NODES * 4 - 1, False)] - ) + @pytest.mark.parametrize(("budget", "accepted"), [(NODES * 4, True), (NODES * 4 - 1, False)]) def test_the_boundary_at_four_bytes_a_node( - self, - mz: ModuleType, - plan: Any, - refused: Any, - monkeypatch: pytest.MonkeyPatch, - memory: int, - accepted: bool, + self, mz: ModuleType, plan: Any, refused: Any, budget: int, accepted: bool ) -> None: - monkeypatch.setattr(mz, "physical_memory", lambda: memory) tiles = quadrants(whole(), row_cut=4, col_cut=6, overlap=1) if accepted: - assert plan(tiles).meta.rows == 9 + assert plan(tiles, memory_budget=budget).meta.rows == 9 else: - refused(lambda: plan(tiles), "--bbox") + refused(lambda: plan(tiles, memory_budget=budget), "--bbox", "--memory-budget") - @pytest.mark.parametrize( - ("memory", "accepted"), [(2 * NODES * 8, True), (2 * NODES * 8 - 1, False)] - ) + def test_one_byte_refuses_the_lattice_naming_the_option_and_the_figures( + self, plan: Any, refused: Any + ) -> None: + tiles = quadrants(whole(), row_cut=4, col_cut=6, overlap=1) + message = refused(lambda: plan(tiles, memory_budget=1), "--memory-budget", "9 x 13") + names_number(message, self.NODES * 4) + names_number(message, 1) + + def test_without_a_budget_the_lattice_is_planned(self, plan: Any) -> None: + """None is the fallback, half the machine's memory: 468 bytes pass.""" + tiles = quadrants(whole(), row_cut=4, col_cut=6, overlap=1) + assert plan(tiles).meta.rows == 9 + + @pytest.mark.parametrize(("budget", "accepted"), [(NODES * 8, True), (NODES * 8 - 1, False)]) def test_the_boundary_at_eight_bytes_a_node_when_a_selected_tile_decodes_to_float64( - self, - mz: ModuleType, - footprint: Any, - refused: Any, - monkeypatch: pytest.MonkeyPatch, - memory: int, - accepted: bool, + self, mz: ModuleType, footprint: Any, refused: Any, budget: int, accepted: bool ) -> None: """S2: the cap counts the planned decoded dtype, `np.result_type` of the selected footprints' `dtype`. One float64 tile makes the canvas float64 (R5), so the window costs 8 bytes a node, not 4.""" - monkeypatch.setattr(mz, "physical_memory", lambda: memory) tiles = quadrants(whole(), row_cut=4, col_cut=6, overlap=1) prints = [ footprint(name=name, meta=tile.meta, dtype=np.dtype(np.float64)) @@ -707,17 +704,16 @@ def test_the_boundary_at_eight_bytes_a_node_when_a_selected_tile_decodes_to_floa else footprint(name=name, meta=tile.meta) for name, tile in tiles.items() ] - call = lambda: mz.plan_mosaic(prints, None, None) # noqa: E731 + call = lambda: mz.plan_mosaic(prints, None, None, memory_budget=budget) # noqa: E731 if accepted: assert call().meta.rows == 9 else: refused(call, "--bbox", "float64") def test_a_float64_tile_outside_the_window_does_not_count( - self, mz: ModuleType, footprint: Any, monkeypatch: pytest.MonkeyPatch + self, mz: ModuleType, footprint: Any ) -> None: """Only the selected tiles decode, so only their dtypes size the canvas.""" - monkeypatch.setattr(mz, "physical_memory", lambda: 2 * 16 * 4) tiles = quadrants(whole(), row_cut=4, col_cut=6, overlap=1) prints = [ footprint(name=name, meta=tile.meta, dtype=np.dtype(np.float64)) @@ -726,7 +722,8 @@ def test_a_float64_tile_outside_the_window_does_not_count( for name, tile in tiles.items() ] box = bounds_of(mz, (500012.0, 6599987.0, 500033.0, 6599996.0)) # 4 x 4, nw alone - assert [p.name for p in mz.plan_mosaic(prints, box, None).tiles] == ["nw.tif"] + planned = mz.plan_mosaic(prints, box, None, memory_budget=16 * 4) + assert [p.name for p in planned.tiles] == ["nw.tif"] def test_a_footprints_dtype_defaults_to_float32(self, footprint: Any) -> None: """S2: `TileFootprint` gains an optional decoded `dtype`, float32 unless given.""" @@ -735,12 +732,10 @@ def test_a_footprints_dtype_defaults_to_float32(self, footprint: Any) -> None: given = footprint(name="a.tif", meta=tile.meta, dtype=np.dtype(np.float64)) assert np.dtype(given.dtype) == np.float64 - def test_the_cap_is_on_the_window_not_the_union( - self, mz: ModuleType, plan: Any, monkeypatch: pytest.MonkeyPatch - ) -> None: - monkeypatch.setattr(mz, "physical_memory", lambda: 2 * 16 * 4) + def test_the_cap_is_on_the_window_not_the_union(self, plan: Any) -> None: tiles = quadrants(whole(), row_cut=4, col_cut=6, overlap=1) - assert plan(tiles, (500012.0, 6599987.0, 500033.0, 6599996.0)).meta.cols == 4 + box = (500012.0, 6599987.0, 500033.0, 6599996.0) + assert plan(tiles, box, memory_budget=16 * 4).meta.cols == 4 class TestM6PlanOrderIndependence: diff --git a/tests/python/test_resources.py b/tests/python/test_resources.py new file mode 100644 index 00000000..bd13cdd3 --- /dev/null +++ b/tests/python/test_resources.py @@ -0,0 +1,288 @@ +"""`tin_engine.resources`: the one module that reads the machine (increment 38). + +`docs/increments/38-threads-and-memory.md`, sections 4.1 to 4.3, 6 and 8 +(T1 to T5, and T11's `decompose`). The module is fetched inside a fixture, so +its absence fails these tests and leaves the rest of the session collecting. + +Figures that depend on the machine (its core count, its memory) are computed +from the module's own readers with no cgroup, never written as literals: the +design's "10 cores, 32 GiB" are this machine's and the suite runs on others. +The cgroup readers take the cgroup root, so a `tmp_path` with a synthetic +`cpu.max` or `memory.max` stands for a container; the scale assumed is one +container with its own cgroup root (the design's stated limit). + +T11's `project_structure.md` assertion is not here: that file is prose under +`tools/ci_changes.py`'s rule, and `conftest.py` fails a session in which a +test opens one (h11, T5). `@reviewer` reads the two rows instead. +""" + +from __future__ import annotations + +import importlib +import importlib.util +import os +from pathlib import Path +from types import ModuleType + +import pytest +from pydantic import ValidationError + +import tin_engine + +PACKAGE = Path(tin_engine.__file__).resolve().parent + +#: The tokens by which Python reads the machine; `resources.py` alone may use them. +MACHINE_TOKENS = ("cpu_count", "sched_getaffinity", "sysconf", "/sys/fs/cgroup") + +GIB = 1 << 30 +#: cgroup v1's "no limit": PAGE_COUNTER_MAX, about 2^63, the figure a host prints. +V1_UNLIMITED = "9223372036854771712" + + +@pytest.fixture(scope="module") +def res() -> ModuleType: + return importlib.import_module("tin_engine.resources") + + +@pytest.fixture +def no_cgroup(tmp_path: Path) -> Path: + """A cgroup root with no files: the bare host.""" + root = tmp_path / "empty" + root.mkdir() + return root + + +def physical_memory() -> int: + return os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") + + +# ---------------------------------------------------------------- T1 + + +def test_t1_only_resources_reads_the_machine() -> None: + """grep-equivalent over the package: each token occurs in `resources.py` only.""" + readers = { + str(path.relative_to(PACKAGE)) + for path in PACKAGE.rglob("*.py") + if any(token in path.read_text(encoding="utf-8") for token in MACHINE_TOKENS) + } + assert readers == {"resources.py"} + + +# ---------------------------------------------------------------- the model + + +class TestResourcesModel: + def test_frozen_and_never_zero(self, res: ModuleType) -> None: + r = res.Resources(threads=3, memory_budget=4096) + assert (r.threads, r.memory_budget) == (3, 4096) + with pytest.raises(ValidationError): + r.threads = 4 + for bad in ({"threads": 0, "memory_budget": 1}, {"threads": 1, "memory_budget": 0}): + with pytest.raises(ValidationError): + res.Resources(**bad) + + def test_fallback_is_one_thread_and_half_the_memory(self, res: ModuleType) -> None: + fallback = res.Resources.fallback() + assert fallback.threads == 1 + assert fallback.memory_budget == res.machine_memory() // 2 + + +# ---------------------------------------------------------------- T2 + + +class TestT2TheThreadsRule: + """Section 4.2's table, one case each; `machine_cores()` is the machine's.""" + + @pytest.mark.parametrize( + ("threads", "environ"), + [ + pytest.param(None, {}, id="nothing given"), + pytest.param(0, {}, id="--threads 0"), + pytest.param(None, {"RASPUTIN_THREADS": "0"}, id="RASPUTIN_THREADS=0"), + pytest.param(None, {"RASPUTIN_THREADS": ""}, id="RASPUTIN_THREADS empty"), + pytest.param(0, {"RASPUTIN_THREADS": "7"}, id="--threads 0 beats the variable"), + ], + ) + def test_every_core( + self, res: ModuleType, threads: int | None, environ: dict[str, str] + ) -> None: + assert res.resources(threads, None, environ).threads == res.machine_cores() + + @pytest.mark.parametrize( + ("threads", "environ", "expected"), + [ + pytest.param(3, {"RASPUTIN_THREADS": "7"}, 3, id="the option beats the variable"), + pytest.param(None, {"RASPUTIN_THREADS": "7"}, 7, id="the variable alone"), + pytest.param(None, {"RASPUTIN_THREADS": " 4 "}, 4, id="the variable is stripped"), + pytest.param(64, {}, 64, id="above the core count, honoured"), + pytest.param(None, {"RASPUTIN_THREADS": "64"}, 64, id="above the cores, by variable"), + ], + ) + def test_a_count( + self, res: ModuleType, threads: int | None, environ: dict[str, str], expected: int + ) -> None: + assert res.resources(threads, None, environ).threads == expected + + @pytest.mark.parametrize("text", ["abc", "-1", "1.5", "0x10"]) + def test_a_variable_that_is_not_a_whole_number_is_refused( + self, res: ModuleType, text: str + ) -> None: + with pytest.raises(ValueError, match="RASPUTIN_THREADS") as info: + res.resources(None, None, {"RASPUTIN_THREADS": text}) + assert text in str(info.value) + assert "0 means every core" in str(info.value) + + +# ---------------------------------------------------------------- T3 + + +class TestT3MachineCores: + """`machine_cores(cgroup)`: the process's count, capped by a cgroup v2 CPU + quota `ceil(MAX / PERIOD)`; anything that is not two positive whole numbers + is no quota.""" + + def test_the_plain_count_is_the_processs(self, res: ModuleType, no_cgroup: Path) -> None: + expected = getattr(os, "process_cpu_count", os.cpu_count)() or 1 + assert res.machine_cores(no_cgroup) == expected + assert res.machine_cores() >= 1 + + def test_a_quota_of_two_cpus_caps_at_two(self, res: ModuleType, tmp_path: Path) -> None: + (tmp_path / "cpu.max").write_text("200000 100000\n") + assert res.machine_cores(tmp_path) == min(2, res.machine_cores(tmp_path / "none")) + + def test_half_a_cpu_rounds_up_to_one(self, res: ModuleType, tmp_path: Path) -> None: + (tmp_path / "cpu.max").write_text("50000 100000\n") + assert res.machine_cores(tmp_path) == 1 + + def test_one_and_a_half_cpus_round_up_to_two(self, res: ModuleType, tmp_path: Path) -> None: + (tmp_path / "cpu.max").write_text("150000 100000\n") + assert res.machine_cores(tmp_path) == min(2, res.machine_cores(tmp_path / "none")) + + @pytest.mark.parametrize( + "text", + [ + pytest.param("max 100000\n", id="no quota"), + pytest.param("abc 100000\n", id="first word not a number"), + pytest.param("200000 0\n", id="zero period"), + pytest.param("200000\n", id="one word"), + pytest.param("200000 abc\n", id="period not a number"), + pytest.param("", id="empty file"), + pytest.param("-200000 100000\n", id="negative quota"), + ], + ) + def test_no_quota_gives_the_plain_count( + self, res: ModuleType, tmp_path: Path, no_cgroup: Path, text: str + ) -> None: + (tmp_path / "cpu.max").write_text(text) + assert res.machine_cores(tmp_path) == res.machine_cores(no_cgroup) + + @pytest.mark.skipif(os.geteuid() == 0, reason="root reads every file") + def test_an_unreadable_file_is_no_quota( + self, res: ModuleType, tmp_path: Path, no_cgroup: Path + ) -> None: + path = tmp_path / "cpu.max" + path.write_text("200000 100000\n") + path.chmod(0) + try: + assert res.machine_cores(tmp_path) == res.machine_cores(no_cgroup) + finally: + path.chmod(0o600) + + +# ---------------------------------------------------------------- T4 + + +class TestT4MachineMemory: + """`machine_memory(cgroup)`: physical memory, capped by `memory.max` (v2) + or `memory/memory.limit_in_bytes` (v1). Scale: the limits below are 1 GiB + and 512 MiB, under any machine this suite runs on (asserted).""" + + def test_the_plain_figure_is_physical_memory(self, res: ModuleType, no_cgroup: Path) -> None: + assert res.machine_memory(no_cgroup) == physical_memory() + assert res.machine_memory() >= 1 + + def test_a_v2_limit_under_the_machine_wins(self, res: ModuleType, tmp_path: Path) -> None: + assert physical_memory() > GIB, "the fixture assumes a machine over 1 GiB" + (tmp_path / "memory.max").write_text(f"{GIB}\n") + assert res.machine_memory(tmp_path) == GIB + + def test_a_v1_limit_under_the_machine_wins(self, res: ModuleType, tmp_path: Path) -> None: + (tmp_path / "memory").mkdir() + (tmp_path / "memory" / "memory.limit_in_bytes").write_text(f"{GIB // 2}\n") + assert res.machine_memory(tmp_path) == GIB // 2 + + def test_v1s_no_limit_value_loses_to_the_physical_figure( + self, res: ModuleType, tmp_path: Path + ) -> None: + (tmp_path / "memory").mkdir() + (tmp_path / "memory" / "memory.limit_in_bytes").write_text(f"{V1_UNLIMITED}\n") + assert res.machine_memory(tmp_path) == physical_memory() + + def test_v2_wins_when_both_exist(self, res: ModuleType, tmp_path: Path) -> None: + (tmp_path / "memory.max").write_text(f"{GIB}\n") + (tmp_path / "memory").mkdir() + (tmp_path / "memory" / "memory.limit_in_bytes").write_text(f"{GIB // 2}\n") + assert res.machine_memory(tmp_path) == GIB + + @pytest.mark.parametrize( + "text", + [ + pytest.param("max\n", id="max"), + pytest.param("0\n", id="zero"), + pytest.param("-1\n", id="negative"), + pytest.param("abc\n", id="text"), + pytest.param("", id="empty file"), + ], + ) + def test_no_limit_gives_the_physical_figure( + self, res: ModuleType, tmp_path: Path, text: str + ) -> None: + (tmp_path / "memory.max").write_text(text) + assert res.machine_memory(tmp_path) == physical_memory() + + +# ---------------------------------------------------------------- T5 + + +class TestT5ParseBytes: + @pytest.mark.parametrize( + ("text", "expected"), + [ + ("8G", 8 * GIB), + ("512m", 512 << 20), + ("1T", 1 << 40), + ("4096", 4096), + ("2k", 2048), + ], + ) + def test_a_whole_number_with_an_optional_1024_suffix( + self, res: ModuleType, text: str, expected: int + ) -> None: + assert res.parse_bytes(text) == expected + + @pytest.mark.parametrize("text", ["0", "8GB", "-1", "abc", "", "1.5G", "G"]) + def test_anything_else_is_refused_naming_the_text(self, res: ModuleType, text: str) -> None: + with pytest.raises(ValueError) as info: + res.parse_bytes(text) + assert (text or repr(text)) in str(info.value) + + def test_the_default_budget_is_half_the_machines_memory(self, res: ModuleType) -> None: + assert res.resources(None, None, {}).memory_budget == res.machine_memory() // 2 + + def test_a_given_budget_is_parsed(self, res: ModuleType) -> None: + assert res.resources(None, "8G", {}).memory_budget == 8 * GIB + assert res.resources(None, "4096", {}).memory_budget == 4096 + + @pytest.mark.parametrize("text", ["0", "8GB", "-1", "abc"]) + def test_a_bad_budget_is_refused_naming_the_text(self, res: ModuleType, text: str) -> None: + with pytest.raises(ValueError) as info: + res.resources(None, text, {}) + assert text in str(info.value) + + +# ---------------------------------------------------------------- T11 + + +def test_t11_decompose_is_gone() -> None: + assert importlib.util.find_spec("tin_engine.decompose") is None diff --git a/tests/python/test_stats.py b/tests/python/test_stats.py index fc7beaea..694da10e 100644 --- a/tests/python/test_stats.py +++ b/tests/python/test_stats.py @@ -34,9 +34,11 @@ is omitted (R4). Spacing prints once when dx == dy, else ``dx × dy``. File sizes are decimal: B, kB, MB, GB, one decimal above bytes. - ``Report(command, sizes, quality, phases, total, stats_seconds, - bounds_checks, threads=None, seams=())``. ``threads`` is the design's "Threads: 10 (hardware - concurrency)" sentence, which R1's list of Report fields does not carry; - printed only when not None. ``bounds_checks`` (increment 24, required) is the + bounds_checks, threads=None, seams=())``. ``threads`` is the count the run + used, printed as "Threads used: 10." (increment 38, Ola's ruling 4; it was + "Threads: 10 (hardware concurrency)", which named the machine, not the + run), which R1's list of Report fields does not carry; printed only when + not None. ``bounds_checks`` (increment 24, required) is the text after ``bounds checks: `` -- ``off`` or ``on ()`` -- and prints as its own paragraph between the command line and ``## Sizes``. A phase named ``": ..."`` where ```` is an earlier phase is a sub-row: listed, not added to the @@ -243,7 +245,7 @@ def test_the_default_source_is_monotonic_and_non_negative(self, stats: ModuleTyp Wall clock, `time.perf_counter_ns` (Python) and `std::chrono::steady_clock` (inside `refine`), one run, no warm-up. Total is the `mesh` command body, from argument checks to the last file written; interpreter start-up and imports are -not in it. Threads: 10 (hardware concurrency). +not in it. Threads used: 10. | phase | seconds | share | |---|---|---| @@ -367,7 +369,7 @@ def test_rows_that_do_not_apply_are_omitted( "domain vertices", "start vertices", "without data dropped", - "Threads:", + "Threads", "Sub-rows", ): assert absent not in text, absent @@ -376,6 +378,14 @@ def test_rows_that_do_not_apply_are_omitted( assert "| c.ply | 2.0 kB |" in text assert "| c-edges.ply | 999 B |" in text + def test_t12_the_threads_row_is_the_count_used(self, stats: ModuleType, quarter: Any) -> None: + """Increment 38, T12: "Threads used: N." and never the machine's figure.""" + import dataclasses + + text = stats.render(dataclasses.replace(quarter, threads=3)) + assert "Threads used: 3." in text + assert "hardware concurrency" not in text and "Threads: " not in text + def test_other_is_total_minus_the_top_level_rows( self, stats: ModuleType, fixture_report: Any ) -> None: From 3b709b6f0c7c76cad6392a3d269a5b18e195f477 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 19:13:23 +0200 Subject: [PATCH 06/13] 38 red recorded in the Status line; the citation into the deleted decompose test pinned at 5d2e706d, by the main session Co-Authored-By: Claude Fable 5.1 --- docs/increments/23-basin-scale.md | 2 +- docs/increments/38-threads-and-memory.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/increments/23-basin-scale.md b/docs/increments/23-basin-scale.md index 126b8940..186b07ef 100644 --- a/docs/increments/23-basin-scale.md +++ b/docs/increments/23-basin-scale.md @@ -3270,7 +3270,7 @@ LOC was 598 added and 6 removed, 592 net, against an estimate of 373. That is +6 **23b, code review, round 8, 2026-10-04.** Range `4541e38..0a3187b` plus `4541e38` (4541e38 round 7 recorded and `20-start-quality.md` states the 23b rule; 0a3187b @perf's acceptance at the merged head). Verdict: CHANGES REQUESTED, closed in the commit that records this round. LOC: 0 this round (no production file in the range), 249 net for the PR against about 285. Round 7's blocker is closed: `20-start-quality.md:608-610` now states the rule, which matches `include/terrain/mesh/quality.hpp@ed125121:64-65,143,182` and `include/terrain/refinement/refine.hpp@ed125121:321-322`. @perf's verdict at `4541e38` against master `d20126b` is ACCEPTED (`docs/benchmarks/2026-10-04/23b-merged-acceptance.md`). The pooled medians, run ranges and changes were recomputed from the eight `raw.tsv` files and match: +0.8 % tile and +0.2 % quarter at 1 thread, -1.7 to +1.7 % over 2 to 20 threads. Each run's commit, module hash, `bench.py` version and AC power state match the doc, and the mesh hashes and quality are identical in all eight runs. `check_citations.py` passes. Blocking: the status line (`23-basin-scale.md:9-10`) and ROADMAP row 23 still said the merged-head acceptance was "owed"; both now say it is recorded. Merge-ready waits on CI: #162's remote head is still `4cd28dc`, which has never had a CI run; not pushed. -**23c-1, code review, round 1, 2026-10-04.** Range `aacf37c..1426245` (red 58fb1f1, 6dd4ffe, 714b0b3; rulings 6ca9a8c; green 3ffae13; rulings 6ce8e57; red b9f154a; green 1426245), stacked on 23b. Verdict: CHANGES REQUESTED. LOC: 154 net (`bindings/core.cpp` 30, `_core.pyi` 3, `decompose.py` 53, `features.py` 3, `io/mesh_index.py` 65) against about 215. No @perf run needed: nothing under `include/` changed, the binding only copies. The loop-bound comment's argument is sound; the binding's GIL and refusals match `refine_points`'s (stricter on negative indices); `project_structure.md` and docstrings match the code. Blocking: (1) "HOW THIS FILE GOES RED" paragraphs in `test_core_mesh_arrays.py:22-23`, `test_decompose_partition.py:27-28`, `test_mesh_index_conformity.py:21-22`; (2) ROADMAP row 23 (still "23c … 430", "23b … in review") and this file's status paragraph ("23b onwards not implemented"). Suggestions: say "integer indices" in `indexed_mesh`'s docstring; a ValueError for `rows == 0` in `partition()`; plain imports for the `dec`/`mi` fixtures. Whichever of 15f-3 and 23c-1 merges second renumbers `tests/python/test_features.py@1426245:583`'s citation. Not pushed; no CI. +**23c-1, code review, round 1, 2026-10-04.** Range `aacf37c..1426245` (red 58fb1f1, 6dd4ffe, 714b0b3; rulings 6ca9a8c; green 3ffae13; rulings 6ce8e57; red b9f154a; green 1426245), stacked on 23b. Verdict: CHANGES REQUESTED. LOC: 154 net (`bindings/core.cpp` 30, `_core.pyi` 3, `decompose.py` 53, `features.py` 3, `io/mesh_index.py` 65) against about 215. No @perf run needed: nothing under `include/` changed, the binding only copies. The loop-bound comment's argument is sound; the binding's GIL and refusals match `refine_points`'s (stricter on negative indices); `project_structure.md` and docstrings match the code. Blocking: (1) "HOW THIS FILE GOES RED" paragraphs in `test_core_mesh_arrays.py:22-23`, `tests/python/test_decompose_partition.py@5d2e706d:27-28`, `test_mesh_index_conformity.py:21-22`; (2) ROADMAP row 23 (still "23c … 430", "23b … in review") and this file's status paragraph ("23b onwards not implemented"). Suggestions: say "integer indices" in `indexed_mesh`'s docstring; a ValueError for `rows == 0` in `partition()`; plain imports for the `dec`/`mi` fixtures. Whichever of 15f-3 and 23c-1 merges second renumbers `tests/python/test_features.py@1426245:583`'s citation. Not pushed; no CI. **23c-1, code review, round 2, 2026-10-04.** Range `5216cb3..0679950` (c4ec906 test headers, 9633282 status paragraph, ROADMAP row 23 and point 4, 7e5fe0a and e5881a5 the empty-window tests, 0679950 the refusal); whole PR `aacf37c..0679950`. Verdict: APPROVED. LOC: 156 net against about 215. Both blockers closed; the empty-window suggestion taken (point 4, ten cases); citations hold; no @perf run needed. Nit: the status paragraph and ROADMAP say 154 lines built; update to 156 at merge. Not pushed; no CI. diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index 7c16a428..4fc9d585 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed and review-approved after two rounds; next `@tester`'s red suite (section 8);** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); next `@developer`'s green step;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds From b252a70dd212ba0e633e0153767561d5aba6ea34 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 19:46:32 +0200 Subject: [PATCH 07/13] Increment 38 green: threads and memory as settings (@developer) The minimal code for docs/increments/38-threads-and-memory.md, sections 4 to 6 and Ola's four rulings (section 10), against @tester's red commit 4b9c0dfe. No test file is touched. New: src_python/tin_engine/resources.py, the one module that reads the machine. Resources (frozen; threads >= 1, memory_budget >= 1; fallback() is one thread and half the machine's memory); machine_cores() (the process's count, capped by a cgroup v2 cpu.max quota of two positive whole numbers, ceil(MAX / PERIOD); anything else, a negative quota included, is no quota); machine_memory() (physical memory capped by memory.max, else v1 memory/memory.limit_in_bytes; "max", 0, -1, text, an empty or unreadable file are no limit); parse_bytes() (a whole number with an optional K, M, G or T, 1024-based; 0, "8GB", "1.5G", "G", "" refused naming the text); resources() (section 4.2's table: --threads, else RASPUTIN_THREADS stripped and whole, 0 = every core; --memory-budget, else half the memory). Where the value travels: cli.py gets two Annotated aliases THREADS (min=0, metavar N: the default "" widened Rich's metavar column and truncated the longest option names in --help) and MEMORY_BUDGET on mesh, catchment and station-catchments, resolves them once per command (_resources; a refusal is a usage error naming RASPUTIN_THREADS or --memory-budget), and passes the count to the five _core sites (refine, SlopeTolerance, edge_strip.run -> refine_strip, final_check.run -> refine_points, assign -> class_areas) and the three pools: repository_for and both repositories keep `threads` for the block decoder; open_dem(request, resources) plans every mosaic under the budget and resamples and checks on the count; the reprojector inside target_grid's workers takes threads=1; FeatureRequest.threads reaches _Tally's reprojector; index_space's reprojector takes assign's count. The library defaults are one worker (reprojector, decode_window, decode_class_window, assign) and the C++'s 0 for the two _core wrappers. crs.WORKERS and mosaic.physical_memory are gone; the chunked reprojector takes min(threads, chunks) workers and opens no pool for threads=1. The budget: plan_mosaic(..., memory_budget=None) caps the canvas (None is Resources.fallback()'s); delineate(request, repository, resources=None) passes it to _grow and _plan; both refusals read "over the memory budget of B bytes (set with --memory-budget; half the machine's memory when it is not set)". run_batch(..., resources=) hands it to each delineate. dem_input._domain_plan's budget defaults to None (test_cli_fetch calls it with two arguments). The report: stats.render prints "Threads used: N."; the record gains the Inputs entry threads ("Threads used"), set on every DEM run; a fixture run has neither. decompose.py is deleted; mesh_index's docstring and project_structure.md's rows follow; README and INSTALL name RASPUTIN_THREADS and --memory-budget. Beyond the design, because a test pins it: mcp/schema.py keeps a constraint of 0 (`minimum: 0` for --threads; `if v` dropped it), one line, where section 4.5 said no mcp/ change. Not green, for @tester (the full suite: 7402 passed, 21 failed before the help and _domain_plan fixes above, which clear 4; the 17 left are tests that pin the old surface or stand in for a function with a narrower signature than the design gives it): test_layering's rows for cli, dem_input, catchment, catchment_batch and mosaic lack `resources`, which section 4.4 has each import (5 tests); test_cli_mesh_domain_crs's as_16_opened_it(request) stands in for open_dem(request, resources) with one parameter (2); test_cli_mesh_plain_output wraps cli.refine with `real(*a, **k, threads=1)`, which the CLI's own threads= keyword now collides with (--threads 1 is the way since 38), and asserts "threads" is not a record key, against T6 and section 4.4 (2); test_mosaic's test_the_plan_never_loads pins plan_mosaic's parameters without memory_budget (1); test_feature_input's fake(src, dst) reprojectors and single_call_reprojector(src, dst) (3) and test_target_grid's _SpiedReprojector.__call__(source, target) (4) do not take the threads= keyword the design's call sites pass. Co-Authored-By: Claude Fable 5.1 --- INSTALL.md | 7 ++ README.md | 5 +- project_structure.md | 7 +- src_python/tin_engine/catchment.py | 57 +++++---- src_python/tin_engine/catchment_batch.py | 7 +- src_python/tin_engine/cli.py | 90 ++++++++++++--- src_python/tin_engine/crs.py | 22 ++-- src_python/tin_engine/decompose.py | 93 --------------- src_python/tin_engine/dem_input.py | 47 +++++--- src_python/tin_engine/edge_strip.py | 5 +- src_python/tin_engine/feature_input.py | 11 +- src_python/tin_engine/final_check.py | 5 +- src_python/tin_engine/io/cog.py | 16 +-- src_python/tin_engine/io/mesh_index.py | 2 +- src_python/tin_engine/io/repository.py | 25 ++-- src_python/tin_engine/land_cover_fractions.py | 8 +- src_python/tin_engine/mcp/schema.py | 3 +- src_python/tin_engine/mosaic.py | 26 +++-- src_python/tin_engine/resources.py | 108 ++++++++++++++++++ src_python/tin_engine/run_record.py | 1 + src_python/tin_engine/stats.py | 2 +- src_python/tin_engine/target_grid.py | 4 +- 22 files changed, 344 insertions(+), 207 deletions(-) delete mode 100644 src_python/tin_engine/decompose.py create mode 100644 src_python/tin_engine/resources.py diff --git a/INSTALL.md b/INSTALL.md index 96ad9df2..51c909eb 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -188,6 +188,13 @@ The static checks CI runs are listed in `CLAUDE.md`, section 4. Two DEMs rasputin fetches itself, into a tile cache (`$RASPUTIN_DATA/cache` by default, or `--cache DIR`): +Two more environment variables: `RASPUTIN_THREADS` is the default thread +count for `mesh`, `catchment` and `station-catchments` (0 or unset means every +core; `--threads N` overrides it), and inside a container the core count and +the memory follow the container's own limits (`cpu.max`, `memory.max`). +`--memory-budget 8G` caps the DEM window one run assembles; the default is +half the machine's memory. + ```sh rasputin fetch glo30 --domain catchment.geojson # --dry-run prints the plan rasputin mesh --dem glo30 --domain catchment.geojson --out-crs EPSG:32723 \ diff --git a/README.md b/README.md index 4daae79a..2fd1d6dd 100644 --- a/README.md +++ b/README.md @@ -50,8 +50,9 @@ its CORINE land-cover class. Elevation © Kartverket (CC BY 4.0); land cover Install first ([INSTALL.md](INSTALL.md)); the example needs the `codecs` extra, because the committed DEM tile is LZW-compressed. The install is bounds-checked by default; INSTALL.md's "Bounds checks" section says how to -build an unchecked copy for heavy runs. From the repository -root: +build an unchecked copy for heavy runs. Every run uses every core unless +`--threads N` or the environment variable `RASPUTIN_THREADS` says otherwise +(0 means every core). From the repository root: ```sh # A 50 km DTM10 tile, meshed inside a quarter disc of radius 30 km, diff --git a/project_structure.md b/project_structure.md index b2ff83b3..e189b698 100644 --- a/project_structure.md +++ b/project_structure.md @@ -157,6 +157,10 @@ src_python/tin_engine/ # public Python API (distribution name: rasputin) # summary and --record JSON; escaped_ascii, the # one non-ASCII escape (\xe9) for file fields and # crs_label; imports json and dataclasses only + resources.py # the one module that reads the machine: cores, + # memory, cgroup limits; Resources (threads, a + # memory budget), resolved once per command (38); + # Pydantic and os only features.py # EdgeVocabulary: which bit means which feature. # The names C++ refuses to hold. Imports nothing # first-party and never imports _core. @@ -167,9 +171,6 @@ src_python/tin_engine/ # public Python API (distribution name: rasputin) # (bit 9), for a cut run's piece files (23c). # TerrainFeature: one feature's mask and lines # (moved from feature_input, audit PR C) - decompose.py # partition: the window cut into cells under - # --pieces and --memory-budget, b(T); pure, reads - # no machine (23c) feature_input.py # --features: a GeoJSON, GeoPackage layer or GML # read, mapped to masks by a ClassMap, pre-clipped, # moved to the DEM's CRS and clipped to the domain diff --git a/src_python/tin_engine/catchment.py b/src_python/tin_engine/catchment.py index 58b042da..9ca097fd 100644 --- a/src_python/tin_engine/catchment.py +++ b/src_python/tin_engine/catchment.py @@ -52,10 +52,10 @@ MosaicError, MosaicPlan, assemble, - physical_memory, plan_mosaic, ) from tin_engine.outline import trace +from tin_engine.resources import Resources from tin_engine.sensitivity import Sensitivity, assess #: Metres round the seed's bounds, and round the catchment's when the window @@ -213,10 +213,14 @@ def _code_hint(given: str, dem_crs: str) -> str: return f"; if you mean EPSG:{code}, write EPSG:{code}" -def delineate(request: CatchmentRequest, repository: DemRepository) -> Catchment: +def delineate( + request: CatchmentRequest, repository: DemRepository, resources: Resources | None = None +) -> Catchment: """The catchment of the request's seed over the repository's DEM, or a :class:`CatchmentError` (truncated by the data's edge or NoData, no lake - or two under the point, over the memory cap).""" + or two under the point, over the memory budget). `resources` (38) gives + the budget one window may take; None is `Resources.fallback()`.""" + budget = (Resources.fallback() if resources is None else resources).memory_budget footprints = repository.footprints() dem_crs = single_crs((f.meta.crs for f in footprints), CatchmentError) ((x, y),) = reprojector(request.seed_crs, dem_crs)([request.seed]) @@ -226,7 +230,7 @@ def delineate(request: CatchmentRequest, repository: DemRepository) -> Catchment if not same_crs(request.seed_crs, dem_crs): hint = _code_hint(request.seed_crs, dem_crs) raise CatchmentError(f"the river reach must be in the DEM's CRS, {dem_crs}{hint}") - return _gauged(request, repository, footprints, dem_crs) + return _gauged(request, repository, footprints, dem_crs, budget) lake = _lake(request, dem_crs) x0, y0, x1, y1 = lake.bounds if lake is not None else (x, y, x, y) margin = float(WINDOW_MARGIN_M) @@ -237,7 +241,7 @@ def flood(tile: DemTile) -> tuple[UpstreamOutcome, Mask]: return upstream(tile, seed), seed bounds = Bounds(x_min=x0 - margin, y_min=y0 - margin, x_max=x1 + margin, y_max=y1 + margin) - m, out, seed, windows = _grow(footprints, repository, bounds, flood, False) + m, out, seed, windows = _grow(footprints, repository, bounds, flood, False, budget) fine = _outline(dem_crs, lake, point, out, m, seed, tuple(windows)) return _reduced(fine, _tolerance(request, m)) @@ -248,23 +252,28 @@ def _tolerance(request: CatchmentRequest, m: RasterMeta) -> float: def _grow[T]( - footprints: Footprints, repository: DemRepository, bounds: Bounds, flood: Flood[T], burnt: bool + footprints: Footprints, + repository: DemRepository, + bounds: Bounds, + flood: Flood[T], + burnt: bool, + budget: int, ) -> tuple[RasterMeta, UpstreamOutcome, T, list[Window]]: """22's window loop: flood, grow until the catchment is clear of the - window's edge, or refuse (NoData, the data's edge, the memory cap, whose - per-node figure grows by a burnt copy and `accumulate`'s 10 bytes when - `burnt`).""" - plan, margin = _plan(footprints, bounds), float(WINDOW_MARGIN_M) + window's edge, or refuse (NoData, the data's edge, the memory budget, + whose per-node figure grows by a burnt copy and `accumulate`'s 10 bytes + when `burnt`).""" + plan, margin = _plan(footprints, bounds, budget), float(WINDOW_MARGIN_M) windows: list[Window] = [] while True: m = plan.meta itemsize = np.result_type(*(t.dtype for t in plan.tiles)).itemsize need = m.rows * m.cols * (itemsize * (1 + burnt) + 2 + 10 * burnt) - cap = physical_memory() // 2 - if need > cap: + if need > budget: raise CatchmentError( - f"the window is {m.rows} x {m.cols} nodes, {need} bytes to flood, over the cap " - f"of half the physical memory ({cap} bytes)" + f"the window is {m.rows} x {m.cols} nodes, {need} bytes to flood, over the " + f"memory budget of {budget} bytes (set with --memory-budget; half the " + "machine's memory when it is not set)" ) tile = assemble(plan, repository.load).tile t0 = time.perf_counter() @@ -276,12 +285,13 @@ def _grow[T]( raise CatchmentError("no seed node has data: the seed lies on NoData") # Grow while the in-nodes' bounds plus the base margin reach past the # window; the step itself takes the margin doubled at each growth. - grown = _grown(plan, _plan(footprints, _joined(m, out, float(WINDOW_MARGIN_M)))) + probe = _plan(footprints, _joined(m, out, float(WINDOW_MARGIN_M)), budget) + grown = _grown(plan, probe) windows.append(Window(Bounds.of(m.node_box()), m.rows, m.cols, seconds, grown)) if not grown: break margin *= 2 - plan = _plan(footprints, _joined(m, out, margin)) + plan = _plan(footprints, _joined(m, out, margin), budget) if out.touches_edge: sides = _edge_sides(m, out) raise CatchmentError(f"the catchment is cut by the data's {' and '.join(sides)} edge") @@ -309,7 +319,11 @@ def flood(tile: DemTile) -> tuple[UpstreamOutcome, tuple[DemTile, GaugePath, Mas def _gauged( - request: CatchmentRequest, repository: DemRepository, footprints: Footprints, dem_crs: str + request: CatchmentRequest, + repository: DemRepository, + footprints: Footprints, + dem_crs: str, + budget: int, ) -> Catchment: """Stage A floods from the placed node and decides the catchment and every refusal; stage B grows on from it until `D` (the first chain node at or @@ -330,10 +344,11 @@ def at_u(path: GaugePath) -> int: return int(past[0]) m, out, (burnt, path, seed), windows = _grow( - footprints, repository, bounds, _burnt_flood(reach, lambda p: int(p.placed)), True + footprints, repository, bounds, _burnt_flood(reach, lambda p: int(p.placed)), True, budget ) try: - b = _grow(footprints, repository, Bounds.of(m.node_box()), _burnt_flood(reach, at_u), True) + again = Bounds.of(m.node_box()), _burnt_flood(reach, at_u), True, budget + b = _grow(footprints, repository, *again) except CatchmentError: pass # stage B never refuses a catchment: read what stage A's window holds else: @@ -386,9 +401,9 @@ def _lake(request: CatchmentRequest, dem_crs: str) -> Polygon | None: return lake -def _plan(footprints: Footprints, bounds: Bounds) -> MosaicPlan: +def _plan(footprints: Footprints, bounds: Bounds, budget: int) -> MosaicPlan: try: - return plan_mosaic(footprints, bounds) + return plan_mosaic(footprints, bounds, memory_budget=budget) except MixedGridError as exc: raise MixedGridRefusal(str(exc), exc.tiles) from exc except MosaicError as exc: diff --git a/src_python/tin_engine/catchment_batch.py b/src_python/tin_engine/catchment_batch.py index e17d0f82..c8606abc 100644 --- a/src_python/tin_engine/catchment_batch.py +++ b/src_python/tin_engine/catchment_batch.py @@ -35,6 +35,7 @@ from tin_engine.hydrography import Lake, RiverSegment, Station from tin_engine.io.repository import DemRepository from tin_engine.reference import Class, Summary, agreement, classify, nodes_inside, summarise +from tin_engine.resources import Resources class BatchRequest(BaseModel): @@ -169,10 +170,12 @@ async def run_batch( references: Mapping[str, BaseGeometry] | None, sink: BatchSink, lakes: Sequence[Lake] | None = None, + resources: Resources | None = None, ) -> Summary: """Run each station in `stations` (or those in `request.only`, in file order); `references` and `lakes` are in the river file's CRS. A station - `gauge.lake_seed` picks is seeded with its lake. Returns the summary.""" + `gauge.lake_seed` picks is seeded with its lake; `resources` (38) goes to + each `delineate`. Returns the summary.""" unknown = set(request.only) - {s.station for s in stations} if unknown: raise ValueError(f"not in the stations file: {', '.join(sorted(unknown))}") @@ -206,7 +209,7 @@ async def run_batch( fields |= {"lake_name": first.name, "lake_distance_m": seed.distance_m} else: fields["seeded_by"] = "river" - result = await asyncio.to_thread(delineate, catchment_request, repository) + result = await asyncio.to_thread(delineate, catchment_request, repository, resources) except NoRiverLine as exc: fields |= {"refusal_cause": "no_river", "refusal_message": str(exc)} except MixedGridRefusal as exc: diff --git a/src_python/tin_engine/cli.py b/src_python/tin_engine/cli.py index 3cd69964..d5b14a53 100644 --- a/src_python/tin_engine/cli.py +++ b/src_python/tin_engine/cli.py @@ -144,6 +144,7 @@ from tin_engine.mosaic import Seam from tin_engine.palettes import PALETTES, paraview_preset from tin_engine.raster import to_core +from tin_engine.resources import VARIABLE, Resources, resources from tin_engine.run_record import ( SEAMS_AGREE, RunRecord, @@ -174,6 +175,29 @@ no_args_is_help=True, ) +#: Increment 38: the two options `mesh`, `catchment` and `station-catchments` share. +THREADS = Annotated[ + int | None, + typer.Option( + "--threads", + min=0, + metavar="N", + help="Threads for every parallel step (refinement, decoding, resampling, " + "reprojection); 0 means every core, a container's CPU quota counted. Default: " + "$RASPUTIN_THREADS, else every core.", + ), +] +MEMORY_BUDGET = Annotated[ + str | None, + typer.Option( + "--memory-budget", + metavar="BYTES", + help="The most one DEM window may take: a number of bytes, or with K, M, G or T " + "(8G). Default: half the machine's memory, a container's limit counting as the " + "machine's.", + ), +] + #: The fixtures' vocabulary, mapped onto the enum. A role outside it is a #: ``KeyError`` here rather than a silently unconstrained chain downstream. ROLES = { @@ -887,6 +911,8 @@ def mesh( help="With --land-cover, the year of the classes. Default: the source's last year.", ), ] = None, + threads: THREADS = None, + memory_budget: MEMORY_BUDGET = None, stats: Annotated[ str | None, typer.Option( @@ -957,6 +983,7 @@ def mesh( ``--record`` all of it, and stderr one summary. """ clock = PhaseClock() + res = _resources(threads, memory_budget) dem_run: _DemMesh | None = None codes: npt.NDArray[np.int32] | None = None codes_text = "" @@ -1097,7 +1124,7 @@ def mesh( except DomainError as exc: raise typer.BadParameter(str(exc), param_hint="--domain") from exc try: - opened = _open_dem(paths, bbox, clock, given, cached, out_crs) + opened = _open_dem(paths, bbox, clock, given, cached, out_crs, res) except NotCached as exc: area = f"--domain {domain}" if domain is not None else "" if bbox is not None: @@ -1116,7 +1143,7 @@ def mesh( sources: tuple[FeatureSource, ...] = () if feature_paths and dem_domain is not None: sources = _feature_sources(feature_paths, feature_crss, feature_layers, feature_maps) - found = _open_features(sources, dem_domain, dem_crs, clock, cleanup) + found = _open_features(sources, dem_domain, dem_crs, clock, cleanup, res.threads) # 32: 15 degrees with land cover simplified, unless given. simplified = ( found is not None and found.cover_vertices is not None and features_tolerance > 0 @@ -1141,6 +1168,7 @@ def mesh( DEFAULT_START_QUALITY_GAIN if start_quality_gain is None else start_quality_gain, lines, steep, + res.threads, ) surface_mesh, meta, values = dem_run.trimmed, dem_run.meta, dict(dem_run.values) names = [t.name for t in plan.tiles] @@ -1211,12 +1239,12 @@ def mesh( cover = [(f.code, f.polygon) for f in found.features if f.polygon and f.code] if classes is not None: table, codes, more = _class_fractions( - classes, dem_run.trimmed, dem_crs, clock, domain, out_crs, cache + classes, dem_run.trimmed, dem_crs, clock, domain, out_crs, cache, res.threads ) codes_text = str(more["land_cover_codes"]) values |= more build = stride_record if tolerance is None else refined_record - record = build(triangles=len(surface_mesh.triangles), **values) + record = build(triangles=len(surface_mesh.triangles), threads=res.threads, **values) else: assert name is not None if stride is not None: @@ -1314,7 +1342,8 @@ def mesh( target.write_bytes(data) typer.echo(f"{target}") if stats is not None: - _write_report(clock, report_target, surface_mesh, dem_run, targets, seams, record) + used = res.threads if dem_run is not None else None + _write_report(clock, report_target, surface_mesh, dem_run, targets, seams, record, used) if record_target is not None: # D5: after the mesh and the report record_target.write_text(as_json(record, installed_version(), _command()), "ascii") typer.echo(f"{record_target}") @@ -1418,12 +1447,13 @@ def _class_fractions( domain: Path | None, out_crs: str | None, cache: Path | None, + threads: int = 1, ) -> tuple[ClassFractions, npt.NDArray[np.int32], dict[str, Value]]: """26b: the exact class fractions from the cached year, timed as the phase ``land cover fractions``; the table, the codes and the record entries. An object the cache lacks is refused naming the ``fetch`` that gets it.""" source, year = classes.source, classes.year - repository = CacheRepository(classes.root, source.id, nodata=source.nodata) + repository = CacheRepository(classes.root, source.id, nodata=source.nodata, threads=threads) url = source.url_for(year) name = url.rsplit("/", 1)[-1].split("?")[0].rsplit(".", 1)[0] try: @@ -1450,6 +1480,7 @@ def _class_fractions( mesh_crs=dem_crs, repository=repository, name=name, + threads=threads, ) except (ValueError, NotCached) as exc: raise typer.BadParameter(_words(exc), param_hint="--land-cover") from exc @@ -1557,9 +1588,11 @@ def _write_report( files: list[Path], seams: tuple[Seam, ...], record: RunRecord, + threads: int | None = None, ) -> None: """Build the report from what ran and write it, or print it for ``-``. - The total stops here; the quality pass is timed on its own line (R4).""" + The total stops here; the quality pass is timed on its own line (R4). + ``threads`` is the count the run used (38), None on a fixture run.""" total = clock.elapsed() t0 = time.perf_counter_ns() meta = dem_run.meta if dem_run else None @@ -1587,7 +1620,7 @@ def _write_report( total=total, stats_seconds=(time.perf_counter_ns() - t0) / 1e9, bounds_checks=bounds_checks(), - threads=os.cpu_count() if sizes.start_vertices is not None else None, + threads=threads, seams=[s.cells() for s in seams], inputs=[row for row, inputs in rows if inputs], result=[row for row, inputs in rows if not inputs], @@ -1753,6 +1786,17 @@ def fetch_stations( typer.echo(f"{source}: {len(files)} files written to {out_dir}", err=True) +def _resources(threads: int | None, memory_budget: str | None) -> Resources: + """``--threads`` and ``--memory-budget`` resolved against the environment + (38, built once per command); a refusal is a usage error naming the + option or the variable.""" + try: + return resources(threads, memory_budget, os.environ) + except ValueError as exc: + hint = VARIABLE if str(exc).startswith(VARIABLE) else "--memory-budget" + raise typer.BadParameter(str(exc), param_hint=hint) from exc + + def cache_root(option: Path | None, environ: Mapping[str, str]) -> Path | None: """The tile cache (B7): ``--cache`` if given, else ``$RASPUTIN_DATA/cache``, else None. The one reader of the variable; ``mesh`` passes ``os.environ``.""" @@ -1793,6 +1837,7 @@ def _open_dem( domain: DomainPolygon | None = None, cached: CachedSource | None = None, target_crs: str | None = None, + res: Resources | None = None, ) -> DemInput: """``--dem`` and ``--bbox`` or the read ``--domain`` to one tile (increment 15a and 15b, R11); every refusal, the reader's, the mosaic's or the @@ -1810,7 +1855,7 @@ def _open_dem( request = DemRequest( sources=dem, cached=cached, bounds=bounds, domain=domain, target_crs=target_crs ) - return open_dem(request) + return open_dem(request, res) except NotCached: raise except OutCrsRequiredError as exc: # Q11: one unwrapped line, outside the panel, to paste @@ -1872,13 +1917,15 @@ def _open_features( dem_crs: str, clock: PhaseClock, cleanup: dict[str, float], + threads: int = 1, ) -> FeatureSet: """R5-R6 and R10: read and clip every source into one set, timed as ``features read`` and ``features clip``; a refusal is a usage error naming the feature.""" t0 = time.perf_counter() try: - found = open_features(FeatureRequest(sources=sources, **cleanup), domain, dem_crs) + request = FeatureRequest(sources=sources, threads=threads, **cleanup) + found = open_features(request, domain, dem_crs) except FeatureError as exc: raise typer.BadParameter(str(exc), param_hint="--features") from exc clock.add("features read", time.perf_counter() - t0 - found.clip_seconds) @@ -2055,6 +2102,7 @@ def _dem_mesh( gain: float = -1.0, lines: ToleranceLines | None = None, steep: ToleranceSlope | None = None, + threads: int = 0, ) -> _DemMesh: """Subsample, triangulate, sample or refine, and trim ``held``'s tile. @@ -2134,7 +2182,7 @@ def _dem_mesh( if steep is not None: with clock.phase("slope"): ramp = (steep.near_m, tolerance, steep.start_deg, steep.end_deg) - slope = SlopeTolerance(to_core(tile), *ramp) + slope = SlopeTolerance(to_core(tile), *ramp, threads=threads) t0 = time.perf_counter_ns() out = refine( to_core(tile), @@ -2147,13 +2195,16 @@ def _dem_mesh( min_gain_deg=gain, field=field, slope=slope, + threads=threads, ) _refine_phases(clock, (time.perf_counter_ns() - t0) / 1e9, out) if not out.ok(): raise typer.BadParameter(f"{dem}: {out.message}", param_hint="--dem") strip = edge_strip.generate(to_core(tile), out, clock) # 15f, D6: while the tile is held if grid is None or checks is None: - final = edge_strip.run(to_core(tile), strip, out, tolerance, clock, feet, field, slope) + final = edge_strip.run( + to_core(tile), strip, out, tolerance, clock, feet, field, slope, threads + ) del tile # 15f, D7: refine's maximum and the strip run's make an upper bound. max_error = max(out.max_error, final.max_error) @@ -2164,7 +2215,7 @@ def _dem_mesh( else: del tile # 15e fix 3: phase 2 runs without the target tile final, n = final_check.run( - out, grid, checks, tolerance, clock, strip, feet, field, slope + out, grid, checks, tolerance, clock, strip, feet, field, slope, threads ) max_error, near_error = final.max_error, final.max_error_near share = final.max_slope_share @@ -2412,6 +2463,8 @@ def catchment( help="How much river above the gauge is burnt into the DEM. Default 1000.", ), ] = None, + threads: THREADS = None, + memory_budget: MEMORY_BUDGET = None, ) -> None: """Write the catchment of a lake, from the DEM, as a GeoJSON polygon in the DEM's CRS that ``mesh --domain`` reads (increment 22). The fine outline is @@ -2441,8 +2494,9 @@ def catchment( found = None if lakes is None else read_lake_polygons(lakes, lakes_layer, seed, seed_crs) except FeatureError as exc: raise typer.BadParameter(str(exc), param_hint="--lakes") from exc + res = _resources(threads, memory_budget) try: - repository, _ = repository_for(tuple(dem)) + repository, _ = repository_for(tuple(dem), threads=res.threads) placement: tuple[Placement, str | None] | None = None if rivers is not None: batch = BatchRequest( @@ -2459,7 +2513,7 @@ def catchment( lakes_crs=None if found is None else found[1], outline_tolerance=outline_tolerance, ) - result = delineate(request, repository) + result = delineate(request, repository, res) except OSError as exc: raise typer.BadParameter(f"cannot read {exc.filename}: {exc}", param_hint="--dem") from exc except ValueError as exc: @@ -2667,6 +2721,8 @@ def station_catchments( Path | None, typer.Option("--out-parent", help="Refuse any output path resolving outside this."), ] = None, + threads: THREADS = None, + memory_budget: MEMORY_BUDGET = None, ) -> None: """Write a catchment per station into --out-dir, as ``catchment --rivers`` does for one, with results.csv (a row per station) and summary.json; with @@ -2696,8 +2752,9 @@ def station_catchments( lake_list = None if lakes is not None: lake_list = _read_beside(read_nve_lakes, lakes, "--lakes", "lakes", crs) + res = _resources(threads, memory_budget) try: - repository, _ = repository_for(tuple(dem)) + repository, _ = repository_for(tuple(dem), threads=res.threads) except (OSError, ValueError) as exc: raise typer.BadParameter( _words(exc) if isinstance(exc, ValueError) else str(exc), param_hint="--dem" @@ -2729,6 +2786,7 @@ def station_catchments( references, sink, lakes=lake_list, + resources=res, ) ) except OSError as exc: diff --git a/src_python/tin_engine/crs.py b/src_python/tin_engine/crs.py index 71b1b1f8..fb9b5247 100644 --- a/src_python/tin_engine/crs.py +++ b/src_python/tin_engine/crs.py @@ -14,7 +14,6 @@ from __future__ import annotations import math -import os import threading from collections.abc import Callable, Iterable from concurrent.futures import ThreadPoolExecutor @@ -33,10 +32,10 @@ Xy = npt.NDArray[np.float64] #: Increment 35, 3.1 (b): an array of `2 * CHUNK_POINTS` points or more is -#: split into chunks of at least `CHUNK_POINTS` (about 60 ms of PROJ work -#: each) and mapped on up to `WORKERS` threads, each with its own `Transformer`. +#: split into chunks of at least `CHUNK_POINTS` (a chunk of work, about 60 ms +#: of PROJ on one machine) and mapped on up to `threads` threads, each with its +#: own `Transformer` (38: the count is the caller's; one worker by default). CHUNK_POINTS = 250_000 -WORKERS = min(8, os.cpu_count() or 1) def parse_crs(text: str | CRS) -> CRS: @@ -49,13 +48,14 @@ def parse_crs(text: str | CRS) -> CRS: raise ValueError(f"cannot read the CRS {text!r}: {exc}") from exc -def reprojector(src: str | CRS, dst: str | CRS) -> Callable[[Any], Xy]: +def reprojector(src: str | CRS, dst: str | CRS, *, threads: int = 1) -> Callable[[Any], Xy]: """A map from an `(N, 2)` array-like of `(x, y)` in `src` to an `(N, 2)` float64 array in `dst`: pyproj's `always_xy` transform, bit for bit. - A point with no image comes back as `inf`; the caller decides. A large - array is mapped in chunks on threads (`CHUNK_POINTS`), one `Transformer` - per thread since pyproj's are not thread-safe; a projection is applied - point by point, so the bits are those of one call.""" + A point with no image comes back as `inf`; the caller decides. With + `threads` above 1, a large array is mapped in chunks on up to that many + threads (`CHUNK_POINTS`), one `Transformer` per thread since pyproj's are + not thread-safe; a projection is applied point by point, so the bits are + those of one call.""" local = threading.local() local.transformer = _transformer(src, dst) @@ -67,9 +67,9 @@ def one(points: Xy) -> Xy: def apply(xy: Any) -> Xy: points = np.asarray(xy, dtype=np.float64).reshape(-1, 2) - if len(points) < 2 * CHUNK_POINTS: + if threads < 2 or len(points) < 2 * CHUNK_POINTS: return one(points) - chunks = np.array_split(points, min(WORKERS, len(points) // CHUNK_POINTS)) + chunks = np.array_split(points, min(threads, len(points) // CHUNK_POINTS)) with ThreadPoolExecutor(len(chunks)) as pool: return np.concatenate(list(pool.map(one, chunks))) diff --git a/src_python/tin_engine/decompose.py b/src_python/tin_engine/decompose.py deleted file mode 100644 index 316be1ca..00000000 --- a/src_python/tin_engine/decompose.py +++ /dev/null @@ -1,93 +0,0 @@ -"""The partition of a run's window into cells, one piece job per cell (increment 23c). - -Pure arithmetic on the window: no DEM, no mesh, no machine. The rule is -`docs/increments/23-basin-scale.md`, "The partition (23c; B2, B3 and B14 as -ruled)", and the memory estimate is "The memory estimate and the defaults"; -`docs/increments/23-probes/partition.py` implements the same rule and its -figures are the design's. - -Neither the count nor the cut reads the machine (K5): the budget is a -constant the caller passes, never the machine's memory or cores. -""" - -from __future__ import annotations - -import itertools -import math -from dataclasses import dataclass - -#: `--memory-budget`'s default, 16 GiB in bytes (B14 as ruled). -DEFAULT_MEMORY_BUDGET = 2**34 - -#: `b(T)`: bytes per window node at tolerance T (m), from the basin-piece -#: sweep (`docs/benchmarks/2026-10-01/basin-piece/README.md`). -_BYTES_PER_NODE = ( - (0.0, 637), - (1.0, 267), - (2.0, 155), - (5.0, 65), - (10.0, 37), - (20.0, 25), - (50.0, 19), -) - - -def bytes_per_node(tolerance: float) -> int: - """`b(T)`: linear between the table's columns, rounded up; 19 above 50 m.""" - for (t0, b0), (t1, b1) in itertools.pairwise(_BYTES_PER_NODE): - if tolerance <= t1: - return math.ceil(b0 + (b1 - b0) * (tolerance - t0) / (t1 - t0)) - return _BYTES_PER_NODE[-1][1] - - -@dataclass(frozen=True, slots=True) -class Partition: - """`nx` x `ny` cells of `dx` x `dy` lattice nodes; the last ones may be narrower.""" - - nx: int - ny: int - dx: int - dy: int - - -def partition( - cols: int, - rows: int, - tolerance: float, - *, - pieces: int = 1, - memory_budget: int = DEFAULT_MEMORY_BUDGET, -) -> Partition: - """The cells of a `cols` x `rows` window: the design's steps 1-3. - - The count approximates `max(pieces, ceil(N b / B))` and is not a floor; - only `dx dy b <= memory_budget` is guaranteed. Refuses an empty window, - `pieces < 1`, and a budget under one node's bytes, for which no cell fits. - """ - if cols < 1 or rows < 1: - raise ValueError(f"the window is empty: cols {cols} and rows {rows} must be at least 1") - if pieces < 1: - raise ValueError(f"pieces must be at least 1, got {pieces}") - b = bytes_per_node(tolerance) - if memory_budget < b: - raise ValueError( - f"the memory budget of {memory_budget} bytes is under one node's {b} bytes" - ) - wanted = max(pieces, -(-cols * rows * b // memory_budget)) - if wanted <= 1: - return Partition(1, 1, cols, rows) - nx = max(1, round(math.sqrt(wanted * cols / rows))) - ny = max(1, round(wanted / nx)) - dx, dy = -(-cols // nx), -(-rows // ny) - # Ends: it runs only while a cell is over budget, and the budget is at - # least one node's bytes (refused above otherwise), so a 1 x 1 cell fits. - # A step adds one to nx only while dx >= dy and the cell is over, so - # dx > 1, i.e. nx < cols; likewise ny only while dy > 1, i.e. ny < rows. - # So nx stays at most cols and ny at most rows: at most cols + rows - 2 steps. - while dx * dy * b > memory_budget: - if dx >= dy: - nx += 1 - else: - ny += 1 - dx, dy = -(-cols // nx), -(-rows // ny) - return Partition(-(-cols // dx), -(-rows // dy), dx, dy) diff --git a/src_python/tin_engine/dem_input.py b/src_python/tin_engine/dem_input.py index 044cb168..0a178444 100644 --- a/src_python/tin_engine/dem_input.py +++ b/src_python/tin_engine/dem_input.py @@ -20,7 +20,6 @@ from __future__ import annotations import math -import os from collections.abc import Iterator, Sequence from dataclasses import dataclass from pathlib import Path @@ -43,6 +42,7 @@ from tin_engine.io.models import Bounds, DemTile, RasterMeta, TileFootprint from tin_engine.io.repository import CacheRepository, DemRepository, TiffDemRepository from tin_engine.mosaic import MosaicError, MosaicPlan, Seam, assemble, plan_mosaic +from tin_engine.resources import Resources from tin_engine.target_grid import ( Block, TargetGrid, @@ -115,30 +115,39 @@ class DemInput: def repository_for( - sources: tuple[Path, ...], nodata: float | None = None, cached: CachedSource | None = None + sources: tuple[Path, ...], + nodata: float | None = None, + cached: CachedSource | None = None, + threads: int = 1, ) -> tuple[TiffDemRepository | CacheRepository, str]: """The repository over one directory of tiles, over the files given, or over a cached source, and the run's name: the directory's name, the first - file's stem, or the source's key.""" + file's stem, or the source's key; `threads` decode its blocks (38).""" if cached is not None: - return CacheRepository(cached.cache, cached.source, nodata=nodata), cached.source + repository = CacheRepository(cached.cache, cached.source, nodata=nodata, threads=threads) + return repository, cached.source first = sources[0] if first.is_dir(): - return TiffDemRepository.from_directory(first, nodata=nodata), first.resolve().name - return TiffDemRepository(sources, nodata=nodata), first.stem + directory = TiffDemRepository.from_directory(first, nodata=nodata, threads=threads) + return directory, first.resolve().name + return TiffDemRepository(sources, nodata=nodata, threads=threads), first.stem -def open_dem(request: DemRequest) -> DemInput: +def open_dem(request: DemRequest, resources: Resources | None = None) -> DemInput: """List, plan and assemble. Every refusal is a `ValueError` (`GeoTiffError`, - `MosaicError`) or, for a file that cannot be opened, an `OSError`.""" - repository, label = repository_for(request.sources, request.nodata, request.cached) + `MosaicError`) or, for a file that cannot be opened, an `OSError`. + `resources` (38) is the thread count for the decoders and the pools and + the memory budget for the canvas; None is `Resources.fallback()`.""" + res = Resources.fallback() if resources is None else resources + repository, label = repository_for(request.sources, request.nodata, request.cached, res.threads) footprints = repository.footprints() if footprints and _reprojected(request, [f.meta for f in footprints]): - return _open_reprojected(request, repository, footprints, label) + return _open_reprojected(request, repository, footprints, label, res) if request.domain is None or not footprints: - plan, domain, grown = plan_mosaic(footprints, request.bounds, None), None, None + plan = plan_mosaic(footprints, request.bounds, None, memory_budget=res.memory_budget) + domain = grown = None else: - plan, domain, grown = _domain_plan(footprints, request.domain) + plan, domain, grown = _domain_plan(footprints, request.domain, res.memory_budget) # With a domain the seam report counts only the needed region's nodes # (Ola, 2026-09-28); which value a node takes does not depend on it. repository.check(plan) @@ -194,6 +203,7 @@ def _open_reprojected( repository: DemRepository, footprints: Sequence[TileFootprint], label: str, + res: Resources, ) -> DemInput: """D1's reprojected path: the target grid around the domain (or box) in the target CRS, the source region planned and assembled as 15a does, @@ -212,10 +222,10 @@ def _open_reprojected( ((ax, ay),) = reprojector(target, meta.crs)([(c.x, c.y)]) grid, grown = target_grid_for(domain, target, default_spacing(meta, (ax, ay))) box, needed = source_region(grid, meta, grown) - plan = plan_mosaic(footprints, box, needed) + plan = plan_mosaic(footprints, box, needed, memory_budget=res.memory_budget) repository.check(plan) mosaic = assemble(plan, repository.load, needed, load_window=repository.load_window) - source, threads = TileWindows(mosaic.tile), os.cpu_count() or 1 + source, threads = TileWindows(mosaic.tile), res.threads return DemInput( tile=resample(grid, source, threads), plan=plan, @@ -229,7 +239,7 @@ def _open_reprojected( def _domain_plan( - footprints: Sequence[TileFootprint], given: DomainPolygon + footprints: Sequence[TileFootprint], given: DomainPolygon, budget: int | None = None ) -> tuple[MosaicPlan, DomainPolygon, Any]: """The domain in the DEM's CRS, the plan of its bounds with the polygon grown by one cell needed (R4 point 5), and that grown polygon, the needed @@ -245,12 +255,13 @@ def _domain_plan( # re-plan while the lattice chosen has a larger one. The reach only # grows, so this ends, and the region needed is never short of the # chosen lattice's own cell. - plan, reach, grown = plan_mosaic(footprints, box, domain.polygon), 0.0, domain.polygon + plan = plan_mosaic(footprints, box, domain.polygon, memory_budget=budget) + reach, grown = 0.0, domain.polygon while (diagonal := math.hypot(plan.meta.delta_x, plan.meta.delta_y)) > reach: reach, grown = diagonal, grow_mitred(domain.polygon, diagonal) - plan = plan_mosaic(footprints, box, grown) + plan = plan_mosaic(footprints, box, grown, memory_budget=budget) if past := _past(box, plan.meta): # a vertex within 1e-6 cell past a node line - plan = plan_mosaic(footprints, past, grown) + plan = plan_mosaic(footprints, past, grown, memory_budget=budget) check_extent(domain, plan.meta) except (DomainError, MosaicError) as exc: raise type(exc)(f"the domain, in {given.crs}, in the DEM's {crs}: {exc}") from exc diff --git a/src_python/tin_engine/edge_strip.py b/src_python/tin_engine/edge_strip.py index c515bdab..2ae0ed3f 100644 --- a/src_python/tin_engine/edge_strip.py +++ b/src_python/tin_engine/edge_strip.py @@ -39,17 +39,20 @@ def run( feet: bool = False, field: LineTolerance | None = None, slope: SlopeTolerance | None = None, + threads: int = 0, ) -> PointRefineOutcome: """``refine_strip`` from ``start``; ``clock`` gets the run's own two times. ``feet`` puts points near a line onto it first (20c, R4); ``field``, if any, sets each triangle's tolerance (increment 33); ``slope``, if any, - holds each point to its slope's tolerance too (increment 34).""" + holds each point to its slope's tolerance too (increment 34); ``threads`` + goes to the C++ as given (38; its 0 is every core).""" arrays = (start.vertices, start.triangles, start.z, start.valid, start.edges, start.masks) out = refine_strip( view, strip, *(np.asarray(a) for a in arrays), tolerance=tolerance, + threads=threads, constraint_feet=feet, field=field, slope=slope, diff --git a/src_python/tin_engine/feature_input.py b/src_python/tin_engine/feature_input.py index 94167706..ce92ae85 100644 --- a/src_python/tin_engine/feature_input.py +++ b/src_python/tin_engine/feature_input.py @@ -143,6 +143,8 @@ class FeatureRequest(BaseModel): merge_same_class: bool = False tolerance_m: float = 0.0 outline_snap_m: float = 0.0 + #: 38: threads for moving a source's vertices into the DEM's CRS. + threads: int = 1 class FeatureSet(BaseModel): @@ -272,7 +274,7 @@ def open_features(request: FeatureRequest, domain: DomainPolygon, dem_crs: str | vocabulary.mask(*named, *(() if isinstance(cmap.otherwise, str) else cmap.otherwise)) except ValueError as exc: raise FeatureError(f"class map {cmap.name}: {exc}") from exc - tally = _Tally(domain, parse_crs(dem_crs), vocabulary) + tally = _Tally(domain, parse_crs(dem_crs), vocabulary, request.threads) switches = (request.repair_m, request.merge_same_class, request.tolerance_m) tally.cleanup = request if any(switches) or request.outline_snap_m else None crss: list[str] = [] @@ -303,8 +305,11 @@ def open_features(request: FeatureRequest, domain: DomainPolygon, dem_crs: str | class _Tally: """What :func:`open_features` accumulates over its sources.""" - def __init__(self, domain: DomainPolygon, dem: CRS, vocabulary: EdgeVocabulary) -> None: + def __init__( + self, domain: DomainPolygon, dem: CRS, vocabulary: EdgeVocabulary, threads: int = 1 + ) -> None: self.domain, self.dem, self.vocabulary = domain, dem, vocabulary + self.threads = threads self.features: list[TerrainFeature] = [] self.outside = self.clipped = self.empty = 0 self.scanned: list[str] = [] @@ -336,7 +341,7 @@ def source(self, source: FeatureSource) -> tuple[str, str | None]: def _take(self, source: FeatureSource, own: str, rows: Iterable[tuple[Any, Any, Any]]) -> None: name, cmap = source.path.name, source.class_map src = parse_crs(own) - move = None if same_crs(src, self.dem) else reprojector(src, self.dem) + move = None if same_crs(src, self.dem) else reprojector(src, self.dem, threads=self.threads) bound, region = None, source_region(self.domain, self.dem, self.dem) shapely.prepare(region) if move is not None and src.is_geographic: diff --git a/src_python/tin_engine/final_check.py b/src_python/tin_engine/final_check.py index 08974f99..8fdc0ce8 100644 --- a/src_python/tin_engine/final_check.py +++ b/src_python/tin_engine/final_check.py @@ -37,13 +37,15 @@ def run( feet: bool = False, field: LineTolerance | None = None, slope: SlopeTolerance | None = None, + threads: int = 0, ) -> tuple[PointRefineOutcome, int]: """Phase 2 from phase 1's outcome `start`; the outcome and the number of check points stored. `clock` gets D7's rows. `strip`, the edge strip on the target grid (15f, D6), joins the same loop. `feet` puts points near a line onto it first (20c, R4). `field`, if any, sets each triangle's tolerance (increment 33); `slope`, if any, holds each check point to the - tolerance of its cell's slope too (increment 34).""" + tolerance of its cell's slope too (increment 34). `threads` goes to the + C++ as given (38; its 0 is every core).""" h = float(grid.spacing) store = CheckPoints( x_min=grid.col0 * h, y_max=-grid.row0 * h, spacing=h, rows=grid.rows, cols=grid.cols @@ -61,6 +63,7 @@ def run( store, *(np.asarray(a) for a in arrays), tolerance=tolerance, + threads=threads, strip=strip, constraint_feet=feet, field=field, diff --git a/src_python/tin_engine/io/cog.py b/src_python/tin_engine/io/cog.py index 86ae5d16..57545584 100644 --- a/src_python/tin_engine/io/cog.py +++ b/src_python/tin_engine/io/cog.py @@ -19,7 +19,6 @@ from __future__ import annotations -import os import threading from collections.abc import Sequence from concurrent.futures import ThreadPoolExecutor @@ -112,7 +111,7 @@ def decode_window( dtype: np.dtype[Any], window: IndexWindow, *, - threads: int | None = None, + threads: int = 1, ) -> DemTile: """`window` of the page `source` holds, decoded and cast to `dtype`. @@ -121,8 +120,9 @@ def decode_window( `source` lacks is `NotCached`, before any block is read; a sparse or undecodable block is a `GeoTiffError` naming `source.where` and the block. - `threads` decoding workers, the machine's cores (`os.cpu_count() or 1`) - when None; the count changes no value (W2). + `threads` decoding workers, one by default (38: the count is the caller's, + and no module but `resources.py` reads the machine); the count changes no + value (W2). """ w, page = window, source.page ends = (meta.rows - w.row0 - w.rows, meta.cols - w.col0 - w.cols) @@ -146,7 +146,7 @@ def decode_window( def decode_class_window( - source: BlockSource, meta: RasterMeta, window: IndexWindow, *, threads: int | None = None + source: BlockSource, meta: RasterMeta, window: IndexWindow, *, threads: int = 1 ) -> ClassWindow: """`window` of the class raster `source` holds, as `uint8` (26a). @@ -182,10 +182,10 @@ def _decode_into( w: IndexWindow, indices: Sequence[int], out: np.ndarray[Any, np.dtype[Any]], - threads: int | None, + threads: int, ) -> None: """Decode each of `indices` into its own slice of `out`, `w`'s canvas, on - `threads` workers (the machine's cores when None); none is sparse.""" + `threads` workers; none is sparse.""" page = source.page def put(index: int) -> None: @@ -200,7 +200,7 @@ def put(index: int) -> None: block ) - with ThreadPoolExecutor(threads if threads is not None else os.cpu_count() or 1) as pool: + with ThreadPoolExecutor(threads) as pool: list(pool.map(put, indices)) diff --git a/src_python/tin_engine/io/mesh_index.py b/src_python/tin_engine/io/mesh_index.py index e0360251..0c7fc5f6 100644 --- a/src_python/tin_engine/io/mesh_index.py +++ b/src_python/tin_engine/io/mesh_index.py @@ -34,7 +34,7 @@ class _Record(BaseModel): class PartitionRecord(_Record): - """The rule's inputs and its cells, as `decompose.partition` gave them.""" + """The rule's inputs and its cells, as the partition rule of increment 23c gave them.""" pieces: int = Field(ge=1) memory_budget: int = Field(ge=1) diff --git a/src_python/tin_engine/io/repository.py b/src_python/tin_engine/io/repository.py index db1798af..59a7479d 100644 --- a/src_python/tin_engine/io/repository.py +++ b/src_python/tin_engine/io/repository.py @@ -103,7 +103,9 @@ class TiffDemRepository: refused. `nodata` is the caller's sentinel for every tile (`decode_dem`). """ - def __init__(self, paths: Iterable[Path], *, nodata: float | None = None) -> None: + def __init__( + self, paths: Iterable[Path], *, nodata: float | None = None, threads: int = 1 + ) -> None: self._paths: dict[str, Path] = {} for path in sorted({Path(p).resolve() for p in paths}, key=str): if path.name in self._paths: @@ -111,11 +113,13 @@ def __init__(self, paths: Iterable[Path], *, nodata: float | None = None) -> Non f"two tiles named {path.name}: {self._paths[path.name]} and {path}" ) self._paths[path.name] = path - self._nodata = nodata + self._nodata, self._threads = nodata, threads self._footprints: tuple[TileFootprint, ...] | None = None @classmethod - def from_directory(cls, directory: Path, *, nodata: float | None = None) -> TiffDemRepository: + def from_directory( + cls, directory: Path, *, nodata: float | None = None, threads: int = 1 + ) -> TiffDemRepository: """The `*.tif` and `*.tiff` files directly in `directory`, not recursively. Side files (`.tfw`, `.aux.xml`) carry nothing the GeoTIFF tags do not. @@ -125,7 +129,7 @@ def from_directory(cls, directory: Path, *, nodata: float | None = None) -> Tiff ] if not paths: raise ValueError(f"no .tif or .tiff file in the directory {directory}") - return cls(paths, nodata=nodata) + return cls(paths, nodata=nodata, threads=threads) def footprints(self) -> tuple[TileFootprint, ...]: """Read every header once, on the first call, and keep them (R2).""" @@ -145,7 +149,8 @@ def load_window(self, name: str, window: IndexWindow) -> DemTile: def read(stream: Any, *, nodata: float | None) -> DemTile: meta, dtype, page = read_page(stream, nodata=nodata) - return decode_window(LocalTiffBlocks(page, stream, name), meta, dtype, window) + blocks = LocalTiffBlocks(page, stream, name) + return decode_window(blocks, meta, dtype, window, threads=self._threads) return self._read(name, read) @@ -232,8 +237,11 @@ class CacheRepository: manifest and each object's `header.bin`, and refuses a cache that is not the copy it claims to be (`CacheError`) or is absent (`NotCached`).""" - def __init__(self, root: Path, source: str, *, nodata: float | None = None) -> None: + def __init__( + self, root: Path, source: str, *, nodata: float | None = None, threads: int = 1 + ) -> None: self._directory, self._source, self._nodata = Path(root) / source, source, nodata + self._threads = threads self._pages: dict[str, tuple[RasterMeta, np.dtype[Any], tifffile.TiffPage]] = {} self._fetched: dict[str, datetime.date | None] = {} @@ -278,11 +286,12 @@ def load(self, name: str) -> DemTile: def load_window(self, name: str, window: IndexWindow) -> DemTile: meta, dtype, _ = self._page(name) - return decode_window(self._blocks(name), meta, dtype, window) + return decode_window(self._blocks(name), meta, dtype, window, threads=self._threads) def load_class_window(self, name: str, window: IndexWindow) -> ClassWindow: """`window` of the class raster `name` (26a), empty blocks as 0.""" - return decode_class_window(self._blocks(name), self._page(name)[0], window) + meta = self._page(name)[0] + return decode_class_window(self._blocks(name), meta, window, threads=self._threads) def class_strips(self, name: str, box: Bounds, *, rows: int) -> Iterator[ClassWindow]: """The cells of `name` whose interior meets `box`, clipped to the diff --git a/src_python/tin_engine/land_cover_fractions.py b/src_python/tin_engine/land_cover_fractions.py index 8e55bf96..d55c0cb3 100644 --- a/src_python/tin_engine/land_cover_fractions.py +++ b/src_python/tin_engine/land_cover_fractions.py @@ -58,13 +58,13 @@ def row_areas(window: ClassWindow) -> npt.NDArray[np.float64]: def index_space( - vertices: npt.ArrayLike, mesh_crs: str, meta: RasterMeta + vertices: npt.ArrayLike, mesh_crs: str, meta: RasterMeta, threads: int = 1 ) -> npt.NDArray[np.float64]: """Every vertex's `(col, row)` in the class raster `meta` describes: moved once to the raster's CRS, then `col = (lon - lon0) / s`, `row = (lat0 - lat) / s` with `(lon0, lat0)` the raster's upper-left cell corner (1.1).""" xy = np.asarray(vertices, dtype=np.float64)[:, :2] - lonlat = reprojector(mesh_crs, meta.crs)(xy) + lonlat = reprojector(mesh_crs, meta.crs, threads=threads)(xy) corner_x, corner_y = meta.x_min - meta.delta_x / 2, meta.y_max + meta.delta_y / 2 cols = (lonlat[:, 0] - corner_x) / meta.delta_x rows = (corner_y - lonlat[:, 1]) / meta.delta_y @@ -91,7 +91,7 @@ def assign( repository: CacheRepository, name: str, chunk: int = CHUNK, - threads: int = 0, + threads: int = 1, ) -> Assigned: """The exact class fractions of every triangle from the cached class raster `name` of `repository`, in chunks of `chunk` triangles in row order.""" @@ -100,7 +100,7 @@ def assign( raise ValueError(f"{name} is not in the cache") meta = metas[name] tri = np.asarray(triangles, dtype=np.int64).reshape(-1, 3) - corners = index_space(vertices, mesh_crs, meta)[tri] + corners = index_space(vertices, mesh_crs, meta, threads)[tri] parts: list[tuple[npt.NDArray[np.uint32], npt.NDArray[np.uint8], npt.NDArray[np.float64]]] = [] for start in range(0, len(tri), chunk): piece = corners[start : start + chunk] diff --git a/src_python/tin_engine/mcp/schema.py b/src_python/tin_engine/mcp/schema.py index d2da580b..df9cbca1 100644 --- a/src_python/tin_engine/mcp/schema.py +++ b/src_python/tin_engine/mcp/schema.py @@ -48,7 +48,8 @@ def _annotation(param_type: Any) -> tuple[Any, dict[str, Any]]: if hasattr(param_type, "types"): # a tuple type: fixed length, one type per place return tuple[tuple(_annotation(t)[0] for t in param_type.types)], {} # type: ignore[misc] bounds = {"ge": getattr(param_type, "min", None), "le": getattr(param_type, "max", None)} - return SCALARS.get(param_type.name.split()[0], str), {k: v for k, v in bounds.items() if v} + limits = {k: v for k, v in bounds.items() if v is not None} + return SCALARS.get(param_type.name.split()[0], str), limits def _default(param: Any) -> Any: diff --git a/src_python/tin_engine/mosaic.py b/src_python/tin_engine/mosaic.py index 3d9fcff1..42e1d873 100644 --- a/src_python/tin_engine/mosaic.py +++ b/src_python/tin_engine/mosaic.py @@ -17,7 +17,6 @@ from __future__ import annotations import math -import os from collections.abc import Callable, Sequence from dataclasses import dataclass from typing import Any @@ -28,6 +27,7 @@ from pydantic import BaseModel, ConfigDict from tin_engine.io.models import Bounds, DemTile, IndexWindow, RasterMeta, TileFootprint, valid_mask +from tin_engine.resources import Resources #: DEM units (metres for DTM10). A seam counts a node only where the two #: tiles differ by at least this (Ola, 2026-09-28: "Ignore below 1mm"). @@ -52,11 +52,6 @@ def __init__(self, message: str, tiles: tuple[str, ...] = ()) -> None: self.tiles = tiles -def physical_memory() -> int: - """Bytes of physical memory, looked up at call time (macOS and Linux).""" - return os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") - - class TilePlacement(BaseModel): """One selected tile: `canvas` in canvas indices, `source` in the tile's own, and the dtype its footprint says it decodes to.""" @@ -145,13 +140,19 @@ def meets(self, r0: int, c0: int, r1: int, c1: int) -> bool: def plan_mosaic( - footprints: Sequence[TileFootprint], bounds: Bounds | None = None, needed: Any = None + footprints: Sequence[TileFootprint], + bounds: Bounds | None = None, + needed: Any = None, + *, + memory_budget: int | None = None, ) -> MosaicPlan: """Select the tiles the request needs, on one lattice, or refuse (R4). `bounds` is the box in the DEM's CRS; without it, the lattice's union. `needed` is a shapely geometry in the DEM's CRS, closed: a node on its boundary is needed. Without it, every node of the window is. + `memory_budget` caps the canvas in bytes (38); None is half the + machine's memory (`Resources.fallback()`). When the request selects tiles on several lattices, the plan is on the one whose own tiles cover every needed node (Ola's Q5 reading, `_covering`). @@ -176,7 +177,8 @@ def plan_mosaic( if not chosen: raise MosaicError(f"the box {bounds} meets no tile") if len(chosen) > 1: - return plan_mosaic(_covering(chosen, bounds, needed), bounds, needed) + covering = _covering(chosen, bounds, needed) + return plan_mosaic(covering, bounds, needed, memory_budget=memory_budget) lattice = chosen[0] (x_ref, y_ref), (r0, c0, r1, c1), selected = lattice.reference, lattice.window, lattice.selected @@ -184,11 +186,13 @@ def plan_mosaic( if rows < 2 or cols < 2: raise MosaicError(f"the request is {rows} x {cols} nodes; a mesh needs at least 2 x 2") dtype = np.result_type(*(p.footprint.dtype for p in selected)) - cap, size = physical_memory() // 2, rows * cols * dtype.itemsize + cap = Resources.fallback().memory_budget if memory_budget is None else memory_budget + size = rows * cols * dtype.itemsize if size > cap: raise MosaicError( - f"the request is {rows} x {cols} nodes, {size} bytes at {dtype}, over the cap " - f"of half the physical memory ({cap} bytes); narrow it with --bbox" + f"the request is {rows} x {cols} nodes, {size} bytes at {dtype}, over the memory " + f"budget of {cap} bytes (set with --memory-budget; half the machine's memory when " + "it is not set); narrow it with --bbox" ) vertical = any(p.footprint.meta.vertical_unit_assumed for p in selected) meta = _grid(selected[0].footprint.meta, lattice.reference, lattice.window, vertical) diff --git a/src_python/tin_engine/resources.py b/src_python/tin_engine/resources.py new file mode 100644 index 00000000..f77e542a --- /dev/null +++ b/src_python/tin_engine/resources.py @@ -0,0 +1,108 @@ +"""What a run may use of the machine (increment 38): the one module that reads it. + +`docs/increments/38-threads-and-memory.md`, sections 4.1 to 4.3. The core +count, the physical memory and a container's cgroup limits are read here and +nowhere else; `Resources` carries the resolved figures, never 0, never None, +and every pool and `_core` call takes the one integer it uses. + +The cgroup root is `/sys/fs/cgroup`, as a container runtime mounts the +container's own cgroup. A limit set on an ancestor cgroup of a process on a +bare host (a systemd slice) is not read. Pydantic and `os` only: no `_core`, +no typer. +""" + +from __future__ import annotations + +import os +import re +from collections.abc import Mapping +from pathlib import Path +from typing import Self + +from pydantic import BaseModel, ConfigDict, Field + +CGROUP = Path("/sys/fs/cgroup") +VARIABLE = "RASPUTIN_THREADS" +_SUFFIX = {"": 1, "K": 1 << 10, "M": 1 << 20, "G": 1 << 30, "T": 1 << 40} + + +class Resources(BaseModel): + """What a run may use of the machine, resolved: never 0, never None.""" + + model_config = ConfigDict(frozen=True) + + #: The count every pool and `_core` call receives. + threads: int = Field(ge=1) + #: Bytes one DEM window may take, the flood's or the mosaic's. + memory_budget: int = Field(ge=1) + + @classmethod + def fallback(cls) -> Self: + """A caller that gave nothing: one thread, half the machine's memory.""" + return cls(threads=1, memory_budget=machine_memory() // 2) + + +def _read(path: Path) -> str: + """`path`'s text, stripped; empty when it is absent or unreadable.""" + try: + return path.read_text(encoding="ascii").strip() + except (OSError, UnicodeDecodeError): + return "" + + +def _whole(word: str) -> int | None: + """`word` as a whole number, decimal digits only, else None.""" + return int(word) if word.isascii() and word.isdigit() else None + + +def machine_cores(cgroup: Path = CGROUP) -> int: + """The cores this process may use (affinity counted where Python reports + it), capped by a cgroup v2 CPU quota: `/cpu.max` as `MAX PERIOD`, + `ceil(MAX / PERIOD)`, counted only when both are positive whole numbers. + Never under 1.""" + count = getattr(os, "process_cpu_count", os.cpu_count)() or 1 + words = _read(cgroup / "cpu.max").split() + if len(words) == 2 and (quota := _whole(words[0])) and (period := _whole(words[1])): + count = min(count, -(-quota // period)) + return max(1, int(count)) + + +def machine_memory(cgroup: Path = CGROUP) -> int: + """Physical memory in bytes, capped by the cgroup's limit: `memory.max` + (v2), else `memory/memory.limit_in_bytes` (v1). A file that is absent, + unreadable, reads `max` or holds no positive whole number is no limit; + v1's "no limit" value (about 2^63) loses to the physical figure.""" + physical = os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") + for name in ("memory.max", "memory/memory.limit_in_bytes"): + if limit := _whole(_read(cgroup / name)): + return min(physical, limit) + return physical + + +def parse_bytes(text: str) -> int: + """`text` as bytes: a whole number with an optional suffix K, M, G or T + (either case), 1024-based; anything else, or 0, is a `ValueError`.""" + found = re.fullmatch(r"([0-9]+)([KMGTkmgt]?)", text) + if found is None or int(found[1]) == 0: + raise ValueError( + f"{text!r} is not a number of bytes (a whole number above 0: 8G, 512M, 4096)" + ) + return int(found[1]) * _SUFFIX[found[2].upper()] + + +def resources( + threads: int | None, memory_budget: str | None, environ: Mapping[str, str] +) -> Resources: + """Sections 4.2 and 4.3: `--threads` (0 = every core), else `RASPUTIN_THREADS` + by the same rule (stripped; empty is unset), else every core; and + `--memory-budget`, else half the machine's memory. A `ValueError` names + the text it refuses.""" + if threads is None: + given = environ.get(VARIABLE, "").strip() + if given and _whole(given) is None: + raise ValueError( + f"{VARIABLE}={given} is not a whole number of threads (0 means every core)" + ) + threads = int(given) if given else 0 + budget = machine_memory() // 2 if memory_budget is None else parse_bytes(memory_budget) + return Resources(threads=threads or machine_cores(), memory_budget=budget) diff --git a/src_python/tin_engine/run_record.py b/src_python/tin_engine/run_record.py index 8057dbf0..da86e7a2 100644 --- a/src_python/tin_engine/run_record.py +++ b/src_python/tin_engine/run_record.py @@ -50,6 +50,7 @@ "tolerance_near_m": "Tolerance on the tolerance lines", "tolerance_ramp_m": "Distance from the lines over which the tolerance rises to its far value", "tolerance_lines": "The tolerance lines file, and its segments after simplifying", + "threads": "Threads used", "crs": "Coordinate system", "tolerance_m": "Tolerance", "max_error_m": "Largest height error", diff --git a/src_python/tin_engine/stats.py b/src_python/tin_engine/stats.py index 356f8d46..3123e5fb 100644 --- a/src_python/tin_engine/stats.py +++ b/src_python/tin_engine/stats.py @@ -239,7 +239,7 @@ def share(seconds: float) -> str: rows.append(("**total**", f"**{total:.3f}**", "**100 %**")) intro = _TIMINGS if report.threads is not None: - intro += f" Threads: {report.threads} (hardware concurrency)." + intro += f" Threads used: {report.threads}." lines = ["## Timings", "", intro, "", *_table(("phase", "seconds", "share"), rows), ""] if subs: lines += ["Sub-rows sum to their parent and are not added to the total.", ""] diff --git a/src_python/tin_engine/target_grid.py b/src_python/tin_engine/target_grid.py index 0e20fa14..0a9e9e27 100644 --- a/src_python/tin_engine/target_grid.py +++ b/src_python/tin_engine/target_grid.py @@ -178,7 +178,7 @@ def resample( def block(r0: int) -> None: r1 = min(r0 + rows_per, grid.rows) - lonlat = reprojector(grid.crs, m.crs)(grid.xy(r0, r1)) + lonlat = reprojector(grid.crs, m.crs, threads=1)(grid.xy(r0, r1)) row, col = m.index_of(lonlat[:, 0], lonlat[:, 1]) inside = (col >= 0) & (col <= m.cols - 1) & (row >= 0) & (row <= m.rows - 1) out = np.full(col.size, fill, dtype=np.float64) @@ -236,7 +236,7 @@ def check_point_blocks( def block(origin: tuple[int, int]) -> Block | None: r0, c0 = origin r1, c1 = min(r0 + BLOCK, m.rows), min(c0 + BLOCK, m.cols) - move = reprojector(m.crs, grid.crs) + move = reprojector(m.crs, grid.crs, threads=1) # 38: no pool inside a pool rows, cols = np.arange(r0, r1, dtype=np.float64), np.arange(c0, c1, dtype=np.float64) top, bottom = np.full(cols.size, rows[0]), np.full(cols.size, rows[-1]) left, right = np.full(rows.size, cols[0]), np.full(rows.size, cols[-1]) From 9de3212c276792654bd2027c9deda4c53ee3b54b Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 20:05:53 +0200 Subject: [PATCH 08/13] Increment 38 test-side fix round: the 17 old tests that pinned the pre-38 surface (@tester) After @developer's green b252a70d, every red test of 4b9c0dfe passes but the five layering rows, and 17 older tests failed on the test side. Each fixed in tests/ only, as docs/increments/38-threads-and-memory.md intends: - test_layering.py: `resources` added to the rows for cli, dem_input, catchment, catchment_batch and mosaic (5 tests). Checked with importscan against b252a70d: each module's first-party import set grew by exactly tin_engine.resources, and resources is L1, under all five. - test_cli_mesh_domain_crs.py: `as_16_opened_it(request, resources=None)` now stands in for `open_dem(request, resources)` and passes the second argument through (2 tests). - test_cli_mesh_plain_output.py: the bit-identity test runs `--threads 1` and `--threads 2` to the same paths instead of wrapping cli.refine with a colliding keyword; the mesh bytes are equal, the records differ only in the `threads` entry (1 and 2) and the echoed command line. The "threads is not a record key" assertion goes, as T6 pins the entry (2 tests). Two imports left unused by the rewrite removed. - test_mosaic.py: plan_mosaic's parameters gain memory_budget (1 test). - test_feature_input.py: the two `fake(src, dst)` reprojectors and `single_call_reprojector` accept `threads=1` (the fakes pass it to the real one; the pre-35 verbatim copy ignores it) (3 tests). - test_target_grid.py: `_SpiedReprojector.__call__` accepts `threads` and passes it through (4 tests). - test_mcp_schema.py: T9's docstring no longer says "no change to mcp/"; it names the one line kept (`minimum: 0` is a bound). No red scaffolding of 38's remained (the `importorskip("mcp")` and the test_mosaic docstring predate it). Full `pytest tests/python/` with the coverage gate: 7422 passed, 1 failed, 18 skipped, 98.44% coverage (floor 85%). The one failure, test_core_cdt.py::TestGil::test_the_ticker_distinguishes_a_held_gil_from_a_released_one ("a GIL-holding call let 7 ticks through", bound 3), ran beside the three detached `rasputin mesh` processes; alone it passed 5 of 5 runs. Untouched by 38; a load-sensitive bound, reported in the handback. `ruff check tests/` and `ruff format --check tests/`: clean. Co-Authored-By: Claude Fable 5.1 --- tests/python/test_cli_mesh_domain_crs.py | 8 +++-- tests/python/test_cli_mesh_plain_output.py | 35 ++++++++++------------ tests/python/test_feature_input.py | 14 +++++---- tests/python/test_layering.py | 11 +++---- tests/python/test_mcp_schema.py | 6 ++-- tests/python/test_mosaic.py | 3 +- tests/python/test_target_grid.py | 6 ++-- 7 files changed, 43 insertions(+), 40 deletions(-) diff --git a/tests/python/test_cli_mesh_domain_crs.py b/tests/python/test_cli_mesh_domain_crs.py index bd8ef427..496bcd87 100644 --- a/tests/python/test_cli_mesh_domain_crs.py +++ b/tests/python/test_cli_mesh_domain_crs.py @@ -82,6 +82,7 @@ from tin_engine.domain import DomainPolygon from tin_engine.io.domain_file import read_domain from tin_engine.io.geotiff import decode_dem +from tin_engine.resources import Resources from vtkread import VtkFile SNAP = 1e-3 # DEFAULT_SNAP_SPACING: the noder's grid, applied in the DEM's CRS (R9) @@ -282,11 +283,12 @@ def spy(*args: Any, **kwargs: Any) -> Any: return seen @staticmethod - def as_16_opened_it(request: DemRequest) -> DemInput: + def as_16_opened_it(request: DemRequest, resources: Resources | None = None) -> DemInput: """16's data flow for one file and a domain: the whole file, opened as without a domain, and the domain object as `read_domain` returned it. - Neither `DomainPolygon.to_crs` nor the window cut to the domain runs.""" - opened = open_dem(request.model_copy(update={"domain": None})) + Neither `DomainPolygon.to_crs` nor the window cut to the domain runs. + `resources` is `open_dem`'s second parameter (38), passed through.""" + opened = open_dem(request.model_copy(update={"domain": None}), resources) return dataclasses.replace(opened, domain=request.domain) def assert_as_16( diff --git a/tests/python/test_cli_mesh_plain_output.py b/tests/python/test_cli_mesh_plain_output.py index de675e2c..40c8b9ab 100644 --- a/tests/python/test_cli_mesh_plain_output.py +++ b/tests/python/test_cli_mesh_plain_output.py @@ -35,13 +35,11 @@ import socket import sys from pathlib import Path -from typing import Any import numpy as np import pytest from typer.testing import Result -import tin_engine.cli as cli from cli_driver import COLS, ROWS, SQUARE, USAGE, geojson, plain, runner, write_tiff from feature_fixtures import Feat, write_geojson from geotiff_fixtures import micro_tiff @@ -554,23 +552,20 @@ def test_one_thread_gives_the_same_bytes( self, tmp_path: Path, holed: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """Refinement is bit-identical for any thread count (increments 14 and - 21), and the record holds no thread count: ``cli.refine`` wrapped to - pass ``threads=1`` (the binding's keyword; the CLI has none) changes - no byte.""" - args = ("--dem", str(holed), "--tolerance", "1") - _, _, _, rec = record_run(monkeypatch, tmp_path, *args) - first = rec.read_bytes() - real = cli.refine - seen: list[int] = [] - - def one_thread(*a: Any, **k: Any) -> Any: - seen.append(1) - return real(*a, **k, threads=1) - - monkeypatch.setattr(cli, "refine", one_thread) - record_run(monkeypatch, tmp_path, *args) - assert seen == [1], "the wrapper did not run" - assert rec.read_bytes() == first + 21): ``--threads 1`` and ``--threads 2`` (38) write the same mesh + bytes to the same paths, and records that differ only where they must, + the ``threads`` entry reporting the count used (38, T6) and the echoed + command line carrying the option.""" + args = ("--dem", str(holed), "--tolerance", "1", "--threads") + _, out, _, rec = record_run(monkeypatch, tmp_path, *args, "1") + one_mesh, one_record = out.read_bytes(), json.loads(rec.read_text(encoding="ascii")) + record_run(monkeypatch, tmp_path, *args, "2") + two_mesh, two_record = out.read_bytes(), json.loads(rec.read_text(encoding="ascii")) + assert two_mesh == one_mesh + assert (one_record.pop("threads"), two_record.pop("threads")) == (1, 2) + one_command, two_command = one_record.pop("command"), two_record.pop("command") + assert two_command == one_command.replace("--threads 1", "--threads 2") != one_command + assert two_record == one_record def test_no_time_date_host_or_seconds( self, tmp_path: Path, holed: Path, monkeypatch: pytest.MonkeyPatch @@ -583,7 +578,7 @@ def test_no_time_date_host_or_seconds( assert "seconds" not in text.lower(), text assert not re.search(r"\b\d{4}-\d{2}-\d{2}\b|\b\d{1,2}:\d{2}(:\d{2})?\b", text), text assert not host or host not in text, text - for absent in ("threads", "total", "stats_seconds"): + for absent in ("total", "stats_seconds"): assert absent not in obj def test_the_path_is_echoed_after_the_others( diff --git a/tests/python/test_feature_input.py b/tests/python/test_feature_input.py index 9d91831e..372528de 100644 --- a/tests/python/test_feature_input.py +++ b/tests/python/test_feature_input.py @@ -550,8 +550,8 @@ def test_a_vertex_with_no_image_is_refused_naming_the_feature( laea = moved(Polygon([at(100, 100), at(200, 100), at(150, 150), at(100, 200)]), UTM33, LAEA) marker = laea.exterior.coords[2] # in the source CRS - def fake(src: Any, dst: Any) -> Any: - move = real(src, dst) + def fake(src: Any, dst: Any, *, threads: int = 1) -> Any: + move = real(src, dst, threads=threads) if CRS.from_user_input(dst) != CRS.from_user_input(UTM33): return move @@ -1591,8 +1591,8 @@ def counting_reprojector(fi: ModuleType, monkeypatch: pytest.MonkeyPatch) -> lis calls: list[int] = [] real = crs_module.reprojector - def fake(src: Any, dst: Any) -> Any: - move = real(src, dst) + def fake(src: Any, dst: Any, *, threads: int = 1) -> Any: + move = real(src, dst, threads=threads) def apply(xy: Any) -> np.ndarray: calls.append(len(np.asarray(xy, dtype=np.float64).reshape(-1, 2))) @@ -1809,8 +1809,10 @@ def untouched_30b(line: Any, domain: Polygon) -> bool: ) -def single_call_reprojector(src: Any, dst: Any) -> Any: - """`crs.reprojector` before 35, verbatim: one `transform` call.""" +def single_call_reprojector(src: Any, dst: Any, *, threads: int = 1) -> Any: + """`crs.reprojector` before 35, verbatim: one `transform` call. `threads` + is accepted as the call sites pass it (38) and unused: one call, no pool.""" + del threads import tin_engine.crs as crs_module transformer = Transformer.from_crs( diff --git a/tests/python/test_layering.py b/tests/python/test_layering.py index 450ced00..69b64262 100644 --- a/tests/python/test_layering.py +++ b/tests/python/test_layering.py @@ -36,7 +36,7 @@ }, { # L1: pure algorithms "crs": "io.models run_record", - "mosaic": "io.models", + "mosaic": "io.models resources", "target_grid": "crs domain grow io.models", "grid_domain": "io.models", "domain": "crs io.models", @@ -78,12 +78,12 @@ "_core": "", }, { # L4: pipelines - "dem_input": "crs domain grow io.models io.repository mosaic target_grid", + "dem_input": "crs domain grow io.models io.repository mosaic resources target_grid", "feature_input": "border_simplify crs domain features io.geojson io.geopackage io.gml" " io.repository", "catchment": "burn catchment_core crs gauge io.models io.repository mosaic outline" - " sensitivity", - "catchment_batch": "catchment crs gauge hydrography io.repository reference", + " resources sensitivity", + "catchment_batch": "catchment crs gauge hydrography io.repository reference resources", "fetch": "", "fetch.plan": "crs domain fetch.http io.cog io.geotiff io.models", "fetch.run": "crs fetch.http fetch.plan io.models io.repository sources tin_engine", @@ -97,7 +97,8 @@ " io.geopackage_write io.mesh_checks io.models io.ply io.repository io.rivers" " io.station_set" " io.vtk_legacy land_cover_fractions landcover mcp.server mosaic palettes raster" - " run_record sources stats target_grid tin_engine tolerance_field viz.fixtures" + " resources run_record sources stats target_grid tin_engine tolerance_field" + " viz.fixtures" " viz.protocols viz.scene viz.style viz.svg", "__main__": "cli", # 37: `python -m tin_engine`, the server's child "mcp": "", "mcp.schema": "", # 37: the MCP server over the commands diff --git a/tests/python/test_mcp_schema.py b/tests/python/test_mcp_schema.py index d4c4ea8c..b88cdcbe 100644 --- a/tests/python/test_mcp_schema.py +++ b/tests/python/test_mcp_schema.py @@ -319,8 +319,10 @@ def test_an_integer_range_carries_its_minimum(self) -> None: def test_t9_threads_and_memory_budget_derive_from_the_cli(self) -> None: """Increment 38, T9: the two options reach the `mesh`, `catchment` and - `station_catchments` tools through the derived schema, with no change - to `mcp/`; `argv` puts them on the command line as the CLI's words.""" + `station_catchments` tools through the derived schema; `argv` puts + them on the command line as the CLI's words. The `minimum: 0` bound + is kept: a bound of zero is a bound (`schema.py` dropped every falsy + bound before 38, the one `mcp/` line the increment changes).""" tools = derived(app) for name in ("mesh", "catchment", "station_catchments"): properties = tools[name].tool().input_schema["properties"] diff --git a/tests/python/test_mosaic.py b/tests/python/test_mosaic.py index c1f112f8..239b6a0c 100644 --- a/tests/python/test_mosaic.py +++ b/tests/python/test_mosaic.py @@ -1800,4 +1800,5 @@ def test_the_plan_never_loads(mz: ModuleType, footprint: Any) -> None: """I6 as a type fact: `plan_mosaic` takes footprints, and no `load`.""" import inspect - assert list(inspect.signature(mz.plan_mosaic).parameters) == ["footprints", "bounds", "needed"] + parameters = ["footprints", "bounds", "needed", "memory_budget"] # 38: the cap + assert list(inspect.signature(mz.plan_mosaic).parameters) == parameters diff --git a/tests/python/test_target_grid.py b/tests/python/test_target_grid.py index 4e7dc079..7fa1b891 100644 --- a/tests/python/test_target_grid.py +++ b/tests/python/test_target_grid.py @@ -492,13 +492,13 @@ class _SpiedReprojector: number of points it is given. One transformer per block (J5), so the record is one entry per block.""" - def __init__(self, real: Callable[[str, str], Callable[[Any], Any]]) -> None: + def __init__(self, real: Callable[..., Callable[[Any], Any]]) -> None: self._real = real self._lock = threading.Lock() self.lengths: list[int] = [] - def __call__(self, source: str, target: str) -> Callable[[Any], Any]: - transform = self._real(source, target) + def __call__(self, source: str, target: str, *, threads: int = 1) -> Callable[[Any], Any]: + transform = self._real(source, target, threads=threads) def recording(points: Any) -> Any: with self._lock: From 8f9289c49f69b95e6e9207a3b2f80ab2467d3351 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 20:07:03 +0200 Subject: [PATCH 09/13] 38 as built: red, green, test fixes, size and pins recorded, with the Status line and ROADMAP row, by the main session Co-Authored-By: Claude Fable 5.1 --- ROADMAP.md | 2 +- docs/increments/38-threads-and-memory.md | 80 +++++++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index ebeb2bc7..e41a4ae4 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -58,7 +58,7 @@ increment that most needs a picture to check against | 32 | Land-cover borders simplified within a band, each class keeping its area exactly (Ola, 2026-10-08: "we need to implement is a CORINE simplification, similar to what we do on the outline of the auto-catchments"): the land-cover coverage cut into borders between junctions, each border reduced by the area-preserving segment collapse of the catchment outline (`reduce_ring`), with junctions and the domain outline fixed, a crossing test and a side test across all borders, and a band no border moves past (two-sided, proved by anchors on the source border). `--features-tolerance` becomes that band, default 50 m (half CORINE's positional accuracy), replacing GEOS's coverage simplifier; start angle 15° when land cover is simplified (band above 0), 25° otherwise. Prototype on the German fused catchment: 5.7 times fewer triangles at 50 m vertical tolerance | **merged (#222, 2026-10-11)** (green `82c0a7d4`, 519 net production lines, one PR; mutation round `30f939b6`, one mutant recorded as not killed); Ola ruled all four questions (2026-10-08); code review round 1 asked for changes (comments, docstrings and `project_structure.md` the change made false), fixed by `4d4f0411` and the docs commit after it; code review rounds 1-3 done (round 3 approved); `@perf`'s short timing check found the runs about 70 % slower (the outline rule on borders clipped to the catchment) and the worst angle far lower (the simplifier placing points millimetres from junctions); fix built (section 15: outline rule first, a 1 m clearance in the simplifier; green `05c8f73a`, 573 counted lines in all; mutation round M7 to M13 killed); its quick check `b192a2c6`: Numedalslagen −11 %, Lagan −1.3 %, outline rule and moved area as before, triangles −42 %; questions 5 and 6 decided on default while Ola was away, reversible; fix code review round 1 asked for prose fixes and the test file's red-step staging removed; fix code review round 3 approved; next the push on Ola's yes | `docs/increments/32-landcover-simplify.md` | | 36 | GeoPackage output, so QGIS opens a mesh as it opens any vector layer: `rasputin mesh --out X.gpkg`, chosen by the suffix of `--out` as `.vtk` and `.ply` are (Ola, 2026-10-05: "Is the vtk_to_phkg.py the way to go, or as a direct output from rasputin? I think the latter, and it should go into the roadmap and eventually into the codebase."). Proposed content: a `triangles` layer, one PolygonZ per triangle in the run's CRS (z at each of the three corners, as the stopgap writes), with the attributes the run already knows: land-cover code and class name (the code system's names, as `rasputin palette` has them), mean z, plan area; a layer of the constraint polygons (the `--features` polygons clipped to the domain, with their code); optionally a QGIS style for land cover stored in the file (the `layer_styles` table), so the triangles open coloured. Land-cover colour: when the mesh carries CORINE classes, each triangle and each constraint polygon also gets its class's colour as a `#rrggbb` text column, so a reader can colour by the column without the stored style (in QGIS, a data-defined fill colour). The Graz file of 2026-10-05 carries `clc_color` on its `triangles` and `corine` layers; its stored styles categorise on `clc_code` and do not read the column. The colours are those of `rasputin palette` (`src_python/tin_engine/palettes.py`), the same 26 the Graz file carries: natural colours, as Ola chose over the official CLC legend in 16c's ruling 2 (that legend paints peat bogs blue and the sea almost white). **One attribute is not in the mesh yet**: the ID of the source polygon each triangle lies in (a sub-catchment cell, a parcel). Today only a class code labels triangles, one code system per mesh (16c, 16e's D1); the stopgap finds the cell by a centroid lookup outside rasputin. It needs a per-triangle array from a polygon source's ID property: its own small step, done just before this writer, because the `.vtk` and `.ply` outputs gain the attribute too (Ola, 2026-10-05, accepting the default: "defaults on the cell ID"). Written with the standard library's `sqlite3` beside the existing reader `src_python/tin_engine/io/geopackage.py`, which can read it back in the tests; no GDAL, OGR or Fiona (CLAUDE.md §2). Python only: the C++ core is unchanged and never sees the file (the I/O boundary). Prior art and working spec: Ola's stopgap `vtk_to_gpkg.py`, outside the repository (`rasputin_data/lands/scripts/`), which converts a mesh `.vtk` with `sqlite3`, NumPy, Shapely and PyProj, no GDAL, and its first use, the Mur above Graz (2 451 734 triangles, 10 m DEM, tolerance 10 m, 2026-10-05): **the GeoPackage was 677.9 MB against 110.6 MB for the binary `.vtk`**; the whole stopgap run took 39 s and 4.1 GB peak, and that includes the centroid cell lookup and the CORINE clipping, so it is not the cost of the GeoPackage encoding alone: the design measures the writer on its own. A GeoPackage row holds the triangle's whole ring (four corners, the first repeated) and a bounding box, and the file adds a spatial index, where `.vtk` stores each vertex once. Questions for the design: file size (keep the spatial index or not; at most an opt-in variant of 2D polygons with mean z as an attribute, proposed default: full PolygonZ; writing in chunks to bound memory); the CRS entry for a CRS with no EPSG code; where the run record (increment 25) goes; whether the constraint lines get a layer of their own | **merged (#231)** (branch `worktree-gpkg-out`: red `87f41c3d`, green `c59246e5`, fix-up `7085dddf`, 377 net production lines, mutation round `dddce8e8` ten of ten killed; `@perf`'s acceptance `57c25074` ACCEPTED: Numedalslagen 3.93 s / 203 MB, Romsdalen 19.4 s / 810 MB, every mesh byte-identical; code review round 1's one blocker, an empty land-cover layer crashing the write, fixed by `55cc3c3e` + `18442b27`, 378 net lines; code review round 2 APPROVED; merged 2026-10-12). Designed by `@architect`, 2026-10-11, on `81a551b3`; proposed by Ola 2026-10-05, "yes, roadmap row now, build after PR D"; PR D merged as #205, so the writer joins the encoders under `io/`). Measured on this design's probe: the quick check's Numedalslagen mesh (653,039 triangles) written in 4.0 s to 203 MB against 0.28 s and 29 MB for the binary `.vtk`, the stopgap 5.1 s; the Romsdalen slope mesh (3.1 million) in 21.5 s to 922 MB at 2.1 GB peak, the stopgap 28.7 s at 4.1 GB; the R-tree index is 60 % of the time; the same run writes the same bytes. Layers `triangles` (PolygonZ with code, name, colour, z_min, z_max, z_mean, area_m2, slope_deg), `lines`, `land_cover`, `land_cover_classes`, a `run` table with the run record, and the stored QGIS style; about 305 net lines, one PR; questions 1-5 answered with their defaults while Ola is away | `docs/increments/36-geopackage-output.md` | | 37 | An MCP (Model Context Protocol) server over the rasputin commands, so an external agent (Codex, Claude Desktop, ChatGPT, Mistral) discovers and calls rasputin as tools instead of reading its help text: `rasputin mcp` serves over stdio, one tool per command with a JSON schema derived from the Typer command's own parameters (one source of truth, so schema and CLI cannot drift), each call run as a child `python -m tin_engine ` so a cancel kills it and a crash ends only the child, with the exit code, output paths, run record and `--stats` returned as data and a refusal returned as a structured error carrying the CLI's plain words and exit code (Ola, 2026-10-11: "my working name for what's we're doing is intent-driven triangulation. This means accessing the tools using agents and intent"; "Add an MCP server to the ideas file after GeoPackage"; 2026-10-12: "Rule the mcp SDK: yes, use it"). Built on the official `mcp` Python SDK 2.3 as an optional extra `rasputin[mcp]`; stdio first, Streamable HTTP later or never; no Tasks extension (the SDK has none server-side); a jobs cap, default 1; the GeoPackage output (36) is the mesh an agent reads | **built and review-approved** (branch `worktree-mcp-server`: design `b990e38c` approved in three review rounds, red `ee5b5d52` 62 tests, green `851592ba`, three code review rounds with a README section, an `error`-field fix and a help-text escape, master merged in as `030fd32b`, 328 net production lines against 260; `dev` pulls the `mcp` extra; next the push on Ola's yes). Designed (`@architect`, 2026-10-12, on `b4a81c0d`): about 250 counted lines, one PR; nine decisions with defaults for Ola (section 6); measured by the design's probe: a tool call costs 0.25 s over the command itself (the child's import of `tin_engine.cli`), the 1 m benchmark mesh 0.72 s through the tool, a cancelled child dead within 0.5 s over stdio and in process, unknown and mistyped arguments refused before any child runs; no refine or mesh code, so no `@perf` run | `docs/increments/37-mcp-server.md` | -| 38 | Threads and memory as settings, the first fix from the generality audit (Ola, 2026-10-12: "the number of cores should of course not be hardcoded"): `--threads N` on `mesh`, `catchment` and `station-catchments` (0 = every core; `RASPUTIN_THREADS` is the default when the option is absent; neither set = every core, a container's CPU quota counting), carried as one `Resources` model built once in `cli.py` to every `_core` call that takes `threads` (five: `refine`, `refine_strip`, `refine_points`, `SlopeTolerance`, `class_areas`) and every Python pool (three: the reprojector, whose fixed cap of 8 goes, the target grid's resample and check points, the block decoder); the stats file and run record print the count used; `--memory-budget 8G` caps the mosaic and flood windows, default half the machine's memory with a container's cgroup limit (`memory.max`, v1 `memory.limit_in_bytes`) counting as the machine's; the unused `decompose.py` deleted with its test. Python only, no C++ change, about 60 net lines | designed (`docs/increments/38-threads-and-memory.md`) | `docs/increments/38-threads-and-memory.md` | +| 38 | Threads and memory as settings, the first fix from the generality audit (Ola, 2026-10-12: "the number of cores should of course not be hardcoded"): `--threads N` on `mesh`, `catchment` and `station-catchments` (0 = every core; `RASPUTIN_THREADS` is the default when the option is absent; neither set = every core, a container's CPU quota counting), carried as one `Resources` model built once in `cli.py` to every `_core` call that takes `threads` (five: `refine`, `refine_strip`, `refine_points`, `SlopeTolerance`, `class_areas`) and every Python pool (three: the reprojector, whose fixed cap of 8 goes, the target grid's resample and check points, the block decoder); the stats file and run record print the count used; `--memory-budget 8G` caps the mosaic and flood windows, default half the machine's memory with a container's cgroup limit (`memory.max`, v1 `memory.limit_in_bytes`) counting as the machine's; the unused `decompose.py` deleted with its test. Python only, no C++ change, about 60 net lines | **built** (branch `worktree-threads`: design `9e0d60c2` approved in two review rounds; red `4b9c0dfe` 110 tests, green `b252a70d`, test fixes `9de3212c`, 96 net production lines against 60; next the quick check and code review). Designed by `@architect`, 2026-10-12, as the generality audit's first fix | `docs/increments/38-threads-and-memory.md` | | — | A missing station number refused in plain words. In a reference-polygon file (`rasputin station-catchments --reference`), a feature whose `station` is null becomes the text "None" (`src_python/tin_engine/io/station_set.py@ed12512:89`). No station can match it, since a station number must look like `2.11.0` (`src_python/tin_engine/hydrography.py@ed12512:40`), so the polygon is dropped without a word and the station it belonged to is written with no comparison (`src_python/tin_engine/catchment_batch.py@ed12512:220`); two such polygons are refused as "station None appears twice". In a stations file a null number is refused, but in Pydantic's validation text rather than a sentence naming the file (`src_python/tin_engine/io/station_set.py@ed12512:79`). NVE's files have a number on every station, so only hand-made or third-party files are hit. The fix: both readers refuse a null `station`, naming the file; one or two lines. Plainer GeoJSON error messages in general are not planned until other people feed rasputin their own files | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | A GeoTIFF whose coordinate-system GeoKeys hold garbage refused in plain words. The reader turns each code key into a whole number with `int()`, and a key stored as a double or as text can hold NaN, infinity or letters. Probed 2026-10-06 on hand-made files: NaN or text in `ProjectedCSTypeGeoKey` (3072) stops `rasputin mesh --dem` with Python's conversion message (for NaN, "Invalid value for --dem: cannot convert float NaN to integer"), which names neither the key nor the file, and infinity ends in a Python traceback (`OverflowError`) (`src_python/tin_engine/io/geotiff.py@ed12512:171`). The same holds for every other code key the reader reads: 1025, 4099, 3076, 3074, 3075, 2048 and 2054 (`src_python/tin_engine/io/geotiff.py@ed12512:310`, `src_python/tin_engine/io/geotiff.py@ed12512:176`, `src_python/tin_engine/io/geotiff.py@ed12512:339`, `src_python/tin_engine/io/geotiff.py@ed12512:352`, `src_python/tin_engine/io/geotiff.py@ed12512:362`, `src_python/tin_engine/io/geotiff.py@ed12512:499`, `src_python/tin_engine/io/geotiff.py@ed12512:423`). Worse, a fractional value is cut to a whole number first: 2.5 in 4099 is refused as 2, and 32633.5 in 3072 is read as EPSG:32633 without a word (`src_python/tin_engine/io/geotiff.py@ed12512:321`); 1.5 in 1025, 9001.7 in 4099 and 9001.5 in 3076 are likewise accepted as 1 and 9001 (probed by code review round 1 on this row's branch, exit 0). The parameter keys are already checked (`_one_number`, `src_python/tin_engine/io/geotiff.py@ed12512:454`). The fix: one check for every code key, a whole number or a refusal naming the key and its value. No real file is known to do this | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | tifffile's warning about the NoData tag silenced. On a float32 DEM whose `GDAL_NODATA` (42113) is the lowest float32 written in 17 digits (`-3.4028234663852886e+38`), tifffile prints "parsing GDAL_NODATA tag raised ValueError: ... is not castable to float32" to stderr when the file is opened (`src_python/tin_engine/io/geotiff.py@ed12512:146`). It is noise: rasputin reads the tag itself and accepts the value, since it is that float32 exactly (`src_python/tin_engine/io/geotiff.py@ed12512:535`). The fix: silence that one tifffile message while the reader opens the file, leaving every other tifffile warning visible | approved by Ola 2026-10-06; not designed (a small PR) | - | diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index 4fc9d585..fda68111 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); next `@developer`'s green step;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); green `b252a70d` (`@developer`, 96 net production lines by `python3 tools/count_loc.py 5d2e706d HEAD` against the estimate of 60, under the worst case of 120); test-side fixes `9de3212c` (`@tester`); section 12 is the as-built record; next the quick check (bit-identical meshes) once the machine is idle, then code review;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds @@ -673,6 +673,84 @@ agent now sets `threads` per call through the derived schema, and for the handback: `tools/bench.py`'s child could pass `--threads` to the CLI and drop its monkeypatch of `refine` (tooling, not counted). +## 12. As built (recorded by the main session from the handbacks) + +Ola picked this as the audit's first fix on 2026-10-12 ("1, 2: yes") and +ruled the four design questions the same day ("Yes to all, but thousands of +sub-catchments is quite extensive", section 10). The design took two review +rounds; round 1's five blockers were figures and call sites, settled by +probing the fixture through a whole `delineate` run rather than its first +window. + +**Red** `4b9c0dfe` (`@tester`): 110 tests, 85 in the new +`tests/python/test_resources.py` and `tests/python/test_cli_threads.py` +(the option and its environment default, the model reaching every call site +through one spy per site, the cgroup reads on synthetic files with the +malformed cases, the memory budget with the probed figures, the stats row), +25 in nine edited files; `test_decompose_partition.py` deleted. Every red +reason a missing thing: the module `tin_engine.resources`, the two options, +a `plan_mosaic` keyword, a `reprojector` keyword, the old wording, the 8- and +10-worker defaults, the layering row. Pins beyond the design, asserted: +`RASPUTIN_THREADS` "1.5" and "0x10" refused as not whole numbers; a negative +`cpu.max` quota means no quota; `parse_bytes` refuses "G" and "1.5G", reads +"2k" as 2048; the three commands' help names `RASPUTIN_THREADS`; `threads` is +a record entry and an Inputs row, not a mesh-file field; the flood and +mosaic refusals name `--memory-budget` and the figure. T11's assertion on +`project_structure.md` left out: a prose file, forbidden to tests by +`conftest.py`; the reviewer reads the rows. One citation into the deleted +test file (`docs/increments/23-basin-scale.md`, a 23c-1 review record) was +pinned at `5d2e706d` by the main session (`3b709b6f`). + +**Green** `b252a70d` (`@developer`): `src_python/tin_engine/resources.py` +new (the `Resources` model: threads from `--threads`, 0 meaning all cores, +`RASPUTIN_THREADS` as the default when the option is absent, the machine's +cores otherwise with a well-formed `cpu.max` quota counted; the memory +budget from `--memory-budget` with `memory.max` read when present); +`--threads` and `--memory-budget` on `mesh`, `catchment` and +`station-catchments`; the count reaching all five `_core` entry points and +the three Python pools, the reprojector's cap of 8 gone and the pools' +library defaults set to one worker so only `resources.py` reads the machine; +both memory caps overridable by the budget; the stats row "Threads used: N." +and the `threads` record entry; `decompose.py` deleted; README, INSTALL and +`project_structure.md` updated. 96 net production lines (237 added, 141 +removed; the overrun over 60 is `cli.py`'s option aliases and `_resources` +helper and the two repositories' `threads`). Gates clean: mypy, ruff, +prohibited deps, detria boundary, check_citations. Two fixes after the first +full run: the `--threads` metavar `N`, since `` made Rich truncate +long option names in `--help`; `_domain_plan`'s budget defaulting to None. +Pins beyond the design: `mcp/schema.py` changed by one line (`minimum: 0` no +longer dropped by `if v`), required by T9 although section 4.5 said no `mcp/` +change (a latent bug for any option with a zero bound; `@architect`'s +one-line correction of 4.5 is due); the `_core` wrappers `edge_strip.run` and +`final_check.run` default `threads=0` with the C++ meaning while the Python +pools default 1, not pinned by a test (`@tester` recommends 1, matching +section 4.2; default kept at 0, documented in the two docstrings; for the +code review to judge). + +**Test-side fixes** `9de3212c` (`@tester`, seven files): the five layering +rows gain `resources` (each module grew by exactly `tin_engine.resources`, +checked with `importscan`); `as_16_opened_it` passes `resources` through; +the plain-output bit-identity test runs `--threads 1` and `--threads 2` to +the same paths (equal mesh bytes, records differing only in `threads` and +the echoed command); `plan_mosaic`'s parameter list gains `memory_budget`; +the feature-input fakes and the spied reprojector accept `threads=`; T9's +docstring no longer claims no `mcp/` change; no scaffolding left. Full +`pytest tests/python/` with the coverage gate: 7,422 passed, 18 skipped, +coverage 98.44 %, one failure beside the night's detached mesh batch +(`test_core_cdt.py`'s GIL ticker, 7 ticks against a bound of 3, load-bound; +5 of 5 alone, untouched by this increment). `ruff` clean on `tests/`. + +**Speed:** no refine or mesh code touched; the quick check for bit-identical +meshes runs once the machine is idle (the Swedish batch holds three cores +tonight); its verdict goes here. + +**Lessons for the next retrospective:** a design that deletes a file lists +the unpinned citations into it first (`check_citations` would have shown the +23c-1 record); a red step that adds a keyword to a shared function widens +the suite's existing fakes of it in the same commit; the editable install's +loader keeps a deleted module's `find_spec` until reinstall; the GIL ticker +test's bound of 3 ticks is load-sensitive. + ## Review **Design review, round 1, 2026-10-12.** Range `5d2e706d..9e0d60c2`, 0 production lines, estimate 60 net with worst case 120, @reviewer: CHANGES REQUESTED, five blockers. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/docs/increments/38-threads-and-memory.md@9e0d60c2:476-481: T10's figures are wrong; probed on the fixture, the first window is 49 x 49, the mosaic cap 9,604 bytes and the flood 14,406, so a budget of 8,192 is refused by the mosaic cap (`/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/catchment.py@5d2e706d:394`) naming 9,604, not the flood, and 13,824 also refuses; fix: a budget in 9,604 to 14,405 for the flood case, 14,406 for "exactly at", the `8K` CLI case restated. (2) @9e0d60c2:485-488: T11 misses `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/tests/python/test_layering.py@5d2e706d:44` (the `decompose` row), whose line 152 fails on a gone module; the red commit drops that row and adds `resources`. (3) @9e0d60c2:457-470: T8 spies no reprojector at `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/feature_input.py@5d2e706d:339` or `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`, two of the three large sites; with the library default of one worker a dropped `threads` serialises them unseen; one spy per site. (4) @9e0d60c2:133-141: 15 reprojector callers, not 14 (`target_grid.py:136`, `tolerance_field.py:83`, `feature_input.py:183` missing; "other ten" is twelve); `target_grid.py:239`'s blocks are 512 x 512 = 262,144 points, under the 500,000 threshold, so only line 181 nests a pool. (5) @9e0d60c2:177-181: `cpu.max` with a non-numeric first word or a zero period is unspecified; apply the memory rule (no quota). Suggestions: the refusal text "half the machine's memory" (:254-256) is false when a budget was given; section 4.5 should say `bench.py` children's stats row still shows the CLI's count; T9's `argv` is `DerivedCommand.argv(model)` (`src_python/tin_engine/mcp/schema.py@5d2e706d:108`). Verified: all 37 pinned cites; the legacy greps; the I/O boundary, the pinned assumptions and the MCP claims; `check_citations` 46 at-risk lines, all `ROADMAP.md` rows 27 and 29, unmoved. Lesson: T10's figures came from a test comment, not a run; one probe script settled it. From f13d13700041b62e6f8bfd6d1b6495a1d8ba8f90 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 20:38:09 +0200 Subject: [PATCH 10/13] 38 @perf's quick check NO CHANGE ACCEPTED recorded, by the main session Co-Authored-By: Claude Fable 5.1 --- docs/increments/38-threads-and-memory.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index fda68111..a99f72fa 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); green `b252a70d` (`@developer`, 96 net production lines by `python3 tools/count_loc.py 5d2e706d HEAD` against the estimate of 60, under the worst case of 120); test-side fixes `9de3212c` (`@tester`); section 12 is the as-built record; next the quick check (bit-identical meshes) once the machine is idle, then code review;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); green `b252a70d` (`@developer`, 96 net production lines by `python3 tools/count_loc.py 5d2e706d HEAD` against the estimate of 60, under the worst case of 120); test-side fixes `9de3212c` (`@tester`); section 12 is the as-built record; `@perf`'s quick check NO CHANGE, ACCEPTED; next code review;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds @@ -740,9 +740,12 @@ coverage 98.44 %, one failure beside the night's detached mesh batch (`test_core_cdt.py`'s GIL ticker, 7 ticks against a bound of 3, load-bound; 5 of 5 alone, untouched by this increment). `ruff` clean on `tests/`. -**Speed:** no refine or mesh code touched; the quick check for bit-identical -meshes runs once the machine is idle (the Swedish batch holds three cores -tonight); its verdict goes here. +**Speed:** no refine or mesh code touched. `@perf`'s quick check of this +branch at `8f9289c4` (2026-10-12 18:31 UTC, idle, AC, against the baseline +refreshed minutes before on master `f09b9863`; evidence in +`docs/benchmarks/2026-10-12/baseline-refresh/README.md`, evening section, on +`worktree-baseline-1012`): **NO CHANGE**, meshes identical on every case, no +phase outside its band; ACCEPTED. **Lessons for the next retrospective:** a design that deletes a file lists the unpinned citations into it first (`check_citations` would have shown the From 7b6d4ddc17d863827df928f206f3921f6591e6c8 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 22:07:21 +0200 Subject: [PATCH 11/13] 38 code review round 1 recorded, the INSTALL paragraph moved and the three no-mcp-change sentences corrected, by the main session Co-Authored-By: Claude Opus 5.5 --- INSTALL.md | 14 +++++++------- docs/increments/38-threads-and-memory.md | 12 +++++++----- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index 51c909eb..cba0ae60 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -188,19 +188,19 @@ The static checks CI runs are listed in `CLAUDE.md`, section 4. Two DEMs rasputin fetches itself, into a tile cache (`$RASPUTIN_DATA/cache` by default, or `--cache DIR`): -Two more environment variables: `RASPUTIN_THREADS` is the default thread -count for `mesh`, `catchment` and `station-catchments` (0 or unset means every -core; `--threads N` overrides it), and inside a container the core count and -the memory follow the container's own limits (`cpu.max`, `memory.max`). -`--memory-budget 8G` caps the DEM window one run assembles; the default is -half the machine's memory. - ```sh rasputin fetch glo30 --domain catchment.geojson # --dry-run prints the plan rasputin mesh --dem glo30 --domain catchment.geojson --out-crs EPSG:32723 \ --tolerance 5 --out catchment.vtk # offline from here ``` +One more environment variable: `RASPUTIN_THREADS` is the default thread +count for `mesh`, `catchment` and `station-catchments` (0 or unset means every +core; `--threads N` overrides it), and inside a container the core count and +the memory follow the container's own limits (`cpu.max`, `memory.max`). +`--memory-budget 8G` caps the DEM window one run assembles; the default is +half the machine's memory. + `glo30` is the Copernicus GLO-30 (30 m, worldwide) and `anadem-v1` the ANADEM terrain model of South America. Both are in geographic coordinates, so `mesh` needs `--out-crs`; its refusal suggests one. The source's cache directory diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index a99f72fa..161d6680 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); green `b252a70d` (`@developer`, 96 net production lines by `python3 tools/count_loc.py 5d2e706d HEAD` against the estimate of 60, under the worst case of 120); test-side fixes `9de3212c` (`@tester`); section 12 is the as-built record; `@perf`'s quick check NO CHANGE, ACCEPTED; next code review;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); green `b252a70d` (`@developer`, 96 net production lines by `python3 tools/count_loc.py 5d2e706d HEAD` against the estimate of 60, under the worst case of 120); test-side fixes `9de3212c` (`@tester`); section 12 is the as-built record; `@perf`'s quick check NO CHANGE, ACCEPTED; code review round 1 two prose blockers, fixed in the recording commit; next round 2;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds @@ -371,7 +371,7 @@ stride records; the gallery's flat record has no DEM and no row. - **The MCP server (increment 37)**: its schemas are derived from the Typer parameters (`src_python/tin_engine/mcp/schema.py@5d2e706d:61-77`), so `threads` and `memory_budget` appear in the `mesh`, `catchment` and `station_catchments` - tools with no server change, and an agent sets them per call; the child + tools with no server change (one line in `mcp/schema.py` keeps a zero lower bound, section 12), and an agent sets them per call; the child inherits the server's environment (`src_python/tin_engine/mcp/server.py@5d2e706d:111`, `env={**os.environ, **CHILD_ENV}`), so `RASPUTIN_THREADS` set where the server starts is the default for every call. One test pins the first (T9). @@ -549,7 +549,7 @@ from a literal. `tests/python/test_crs.py@5d2e706d:207-213`'s `WORKERS` assertion goes. - **T9 the MCP schema carries the options**: the `mesh` entry of `derive(app)` has a model with fields `threads` (int or None, default None) and `memory_budget` (str or - None), with no change to `mcp/`; `DerivedCommand.argv(model)` + None), with one line changed in `mcp/schema.py` (a zero lower bound kept; section 12); `DerivedCommand.argv(model)` (`src_python/tin_engine/mcp/schema.py@5d2e706d:108`) with the entry's model built from `{"threads": 3}` plus the required arguments ends with `--threads 3`, and with `{"memory_budget": "8G"}` ends with @@ -668,8 +668,8 @@ Four questions were put to Ola with defaults; he ruled "Yes to all" on Entries this design takes up or overlaps: *Product review (b), generality audit* (this is its first fix); *MCP server over the rasputin commands* (an agent now sets `threads` per call through the derived schema, and -`RASPUTIN_THREADS` in the server's environment is the default; nothing in -`mcp/` changes). Every other entry is untouched. New idea from this design, +`RASPUTIN_THREADS` in the server's environment is the default; `mcp/schema.py` +changes by one line, section 12). Every other entry is untouched. New idea from this design, for the handback: `tools/bench.py`'s child could pass `--threads` to the CLI and drop its monkeypatch of `refine` (tooling, not counted). @@ -759,3 +759,5 @@ test's bound of 3 ticks is load-sensitive. **Design review, round 1, 2026-10-12.** Range `5d2e706d..9e0d60c2`, 0 production lines, estimate 60 net with worst case 120, @reviewer: CHANGES REQUESTED, five blockers. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/docs/increments/38-threads-and-memory.md@9e0d60c2:476-481: T10's figures are wrong; probed on the fixture, the first window is 49 x 49, the mosaic cap 9,604 bytes and the flood 14,406, so a budget of 8,192 is refused by the mosaic cap (`/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/catchment.py@5d2e706d:394`) naming 9,604, not the flood, and 13,824 also refuses; fix: a budget in 9,604 to 14,405 for the flood case, 14,406 for "exactly at", the `8K` CLI case restated. (2) @9e0d60c2:485-488: T11 misses `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/tests/python/test_layering.py@5d2e706d:44` (the `decompose` row), whose line 152 fails on a gone module; the red commit drops that row and adds `resources`. (3) @9e0d60c2:457-470: T8 spies no reprojector at `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/feature_input.py@5d2e706d:339` or `/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/land_cover_fractions.py@5d2e706d:67`, two of the three large sites; with the library default of one worker a dropped `threads` serialises them unseen; one spy per site. (4) @9e0d60c2:133-141: 15 reprojector callers, not 14 (`target_grid.py:136`, `tolerance_field.py:83`, `feature_input.py:183` missing; "other ten" is twelve); `target_grid.py:239`'s blocks are 512 x 512 = 262,144 points, under the 500,000 threshold, so only line 181 nests a pool. (5) @9e0d60c2:177-181: `cpu.max` with a non-numeric first word or a zero period is unspecified; apply the memory rule (no quota). Suggestions: the refusal text "half the machine's memory" (:254-256) is false when a budget was given; section 4.5 should say `bench.py` children's stats row still shows the CLI's count; T9's `argv` is `DerivedCommand.argv(model)` (`src_python/tin_engine/mcp/schema.py@5d2e706d:108`). Verified: all 37 pinned cites; the legacy greps; the I/O boundary, the pinned assumptions and the MCP claims; `check_citations` 46 at-risk lines, all `ROADMAP.md` rows 27 and 29, unmoved. Lesson: T10's figures came from a test comment, not a run; one probe script settled it. **Design review, round 2, 2026-10-12.** Range `3ff48435..3018192a`, 0 production lines, estimate 60 net, @reviewer: APPROVED. Verified by running: T10's figures with the main venv (8,192 refused by the mosaic cap at 9,604; 9,604 to 14,405 by the flood cap at 14,406; 14,406 refused on the probe window 73 x 55 at 16,060; 235,199 refused naming 224 x 175; 235,200 floods three windows), every figure in the table matching; the `decompose` layering row and its two assertions; the three T8 spy sites and the EPSG:4326-over-25833 case; the caller grep reproducing 15 lines, `target_grid.py` under the 500,000 threshold; the `cpu.max` rule in 4.1 with its cases; section 10's four rulings stated as rulings with date and quote; check_citations exit 0. Idea, non-blocking: T3 omits section 6's unreadable-file case (chmod 000 in `tmp_path`, skipped as root). + +**Code review, round 1, 2026-10-12.** Range `5d2e706d..f13d1370`, 96 net production lines (237 added, 141 removed; under 700), @reviewer: CHANGES REQUESTED, two prose blockers, the code sound; applied in this recording commit. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/INSTALL.md@f13d1370:191-196: the new paragraph sat between a sentence ending in a colon and the code block it introduces, and said "Two more environment variables" naming one; moved below the block, "One more environment variable". (2) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/docs/increments/38-threads-and-memory.md@f13d1370:374, :552 and :671-672 said no `mcp/` change; the branch changes `mcp/schema.py` by one line; corrected. Pins accepted: the `mcp/schema.py` zero-bound fix; `edge_strip.run` and `final_check.run` defaulting to `threads=0` (section 4.2 keeps 0 for direct callers; the CLI always passes 1 or more). Verified: no scaffolding; the machine read only in `resources.py`; the model's rules, the cgroup reads and `parse_bytes` as sections 4.1 to 4.3 and 6; the count reaching all five C++ calls and three pools, the cap of 8 gone; stats row and record entry; both refusals naming `--memory-budget` and the figure; `decompose.py` gone; the C++ still takes a plain integer; check_citations exit 0; the four new suites 181 passed. Lesson: a paragraph inserted into a document breaks a "sentence, colon, code block" pattern unless the lines around it are read. From 173324a2780ddbadec8e12af918be307a549717c Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Mon, 12 Oct 2026 22:10:29 +0200 Subject: [PATCH 12/13] 38 code review round 2 APPROVED recorded, with the Status line and ROADMAP row, by the main session Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 2 +- docs/increments/38-threads-and-memory.md | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index e41a4ae4..1fc08663 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -58,7 +58,7 @@ increment that most needs a picture to check against | 32 | Land-cover borders simplified within a band, each class keeping its area exactly (Ola, 2026-10-08: "we need to implement is a CORINE simplification, similar to what we do on the outline of the auto-catchments"): the land-cover coverage cut into borders between junctions, each border reduced by the area-preserving segment collapse of the catchment outline (`reduce_ring`), with junctions and the domain outline fixed, a crossing test and a side test across all borders, and a band no border moves past (two-sided, proved by anchors on the source border). `--features-tolerance` becomes that band, default 50 m (half CORINE's positional accuracy), replacing GEOS's coverage simplifier; start angle 15° when land cover is simplified (band above 0), 25° otherwise. Prototype on the German fused catchment: 5.7 times fewer triangles at 50 m vertical tolerance | **merged (#222, 2026-10-11)** (green `82c0a7d4`, 519 net production lines, one PR; mutation round `30f939b6`, one mutant recorded as not killed); Ola ruled all four questions (2026-10-08); code review round 1 asked for changes (comments, docstrings and `project_structure.md` the change made false), fixed by `4d4f0411` and the docs commit after it; code review rounds 1-3 done (round 3 approved); `@perf`'s short timing check found the runs about 70 % slower (the outline rule on borders clipped to the catchment) and the worst angle far lower (the simplifier placing points millimetres from junctions); fix built (section 15: outline rule first, a 1 m clearance in the simplifier; green `05c8f73a`, 573 counted lines in all; mutation round M7 to M13 killed); its quick check `b192a2c6`: Numedalslagen −11 %, Lagan −1.3 %, outline rule and moved area as before, triangles −42 %; questions 5 and 6 decided on default while Ola was away, reversible; fix code review round 1 asked for prose fixes and the test file's red-step staging removed; fix code review round 3 approved; next the push on Ola's yes | `docs/increments/32-landcover-simplify.md` | | 36 | GeoPackage output, so QGIS opens a mesh as it opens any vector layer: `rasputin mesh --out X.gpkg`, chosen by the suffix of `--out` as `.vtk` and `.ply` are (Ola, 2026-10-05: "Is the vtk_to_phkg.py the way to go, or as a direct output from rasputin? I think the latter, and it should go into the roadmap and eventually into the codebase."). Proposed content: a `triangles` layer, one PolygonZ per triangle in the run's CRS (z at each of the three corners, as the stopgap writes), with the attributes the run already knows: land-cover code and class name (the code system's names, as `rasputin palette` has them), mean z, plan area; a layer of the constraint polygons (the `--features` polygons clipped to the domain, with their code); optionally a QGIS style for land cover stored in the file (the `layer_styles` table), so the triangles open coloured. Land-cover colour: when the mesh carries CORINE classes, each triangle and each constraint polygon also gets its class's colour as a `#rrggbb` text column, so a reader can colour by the column without the stored style (in QGIS, a data-defined fill colour). The Graz file of 2026-10-05 carries `clc_color` on its `triangles` and `corine` layers; its stored styles categorise on `clc_code` and do not read the column. The colours are those of `rasputin palette` (`src_python/tin_engine/palettes.py`), the same 26 the Graz file carries: natural colours, as Ola chose over the official CLC legend in 16c's ruling 2 (that legend paints peat bogs blue and the sea almost white). **One attribute is not in the mesh yet**: the ID of the source polygon each triangle lies in (a sub-catchment cell, a parcel). Today only a class code labels triangles, one code system per mesh (16c, 16e's D1); the stopgap finds the cell by a centroid lookup outside rasputin. It needs a per-triangle array from a polygon source's ID property: its own small step, done just before this writer, because the `.vtk` and `.ply` outputs gain the attribute too (Ola, 2026-10-05, accepting the default: "defaults on the cell ID"). Written with the standard library's `sqlite3` beside the existing reader `src_python/tin_engine/io/geopackage.py`, which can read it back in the tests; no GDAL, OGR or Fiona (CLAUDE.md §2). Python only: the C++ core is unchanged and never sees the file (the I/O boundary). Prior art and working spec: Ola's stopgap `vtk_to_gpkg.py`, outside the repository (`rasputin_data/lands/scripts/`), which converts a mesh `.vtk` with `sqlite3`, NumPy, Shapely and PyProj, no GDAL, and its first use, the Mur above Graz (2 451 734 triangles, 10 m DEM, tolerance 10 m, 2026-10-05): **the GeoPackage was 677.9 MB against 110.6 MB for the binary `.vtk`**; the whole stopgap run took 39 s and 4.1 GB peak, and that includes the centroid cell lookup and the CORINE clipping, so it is not the cost of the GeoPackage encoding alone: the design measures the writer on its own. A GeoPackage row holds the triangle's whole ring (four corners, the first repeated) and a bounding box, and the file adds a spatial index, where `.vtk` stores each vertex once. Questions for the design: file size (keep the spatial index or not; at most an opt-in variant of 2D polygons with mean z as an attribute, proposed default: full PolygonZ; writing in chunks to bound memory); the CRS entry for a CRS with no EPSG code; where the run record (increment 25) goes; whether the constraint lines get a layer of their own | **merged (#231)** (branch `worktree-gpkg-out`: red `87f41c3d`, green `c59246e5`, fix-up `7085dddf`, 377 net production lines, mutation round `dddce8e8` ten of ten killed; `@perf`'s acceptance `57c25074` ACCEPTED: Numedalslagen 3.93 s / 203 MB, Romsdalen 19.4 s / 810 MB, every mesh byte-identical; code review round 1's one blocker, an empty land-cover layer crashing the write, fixed by `55cc3c3e` + `18442b27`, 378 net lines; code review round 2 APPROVED; merged 2026-10-12). Designed by `@architect`, 2026-10-11, on `81a551b3`; proposed by Ola 2026-10-05, "yes, roadmap row now, build after PR D"; PR D merged as #205, so the writer joins the encoders under `io/`). Measured on this design's probe: the quick check's Numedalslagen mesh (653,039 triangles) written in 4.0 s to 203 MB against 0.28 s and 29 MB for the binary `.vtk`, the stopgap 5.1 s; the Romsdalen slope mesh (3.1 million) in 21.5 s to 922 MB at 2.1 GB peak, the stopgap 28.7 s at 4.1 GB; the R-tree index is 60 % of the time; the same run writes the same bytes. Layers `triangles` (PolygonZ with code, name, colour, z_min, z_max, z_mean, area_m2, slope_deg), `lines`, `land_cover`, `land_cover_classes`, a `run` table with the run record, and the stored QGIS style; about 305 net lines, one PR; questions 1-5 answered with their defaults while Ola is away | `docs/increments/36-geopackage-output.md` | | 37 | An MCP (Model Context Protocol) server over the rasputin commands, so an external agent (Codex, Claude Desktop, ChatGPT, Mistral) discovers and calls rasputin as tools instead of reading its help text: `rasputin mcp` serves over stdio, one tool per command with a JSON schema derived from the Typer command's own parameters (one source of truth, so schema and CLI cannot drift), each call run as a child `python -m tin_engine ` so a cancel kills it and a crash ends only the child, with the exit code, output paths, run record and `--stats` returned as data and a refusal returned as a structured error carrying the CLI's plain words and exit code (Ola, 2026-10-11: "my working name for what's we're doing is intent-driven triangulation. This means accessing the tools using agents and intent"; "Add an MCP server to the ideas file after GeoPackage"; 2026-10-12: "Rule the mcp SDK: yes, use it"). Built on the official `mcp` Python SDK 2.3 as an optional extra `rasputin[mcp]`; stdio first, Streamable HTTP later or never; no Tasks extension (the SDK has none server-side); a jobs cap, default 1; the GeoPackage output (36) is the mesh an agent reads | **built and review-approved** (branch `worktree-mcp-server`: design `b990e38c` approved in three review rounds, red `ee5b5d52` 62 tests, green `851592ba`, three code review rounds with a README section, an `error`-field fix and a help-text escape, master merged in as `030fd32b`, 328 net production lines against 260; `dev` pulls the `mcp` extra; next the push on Ola's yes). Designed (`@architect`, 2026-10-12, on `b4a81c0d`): about 250 counted lines, one PR; nine decisions with defaults for Ola (section 6); measured by the design's probe: a tool call costs 0.25 s over the command itself (the child's import of `tin_engine.cli`), the 1 m benchmark mesh 0.72 s through the tool, a cancelled child dead within 0.5 s over stdio and in process, unknown and mistyped arguments refused before any child runs; no refine or mesh code, so no `@perf` run | `docs/increments/37-mcp-server.md` | -| 38 | Threads and memory as settings, the first fix from the generality audit (Ola, 2026-10-12: "the number of cores should of course not be hardcoded"): `--threads N` on `mesh`, `catchment` and `station-catchments` (0 = every core; `RASPUTIN_THREADS` is the default when the option is absent; neither set = every core, a container's CPU quota counting), carried as one `Resources` model built once in `cli.py` to every `_core` call that takes `threads` (five: `refine`, `refine_strip`, `refine_points`, `SlopeTolerance`, `class_areas`) and every Python pool (three: the reprojector, whose fixed cap of 8 goes, the target grid's resample and check points, the block decoder); the stats file and run record print the count used; `--memory-budget 8G` caps the mosaic and flood windows, default half the machine's memory with a container's cgroup limit (`memory.max`, v1 `memory.limit_in_bytes`) counting as the machine's; the unused `decompose.py` deleted with its test. Python only, no C++ change, about 60 net lines | **built** (branch `worktree-threads`: design `9e0d60c2` approved in two review rounds; red `4b9c0dfe` 110 tests, green `b252a70d`, test fixes `9de3212c`, 96 net production lines against 60; next the quick check and code review). Designed by `@architect`, 2026-10-12, as the generality audit's first fix | `docs/increments/38-threads-and-memory.md` | +| 38 | Threads and memory as settings, the first fix from the generality audit (Ola, 2026-10-12: "the number of cores should of course not be hardcoded"): `--threads N` on `mesh`, `catchment` and `station-catchments` (0 = every core; `RASPUTIN_THREADS` is the default when the option is absent; neither set = every core, a container's CPU quota counting), carried as one `Resources` model built once in `cli.py` to every `_core` call that takes `threads` (five: `refine`, `refine_strip`, `refine_points`, `SlopeTolerance`, `class_areas`) and every Python pool (three: the reprojector, whose fixed cap of 8 goes, the target grid's resample and check points, the block decoder); the stats file and run record print the count used; `--memory-budget 8G` caps the mosaic and flood windows, default half the machine's memory with a container's cgroup limit (`memory.max`, v1 `memory.limit_in_bytes`) counting as the machine's; the unused `decompose.py` deleted with its test. Python only, no C++ change, about 60 net lines | **built** (branch `worktree-threads`: design `9e0d60c2` approved in two review rounds; red `4b9c0dfe` 110 tests, green `b252a70d`, test fixes `9de3212c`, 96 net production lines against 60; quick check NO CHANGE; code review two rounds, APPROVED; next the push on Ola's yes). Designed by `@architect`, 2026-10-12, as the generality audit's first fix | `docs/increments/38-threads-and-memory.md` | | — | A missing station number refused in plain words. In a reference-polygon file (`rasputin station-catchments --reference`), a feature whose `station` is null becomes the text "None" (`src_python/tin_engine/io/station_set.py@ed12512:89`). No station can match it, since a station number must look like `2.11.0` (`src_python/tin_engine/hydrography.py@ed12512:40`), so the polygon is dropped without a word and the station it belonged to is written with no comparison (`src_python/tin_engine/catchment_batch.py@ed12512:220`); two such polygons are refused as "station None appears twice". In a stations file a null number is refused, but in Pydantic's validation text rather than a sentence naming the file (`src_python/tin_engine/io/station_set.py@ed12512:79`). NVE's files have a number on every station, so only hand-made or third-party files are hit. The fix: both readers refuse a null `station`, naming the file; one or two lines. Plainer GeoJSON error messages in general are not planned until other people feed rasputin their own files | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | A GeoTIFF whose coordinate-system GeoKeys hold garbage refused in plain words. The reader turns each code key into a whole number with `int()`, and a key stored as a double or as text can hold NaN, infinity or letters. Probed 2026-10-06 on hand-made files: NaN or text in `ProjectedCSTypeGeoKey` (3072) stops `rasputin mesh --dem` with Python's conversion message (for NaN, "Invalid value for --dem: cannot convert float NaN to integer"), which names neither the key nor the file, and infinity ends in a Python traceback (`OverflowError`) (`src_python/tin_engine/io/geotiff.py@ed12512:171`). The same holds for every other code key the reader reads: 1025, 4099, 3076, 3074, 3075, 2048 and 2054 (`src_python/tin_engine/io/geotiff.py@ed12512:310`, `src_python/tin_engine/io/geotiff.py@ed12512:176`, `src_python/tin_engine/io/geotiff.py@ed12512:339`, `src_python/tin_engine/io/geotiff.py@ed12512:352`, `src_python/tin_engine/io/geotiff.py@ed12512:362`, `src_python/tin_engine/io/geotiff.py@ed12512:499`, `src_python/tin_engine/io/geotiff.py@ed12512:423`). Worse, a fractional value is cut to a whole number first: 2.5 in 4099 is refused as 2, and 32633.5 in 3072 is read as EPSG:32633 without a word (`src_python/tin_engine/io/geotiff.py@ed12512:321`); 1.5 in 1025, 9001.7 in 4099 and 9001.5 in 3076 are likewise accepted as 1 and 9001 (probed by code review round 1 on this row's branch, exit 0). The parameter keys are already checked (`_one_number`, `src_python/tin_engine/io/geotiff.py@ed12512:454`). The fix: one check for every code key, a whole number or a refusal naming the key and its value. No real file is known to do this | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | tifffile's warning about the NoData tag silenced. On a float32 DEM whose `GDAL_NODATA` (42113) is the lowest float32 written in 17 digits (`-3.4028234663852886e+38`), tifffile prints "parsing GDAL_NODATA tag raised ValueError: ... is not castable to float32" to stderr when the file is opened (`src_python/tin_engine/io/geotiff.py@ed12512:146`). It is noise: rasputin reads the tag itself and accepts the value, since it is that float32 exactly (`src_python/tin_engine/io/geotiff.py@ed12512:535`). The fix: silence that one tifffile message while the reader opens the file, leaving every other tifffile warning visible | approved by Ola 2026-10-06; not designed (a small PR) | - | diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index 161d6680..1b18967e 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -1,6 +1,6 @@ # Increment 38: threads and memory as settings -**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); green `b252a70d` (`@developer`, 96 net production lines by `python3 tools/count_loc.py 5d2e706d HEAD` against the estimate of 60, under the worst case of 120); test-side fixes `9de3212c` (`@tester`); section 12 is the as-built record; `@perf`'s quick check NO CHANGE, ACCEPTED; code review round 1 two prose blockers, fixed in the recording commit; next round 2;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch +**Status: designed and review-approved after two rounds; red `4b9c0dfe` (`@tester`, 110 tests); green `b252a70d` (`@developer`, 96 net production lines by `python3 tools/count_loc.py 5d2e706d HEAD` against the estimate of 60, under the worst case of 120); test-side fixes `9de3212c` (`@tester`); section 12 is the as-built record; `@perf`'s quick check NO CHANGE, ACCEPTED; code review round 1 two prose blockers, fixed; round 2 APPROVED; next the push on Ola's yes;** by `@architect`, 2026-10-12, on master `5d2e706d` (branch `worktree-threads`). The first fix from the generality audit (`docs/increments/generality-audit.md`, section 6 item 1; rows D1, D2, D3, D5, D6, D7). Ola's yes, 2026-10-12: "1, 2: yes" to a single pull request that adds @@ -761,3 +761,5 @@ test's bound of 3 ticks is load-sensitive. **Design review, round 2, 2026-10-12.** Range `3ff48435..3018192a`, 0 production lines, estimate 60 net, @reviewer: APPROVED. Verified by running: T10's figures with the main venv (8,192 refused by the mosaic cap at 9,604; 9,604 to 14,405 by the flood cap at 14,406; 14,406 refused on the probe window 73 x 55 at 16,060; 235,199 refused naming 224 x 175; 235,200 floods three windows), every figure in the table matching; the `decompose` layering row and its two assertions; the three T8 spy sites and the EPSG:4326-over-25833 case; the caller grep reproducing 15 lines, `target_grid.py` under the 500,000 threshold; the `cpu.max` rule in 4.1 with its cases; section 10's four rulings stated as rulings with date and quote; check_citations exit 0. Idea, non-blocking: T3 omits section 6's unreadable-file case (chmod 000 in `tmp_path`, skipped as root). **Code review, round 1, 2026-10-12.** Range `5d2e706d..f13d1370`, 96 net production lines (237 added, 141 removed; under 700), @reviewer: CHANGES REQUESTED, two prose blockers, the code sound; applied in this recording commit. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/INSTALL.md@f13d1370:191-196: the new paragraph sat between a sentence ending in a colon and the code block it introduces, and said "Two more environment variables" naming one; moved below the block, "One more environment variable". (2) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/docs/increments/38-threads-and-memory.md@f13d1370:374, :552 and :671-672 said no `mcp/` change; the branch changes `mcp/schema.py` by one line; corrected. Pins accepted: the `mcp/schema.py` zero-bound fix; `edge_strip.run` and `final_check.run` defaulting to `threads=0` (section 4.2 keeps 0 for direct callers; the CLI always passes 1 or more). Verified: no scaffolding; the machine read only in `resources.py`; the model's rules, the cgroup reads and `parse_bytes` as sections 4.1 to 4.3 and 6; the count reaching all five C++ calls and three pools, the cap of 8 gone; stats row and record entry; both refusals naming `--memory-budget` and the figure; `decompose.py` gone; the C++ still takes a plain integer; check_citations exit 0; the four new suites 181 passed. Lesson: a paragraph inserted into a document breaks a "sentence, colon, code block" pattern unless the lines around it are read. + +**Code review, round 2, 2026-10-12.** Range `f13d1370..7b6d4ddc`, @reviewer: APPROVED. Both round-1 fixes in place (the INSTALL paragraph after the code block, "One more environment variable"; the three design sentences naming the one-line `mcp/schema.py` change, matching the diff); round 1's citations resolve; nothing else changed; check_citations exit 0. From 678f45851f6f59a3f3a04af877683f4085042e69 Mon Sep 17 00:00:00 2001 From: Ola Skavhaug Date: Tue, 13 Oct 2026 08:40:41 +0200 Subject: [PATCH 13/13] 38 merge review of 3942af15 recorded: APPROVED, by the main session Co-Authored-By: Claude Opus 5.5 --- docs/increments/38-threads-and-memory.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/increments/38-threads-and-memory.md b/docs/increments/38-threads-and-memory.md index 1b18967e..78dda655 100644 --- a/docs/increments/38-threads-and-memory.md +++ b/docs/increments/38-threads-and-memory.md @@ -763,3 +763,5 @@ test's bound of 3 ticks is load-sensitive. **Code review, round 1, 2026-10-12.** Range `5d2e706d..f13d1370`, 96 net production lines (237 added, 141 removed; under 700), @reviewer: CHANGES REQUESTED, two prose blockers, the code sound; applied in this recording commit. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/INSTALL.md@f13d1370:191-196: the new paragraph sat between a sentence ending in a colon and the code block it introduces, and said "Two more environment variables" naming one; moved below the block, "One more environment variable". (2) /Users/skavhaug/projects/rasputin/.claude/worktrees/threads/docs/increments/38-threads-and-memory.md@f13d1370:374, :552 and :671-672 said no `mcp/` change; the branch changes `mcp/schema.py` by one line; corrected. Pins accepted: the `mcp/schema.py` zero-bound fix; `edge_strip.run` and `final_check.run` defaulting to `threads=0` (section 4.2 keeps 0 for direct callers; the CLI always passes 1 or more). Verified: no scaffolding; the machine read only in `resources.py`; the model's rules, the cgroup reads and `parse_bytes` as sections 4.1 to 4.3 and 6; the count reaching all five C++ calls and three pools, the cap of 8 gone; stats row and record entry; both refusals naming `--memory-budget` and the figure; `decompose.py` gone; the C++ still takes a plain integer; check_citations exit 0; the four new suites 181 passed. Lesson: a paragraph inserted into a document breaks a "sentence, colon, code block" pattern unless the lines around it are read. **Code review, round 2, 2026-10-12.** Range `f13d1370..7b6d4ddc`, @reviewer: APPROVED. Both round-1 fixes in place (the INSTALL paragraph after the code block, "One more environment variable"; the three design sentences naming the one-line `mcp/schema.py` change, matching the diff); round 1's citations resolve; nothing else changed; check_citations exit 0. + +**Merge review, 2026-10-13.** Merge `3942af15` (origin/master with increment 41 into this branch, by `@developer`), @reviewer: APPROVED. The five `cli.py` conflict hunks keep both sides: `_open_features` takes the vocabulary then `threads` and builds `FeatureRequest` with both (/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/cli.py@3942af15:1955), its one call passes them in that order (/Users/skavhaug/projects/rasputin/.claude/worktrees/threads/src_python/tin_engine/cli.py@3942af15:1168), and `_dem_mesh`'s one call passes `res.threads, vocabulary` in its signature's order; 450 targeted tests passed, 2 skipped (Ola's data).