From 9c96a668aa2c4fa8172769a282d342100f318c4a Mon Sep 17 00:00:00 2001 From: Patrick Leiverkus Date: Sun, 26 Jul 2026 15:26:47 +0200 Subject: [PATCH] feat: add RasterKind.SEMANTIC and make intake layout-aware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Effigies now ships a per-pixel class field at `odm_semantic/orthophoto_semantic.tif`. Pointed at such a delivery tree, `discover_inputs` tagged it ORTHO — the filename contains "ortho" and that substring was tested first — so the class raster went to the 2D segmenter, which read its class codes as pixel intensities and emitted spurious STONE polygons. Nothing downstream re-checks the kind, so this failed silently; the added pipeline test reproduces it as 4 features where 2 are correct. Classification now prefers the delivery layout: a raster under odm_orthophoto/, odm_dem/ or odm_semantic/ takes its kind from the directory, because the directory states what the product is. The filename fallback remains for flat input dirs, with "semantic" tested before "ortho". Rasters matching neither rule are skipped rather than guessed at, which is a behaviour change only for files that were previously mis-tagged. `classify` is split out of `discover_inputs` so the rule is testable without a directory walk. SEMANTIC products are tagged and deliberately not consumed — the field becomes a prior for the vector tracks in v0.8. The explicit branch in `pipeline.run` documents that, rather than letting them fall through the dispatch unmentioned. Closes the intake prerequisite for v0.8 and the intake-layout open decision. Verified on 3.11 and 3.12: ruff clean, mypy clean on 26 files, 57 passed / 2 skipped. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 18 ++++++++ ROADMAP.md | 14 +++--- docs/architecture.md | 32 +++++++++----- src/structura/cli.py | 2 +- src/structura/intake.py | 54 ++++++++++++++++++----- src/structura/pipeline.py | 10 ++++- tests/test_intake.py | 92 +++++++++++++++++++++++++++++++++++++++ tests/test_pipeline.py | 17 ++++++++ 8 files changed, 210 insertions(+), 29 deletions(-) create mode 100644 tests/test_intake.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7163fcc..06e5b8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **`RasterKind.SEMANTIC`** — Effigies' per-pixel class field + (`odm_semantic/orthophoto_semantic.tif`) is now a first-class intake product. + It is tagged and deliberately not consumed; consuming it as a prior for the + vector tracks is ROADMAP v0.8. +- `intake.classify`, covering one raster, split out of `discover_inputs` so the + classification rule is testable on its own. + +### Fixed +- **The semantic class raster was silently segmented as an orthophoto.** + `discover_inputs` tested filenames for `"ortho"` first, and + `orthophoto_semantic.tif` contains that substring — so pointing Structura at an + Effigies delivery tree fed the class raster to the 2D segmenter, which read its + class codes as pixel intensities and emitted spurious `STONE` polygons. No error + was raised. Discovery now prefers the delivery layout (`odm_orthophoto/`, + `odm_dem/`, `odm_semantic/`), and the filename fallback tests `"semantic"` + before `"ortho"`. Rasters matching no rule are skipped instead of guessed at. + ### Changed - **Ruff is now pinned exactly (`ruff==0.16.0`) and the lint rule set is selected explicitly.** Ruff 0.16.0 enabled `RUF100` (unused-`noqa`) by default, which diff --git a/ROADMAP.md b/ROADMAP.md index 38953a4..e9b6a31 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -89,9 +89,11 @@ by construction. Effigies already ships ONNX Runtime (CPU + CUDA) and the patter for versioned, SHA256-pinned weights in `$EFFIGIES_MODEL_DIR`, which is the intended delivery path. -- [ ] **Intake prerequisite:** add a `SEMANTIC` `RasterKind` and fix the - discovery heuristic — `orthophoto_semantic.tif` matches `"ortho"` today and - is silently mis-tagged as an RGB orthophoto (see *Open decisions*). +- [x] **Intake prerequisite:** `SEMANTIC` `RasterKind` added, and discovery is + now layout-aware (`odm_orthophoto/`, `odm_dem/`, `odm_semantic/`) with the + filename fallback testing `"semantic"` before `"ortho"`. The class raster + is tagged and deliberately left unconsumed, which is what keeps it out of + the 2D segmenter until the rest of this milestone lands. - [ ] Consume the class raster as a prior / input channel for the vector tracks (e.g. vectorise only where the field says `structure`). - [ ] Implement the field-vs-object contract: Structura owns vector objects, the @@ -121,9 +123,9 @@ intended delivery path. ## Open decisions Tracked in [`docs/architecture.md`](docs/architecture.md#open-decisions): the DB -sink default (v0.5), the 2D model default (decided by the v0.9 evaluation), and -the intake layout heuristic — the last of which now gates v0.8, since the -filename-based discovery cannot tell a semantic class raster from an orthophoto. +sink default (v0.5) and the 2D model default (decided by the v0.9 evaluation). +The intake layout heuristic is resolved — discovery now reads the delivery layout +and distinguishes a semantic class raster from an orthophoto. > This is a living document. Milestone scope and ordering may shift as the > research plan and the upstream Effigies engine evolve. diff --git a/docs/architecture.md b/docs/architecture.md index 9a1b891..0e00636 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -17,7 +17,7 @@ intake ──► tracks ──► Feature[] ──► sink |--------|----------------| | `structura.cli` | `structura` command-line entry point (`intake`, `run`). | | `structura.config` | `Settings` loaded from environment / `.env`. | -| `structura.intake` | Discover WebODM rasters, tag each as `ORTHO` or `DEM`. | +| `structura.intake` | Discover WebODM rasters, tag each as `ORTHO`, `DEM` or `SEMANTIC`. | | `structura.models` | `Feature` (the shared vector object), `Track`, `FeatureType`. | | `structura.geo` | Raster I/O and raster→vector conversion (georeferencing core). | | `structura.segmentation` | 2D track — stones & surfaces (classical / SAM / Cellpose) behind a `Segmenter` protocol; `_common` shares the mask→Feature step. | @@ -47,6 +47,25 @@ Everything a track produces is a `Feature` (`structura.models`): are what make every derived vector world-referenced: pixel `(col, row)` → world `(x, y)` via `transform * (col, row)`. See `structura.geo.read_raster`. +## Intake (`structura.intake`) + +Each discovered raster is tagged with a `RasterKind` — `ORTHO`, `DEM`, or +`SEMANTIC` (Effigies' per-pixel class field) — and the tag is what routes it to a +track. Classification prefers the **delivery layout** over the filename: a raster +under `odm_orthophoto/`, `odm_dem/` or `odm_semantic/` takes its kind from the +directory, because the directory states what the product *is*. + +Only when that layout is absent does `classify` fall back to the filename, and +there the order of tests is load-bearing: `orthophoto_semantic.tif` contains +`"ortho"`, so `"semantic"` must be tested first. Getting this wrong is not a +visible failure — the class raster is simply segmented as though its class codes +were pixel intensities. Rasters matching neither rule are skipped rather than +guessed at. + +`SEMANTIC` products are tagged but not yet consumed; the field becomes a prior +for the vector tracks in ROADMAP v0.8. Tagging it is what keeps it out of the 2D +segmenter in the meantime. + ## Tracks ### 2D — stones & surfaces (`structura.segmentation`) @@ -120,12 +139,5 @@ The active sink is chosen by `STRUCTURA_SINK` (`file` | `postgis` | `api`). API**. Both sinks exist behind one interface; the default is unresolved. - **2D model choice:** SAM vs. Cellpose vs. classical CV — to be decided by the comparative evaluation (blocked on real excavation data). -- **Intake layout:** the discovery heuristic in `intake.discover_inputs` is - filename-based and must be refined to the actual WebODM delivery layout - (e.g. `odm_orthophoto/odm_orthophoto.tif`, `odm_dem/dsm.tif`). It is also - lossy in a way that now matters: `RasterKind` knows only `ORTHO` and `DEM`, and - the substring test tags Effigies' `odm_semantic/orthophoto_semantic.tif` as an - RGB orthophoto — the class raster is handed to the 2D segmenter as if it were a - photo, with no error raised. Resolving this (a `SEMANTIC` kind plus - layout-aware rather than name-aware discovery) is a prerequisite for - [ROADMAP](../ROADMAP.md) v0.8. +*(The intake-layout question that stood here is resolved — see +[Intake](#intake-structuraintake) above.)* diff --git a/src/structura/cli.py b/src/structura/cli.py index 20b4d96..136a2ab 100644 --- a/src/structura/cli.py +++ b/src/structura/cli.py @@ -30,7 +30,7 @@ def main(argv: list[str] | None = None) -> int: print(f"No raster products found under {settings.input_dir}") return 0 for prod in products: - print(f"{prod.kind.value:6} {prod.path}") + print(f"{prod.kind.value:8} {prod.path}") return 0 if args.command == "run": diff --git a/src/structura/intake.py b/src/structura/intake.py index 653183e..61096e1 100644 --- a/src/structura/intake.py +++ b/src/structura/intake.py @@ -1,6 +1,6 @@ # SPDX-License-Identifier: AGPL-3.0-or-later -"""Intake of WebODM raster products (orthophoto + DEM). +"""Intake of WebODM raster products (orthophoto, DEM, semantic class field). Discovers georeferenced rasters delivered into the input directory and tags them by kind so downstream tracks know what to consume. @@ -15,8 +15,9 @@ class RasterKind(str, Enum): - ORTHO = "ortho" # RGB orthophoto -> 2D segmentation track - DEM = "dem" # elevation model -> 2.5D track + ORTHO = "ortho" # RGB orthophoto -> 2D segmentation track + DEM = "dem" # elevation model -> 2.5D track + SEMANTIC = "semantic" # per-pixel class field from Effigies -> prior (v0.8) @dataclass(slots=True) @@ -27,17 +28,50 @@ class RasterProduct: captured_on: date | None = None +# WebODM — and Effigies, which extends the same tree — deliver each product in +# its own directory. Where that layout is present it is authoritative: the +# directory states what a raster *is*, independent of what the file is called. +_PRODUCT_DIRS = { + "odm_orthophoto": RasterKind.ORTHO, + "odm_dem": RasterKind.DEM, + "odm_semantic": RasterKind.SEMANTIC, +} + + +def classify(path: Path) -> RasterKind | None: + """Tag one raster, preferring the delivery layout over the filename. + + Returns ``None`` for rasters matching neither, so unrelated ``.tif`` files in + the input directory are skipped rather than guessed at. + """ + kind = _PRODUCT_DIRS.get(path.parent.name.lower()) + if kind is not None: + return kind + + # Filename fallback, for flat or ad-hoc input directories. Order matters: + # `orthophoto_semantic.tif` contains "ortho", so the semantic test has to run + # first — otherwise the class raster is tagged as a photo and handed to the + # 2D segmenter, which would read its class codes as pixel intensities. + name = path.name.lower() + if "semantic" in name: + return RasterKind.SEMANTIC + if "ortho" in name: + return RasterKind.ORTHO + if any(tag in name for tag in ("dem", "dsm", "dtm")): + return RasterKind.DEM + return None + + def discover_inputs(input_dir: Path) -> list[RasterProduct]: """Find raster products under ``input_dir``. - Heuristic by filename; refine to match the actual WebODM delivery layout - (e.g. ``odm_orthophoto/odm_orthophoto.tif`` and ``odm_dem/dsm.tif``). + Classification prefers the WebODM delivery layout (``odm_orthophoto/``, + ``odm_dem/``, ``odm_semantic/``) and falls back to the filename; see + :func:`classify`. """ products: list[RasterProduct] = [] for tif in sorted(input_dir.rglob("*.tif")): - name = tif.name.lower() - if "ortho" in name: - products.append(RasterProduct(tif, RasterKind.ORTHO)) - elif "dem" in name or "dsm" in name or "dtm" in name: - products.append(RasterProduct(tif, RasterKind.DEM)) + kind = classify(tif) + if kind is not None: + products.append(RasterProduct(tif, kind)) return products diff --git a/src/structura/pipeline.py b/src/structura/pipeline.py index 50d0a68..c4232c1 100644 --- a/src/structura/pipeline.py +++ b/src/structura/pipeline.py @@ -51,8 +51,9 @@ def run(settings: Settings, *, write: bool = True) -> list[Feature]: """Run the vectorisation pipeline over all discovered inputs. Orthophotos are segmented with the configured 2D backend (``make_segmenter``); - DEMs are traced into wall + edge polylines (2.5D track). Returns the produced - features, and writes them to the configured sink unless ``write`` is False. + DEMs are traced into wall + edge polylines (2.5D track); semantic class fields + are tagged but not yet consumed (v0.8). Returns the produced features, and + writes them to the configured sink unless ``write`` is False. """ products = discover_inputs(settings.input_dir) features: list[Feature] = [] @@ -64,6 +65,11 @@ def run(settings: Settings, *, write: bool = True) -> list[Feature]: elif product.kind is RasterKind.DEM: features += WallTracer(gap_bridge_m=settings.gap_bridge_m).trace(product.path) features += EdgeTracer().trace(product.path) + elif product.kind is RasterKind.SEMANTIC: + # Discovered and tagged, but not consumed yet — the class field + # becomes a prior for the vector tracks in v0.8. Tagging it is what + # keeps it out of the 2D segmenter in the meantime. + continue if write and features: make_sink(settings).write(features) diff --git a/tests/test_intake.py b/tests/test_intake.py new file mode 100644 index 0000000..05f63a2 --- /dev/null +++ b/tests/test_intake.py @@ -0,0 +1,92 @@ +"""Intake classification: delivery layout, filename fallback, and the +semantic-vs-orthophoto ambiguity that the substring heuristic used to get wrong. +""" + +from pathlib import Path + +import pytest + +from structura.intake import RasterKind, classify, discover_inputs + + +def _touch(path: Path) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.touch() + return path + + +# --- the regression ------------------------------------------------------- + +def test_semantic_field_is_not_an_orthophoto(tmp_path: Path) -> None: + """`orthophoto_semantic.tif` contains "ortho" and must still tag SEMANTIC. + + The old heuristic tested for "ortho" first, so Effigies' class raster was + handed to the 2D segmenter as if it were a photo — silently, since nothing + downstream re-checks the kind. + """ + assert classify(_touch(tmp_path / "orthophoto_semantic.tif")) is RasterKind.SEMANTIC + + +def test_semantic_field_in_webodm_tree_is_not_segmented(tmp_path: Path) -> None: + """The realistic case: a full Effigies delivery tree.""" + _touch(tmp_path / "odm_orthophoto" / "odm_orthophoto.tif") + _touch(tmp_path / "odm_dem" / "dsm.tif") + _touch(tmp_path / "odm_semantic" / "orthophoto_semantic.tif") + + by_kind = {p.kind: p.path for p in discover_inputs(tmp_path)} + + assert set(by_kind) == {RasterKind.ORTHO, RasterKind.DEM, RasterKind.SEMANTIC} + assert by_kind[RasterKind.ORTHO].name == "odm_orthophoto.tif" + assert by_kind[RasterKind.SEMANTIC].name == "orthophoto_semantic.tif" + + +# --- layout beats filename ------------------------------------------------ + +@pytest.mark.parametrize( + ("directory", "expected"), + [ + ("odm_orthophoto", RasterKind.ORTHO), + ("odm_dem", RasterKind.DEM), + ("odm_semantic", RasterKind.SEMANTIC), + ], +) +def test_product_directory_is_authoritative( + tmp_path: Path, directory: str, expected: RasterKind +) -> None: + """A deliberately unhelpful filename must not override the directory.""" + assert classify(_touch(tmp_path / directory / "output.tif")) is expected + + +def test_directory_wins_over_conflicting_filename(tmp_path: Path) -> None: + assert classify(_touch(tmp_path / "odm_semantic" / "dsm.tif")) is RasterKind.SEMANTIC + assert classify(_touch(tmp_path / "odm_dem" / "ortho.tif")) is RasterKind.DEM + + +# --- filename fallback ---------------------------------------------------- + +@pytest.mark.parametrize( + ("filename", "expected"), + [ + ("odm_orthophoto.tif", RasterKind.ORTHO), + ("ortho_day3.tif", RasterKind.ORTHO), + ("dsm.tif", RasterKind.DEM), + ("odm_dem.tif", RasterKind.DEM), + ("dtm_clipped.tif", RasterKind.DEM), + ("orthophoto_semantic.tif", RasterKind.SEMANTIC), + ("semantic.tif", RasterKind.SEMANTIC), + ], +) +def test_filename_fallback(tmp_path: Path, filename: str, expected: RasterKind) -> None: + assert classify(_touch(tmp_path / filename)) is expected + + +def test_unrelated_rasters_are_skipped(tmp_path: Path) -> None: + _touch(tmp_path / "hillshade_preview.tif") + _touch(tmp_path / "notes.tif") + assert discover_inputs(tmp_path) == [] + + +def test_discover_is_deterministic(tmp_path: Path) -> None: + for name in ("odm_orthophoto.tif", "dsm.tif", "orthophoto_semantic.tif"): + _touch(tmp_path / name) + assert [p.path for p in discover_inputs(tmp_path)] == sorted(tmp_path.rglob("*.tif")) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index acbf933..da714a6 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -1,5 +1,6 @@ """End-to-end pipeline tests (requires the geo extra).""" +import shutil from pathlib import Path import pytest @@ -58,6 +59,22 @@ def test_run_traces_dem_to_polylines(synthetic_dem: Path, tmp_path: Path) -> Non assert (gdf.geometry.geom_type == "LineString").any() +def test_run_ignores_the_semantic_field(synthetic_ortho: Path, tmp_path: Path) -> None: + """A semantic class raster alongside the ortho must not reach the segmenter. + + The copy is byte-identical to the orthophoto, so if it were mis-tagged as one + it would segment cleanly into two more polygons and the count would double — + which is exactly what the old substring heuristic did. + """ + shutil.copy(synthetic_ortho, synthetic_ortho.parent / "orthophoto_semantic.tif") + out = tmp_path / "features.gpkg" + settings = _settings(synthetic_ortho.parent, out) + + features = pipeline.run(settings, write=False) + + assert len(features) == 2 + + def test_run_dry_run_writes_nothing(synthetic_ortho: Path, tmp_path: Path) -> None: out = tmp_path / "features.gpkg" settings = _settings(synthetic_ortho.parent, out)