fix(experiments): address review debt from merged PRs

experiments/autodetect/README.md

@@ -17,7 +17,11 @@ Needs about 300 MB in `~/house-scanning-data/autodetect/`, plus one model's weig
 ```sh
 uv sync && uv run pytest
 uv run python -m autodetect.openimages select && uv run python -m autodetect.openimages download
+curl --fail --location https://cmp.felk.cvut.cz/~tylecr1/facade/CMP_facade_DB_base.zip -o /tmp/CMP_facade_DB_base.zip
+mkdir -p ~/house-scanning-data/autodetect/cmp
+unzip /tmp/CMP_facade_DB_base.zip -d ~/house-scanning-data/autodetect/cmp
 uv run python -m autodetect.cmp
+uv run python -m autodetect.sets                              # downscale the installed electro packet photos
 swift build -c release --package-path vision   # copy rects, coremldet, trainod to ~/house-scanning-data/autodetect/bin
 uv run python -m autodetect.run_vision oi_tune oi_eval cmp electro
 uv run python -m autodetect.owl oi_tune oi_eval cmp electro      # needs weights/owlv2/model_fp16.onnx, then delete it

experiments/autodetect/autodetect/dfine.py

@@ -117,6 +117,7 @@ def train(epochs: int, batch: int = 4, max_steps: int | None = None) -> None:
     gt = ground_truth("oi_train")
     rng = random.Random(SEED)
     step, t0 = 0, time.perf_counter()
+    (DATA / "student").mkdir(parents=True, exist_ok=True)
     log = open(DATA / "student" / "dfine_small_train.log", "a")
     for epoch in range(epochs):
         losses = []

experiments/autodetect/autodetect/extent_gt.py

@@ -137,6 +137,7 @@ def render(door: str) -> str:
         if major:
             d.text((2, y - 11), f"{g:.1f}", fill=(255, 255, 0))
     out = DATA / "preview" / f"elev_{door}.png"
+    out.parent.mkdir(parents=True, exist_ok=True)
     im.save(out)
     return str(out)
 

experiments/autodetect/autodetect/sets.py

@@ -10,11 +10,13 @@
 import os
 from pathlib import Path
 
+from PIL import Image
+
 from .paths import CMP, DATA, ELECTRO, OI, PREDS
 
 SETS = ("oi_tune", "oi_eval", "cmp")
 # Prediction-only set for the 3D-extent step: the eight ETH3D electro photos, downscaled to 1024 px
-# on the long side (python -m autodetect.extent prepare). No 2D ground truth.
+# on the long side (python -m autodetect.sets). No 2D ground truth.
 ELECTRO_1024 = DATA / "electro_1024"
 
 
@@ -52,3 +54,16 @@ def save_preds(model: str, name: str, meta: dict, images: dict) -> None:
 
 def load_preds(model: str, name: str) -> dict:
     return json.loads(pred_path(model, name).read_text())
+
+
+def prepare_electro() -> None:
+    photos = json.loads((ELECTRO / "manifest.json").read_text())["photos"]
+    ELECTRO_1024.mkdir(parents=True, exist_ok=True)
+    for photo in photos:
+        with Image.open(ELECTRO / photo["image"]["path"]) as image:
+            image.thumbnail((1024, 1024))
+            image.convert("RGB").save(image_path("electro", photo["id"]), quality=95)
+
+
+if __name__ == "__main__":
+    prepare_electro()

experiments/autodetect/meter_brand/README.md

@@ -1,5 +1,7 @@
 # Meter brand
 
+The tracked brand scores predate token-boundary fixes in the reader and scorer. They need a new scoring run before use.
+
 Can the phone read the meter's maker from the close-up it already reads the number from? It reuses #16's photos and #16's cached Vision output (`~/house-scanning-data/meter/ocr/clean.jsonl`, revision 3, `accurate`, no language correction), so its numbers compare directly with #16's.
 
 `brands.py` lists 36 meter makers and matches each recognised line against them. `score.py` picks the listed maker on the tallest line that names one. No pass bar was set before the run.

experiments/autodetect/meter_brand/brands.py

@@ -98,7 +98,7 @@ def within_one_edit(a: str, b: str) -> bool:
 def match_brand(line: str) -> str | None:
     """The canonical maker named in one recognised line, or None.
 
-    Long spellings match as a substring of the line, or one edit from a token or a pair of
+    Long spellings match whole tokens or one edit from a token or a pair of
     adjacent tokens (OCR drops or swaps a letter, or splits `LANDIS+GYR`). Short spellings such
     as `GE` must be a whole token, because they occur inside unrelated words.
     """
@@ -112,7 +112,7 @@ def match_brand(line: str) -> str | None:
             if alias in tokens:
                 return canon
             continue
-        if re.search(rf"(^| ){re.escape(alias)}($| )", norm) or alias.replace(" ", "") in norm.replace(" ", ""):
+        if re.search(rf"(^| ){re.escape(alias)}($| )", norm) or alias.replace(" ", "") in windows:
             return canon
         # One edit is allowed from 6 letters up: at 5, GENUS is one edit from "GE US".
         if len(alias.replace(" ", "")) >= 6 and any(within_one_edit(alias.replace(" ", ""), w.replace(" ", "")) for w in windows):

experiments/autodetect/meter_brand/score.py

@@ -70,7 +70,8 @@ def pick_unlisted(lines: list[dict]) -> str | None:
 def names_brand(text: str | None, brand: str) -> bool:
     if not text:
         return False
-    return match_brand(text) == brand or normalise(brand).replace(" ", "") in normalise(text).replace(" ", "")
+    return match_brand(text) == brand or bool(re.search(
+        rf"(?:^| ){re.escape(normalise(brand))}(?:$| )", normalise(text)))
 
 
 def note_head(pid: str, manifest: dict[str, dict]) -> str:

experiments/autodetect/tests/test_review_regressions.py

@@ -0,0 +1,108 @@
+import ast
+import importlib.util
+import json
+import sys
+from pathlib import Path
+from types import CodeType, FunctionType, SimpleNamespace
+from unittest.mock import Mock
+
+import numpy as np
+import pytest
+from autodetect import sets
+from PIL import Image
+
+BRAND_DIR = Path(__file__).resolve().parents[1] / "meter_brand"
+spec = importlib.util.spec_from_file_location("brands", BRAND_DIR / "brands.py")
+brands = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(brands)
+sys.modules["brands"] = brands
+spec = importlib.util.spec_from_file_location("brand_score", BRAND_DIR / "score.py")
+score = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(score)
+
+
+def test_brand_matches_are_tokens_not_substrings():
+    for text in ["VOLTAGE", "GENERAL ELECTRICITY", "NOTITRONWORD"]:
+        assert brands.match_brand(text) is None
+        assert not score.names_brand(text, "GE")
+    assert score.names_brand("GE 120 VOLT", "GE")
+    assert score.names_brand("GENERAL ELECTRIC", "GE")
+    assert brands.match_brand("LANDIS GYR") == "LANDIS+GYR"
+    assert score.names_brand("OXFORO D4S", "OXFORO")
+    assert not score.names_brand("OTHER OXFOROISH", "OXFORO")
+
+
+def test_electro_preparation_uses_packet_image_reference(tmp_path, monkeypatch):
+    packet = tmp_path / "packet"
+    (packet / "photos").mkdir(parents=True)
+    Image.new("RGB", (2048, 1024)).save(packet / "photos" / "p1.jpg")
+    (packet / "manifest.json").write_text(
+        json.dumps({"photos": [{"id": "p1", "image": {"path": "photos/p1.jpg"}}]})
+    )
+    monkeypatch.setattr(sets, "ELECTRO", packet)
+    monkeypatch.setattr(sets, "ELECTRO_1024", tmp_path / "prepared")
+    sets.prepare_electro()
+    with Image.open(sets.image_path("electro", "p1")) as image:
+        assert image.size == (1024, 512)
+
+
+def isolated_function(filename, name, namespace):
+    tree = ast.parse(
+        (Path(__file__).resolve().parents[1] / "autodetect" / filename).read_text()
+    )
+    function = next(
+        n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name == name
+    )
+    module = compile(ast.Module(body=[function], type_ignores=[]), filename, "exec")
+    code = next(value for value in module.co_consts if isinstance(value, CodeType))
+    defaults = tuple(ast.literal_eval(value) for value in function.args.defaults)
+    return FunctionType(code, namespace, name, defaults)
+
+
+def test_elevation_render_creates_output_directory(tmp_path):
+    render = isolated_function(
+        "extent_gt.py",
+        "render",
+        {
+            "raster": lambda door: (np.zeros((2, 2)), (0, 0.02, 0, 0.02), None),
+            "np": np,
+            "Image": Image,
+            "ImageDraw": __import__("PIL.ImageDraw", fromlist=["Draw"]),
+            "RES": 0.01,
+            "DATA": tmp_path,
+        },
+    )
+    assert Path(render("test")).is_file()
+
+
+def test_dfine_train_creates_log_without_student_prepare(tmp_path):
+    torch = SimpleNamespace(
+        manual_seed=Mock(),
+        optim=SimpleNamespace(AdamW=lambda *a, **k: SimpleNamespace(param_groups=[])),
+    )
+    model = Mock()
+    model.train.return_value = model
+    model.named_parameters.return_value = []
+    train = isolated_function(
+        "dfine.py",
+        "train",
+        {
+            "SEED": 1,
+            "_device": lambda: "cpu",
+            "_model": lambda *a: model,
+            "BASE": "base",
+            "LR": 1,
+            "BACKBONE_LR_SCALE": 1,
+            "WEIGHT_DECAY": 1,
+            "ground_truth": lambda name: {},
+            "random": __import__("random"),
+            "time": __import__("time"),
+            "DATA": tmp_path,
+            "OUT": tmp_path / "student" / "dfine_small",
+            "json": json,
+        },
+    )
+    with pytest.MonkeyPatch.context() as patch:
+        patch.setitem(sys.modules, "torch", torch)
+        train(0)
+    assert (tmp_path / "student" / "dfine_small_train.log").is_file()

experiments/capture-contract/capture_contract.py

@@ -2,8 +2,11 @@
 from __future__ import annotations
 
 import argparse
-from hashlib import sha256
 import json
+import os
+import stat
+import tempfile
+from hashlib import sha256
 from math import hypot, isfinite
 from pathlib import Path
 
@@ -81,7 +84,8 @@ def validate_frame(frame):
     pose = camera_pose(frame.get("pose"), label + ".pose")
     vector([frame.get("timestamp")], 1, label + ".timestamp")
     if not isinstance(frame.get("tracking"), str):
-        raise ValueError(f"{label}: tracking must be explicit")
+        # Keep ValueError consistent with the public manifest-validation contract.
+        raise ValueError(f"{label}: tracking must be explicit")  # noqa: TRY004
     if not isinstance(frame.get("img"), str) or not frame["img"]:
         raise ValueError(f"{label}: image reference must be explicit")
     return width, height, intrinsics, pose
@@ -90,7 +94,7 @@ def validate_frame(frame):
 def ray_for_pixel(frame, pixel):
     width, height, (fx, fy, cx, cy), pose = validate_frame(frame)
     u, v = vector(pixel, 2, "pixel")
-    if not (0 <= u <= width and 0 <= v <= height):
+    if not (0 <= u < width and 0 <= v < height):
         raise ValueError("tap pixel lies outside its saved image")
     local = ((u-cx)/fx, -(v-cy)/fy, -1)
     # ARKit camera axes are columns; camera looks along -Z.
@@ -114,7 +118,8 @@ def crop_resize_intrinsics(intrinsics, crop_origin, scale):
     sx, sy = vector(scale, 2, "scale")
     if fx <= 0 or fy <= 0 or sx <= 0 or sy <= 0:
         raise ValueError("focal lengths and image scale must be positive")
-    return [fx*sx, fy*sy, (cx-x0)*sx, (cy-y0)*sy]
+    return list(vector([fx*sx, fy*sy, (cx-x0)*sx, (cy-y0)*sy],
+                       4, "resized intrinsics"))
 
 
 def validate_session(session):
@@ -134,7 +139,14 @@ def validate_session(session):
     if not frames:
         raise ValueError("no listed keyframes")
     tap_checks = []
+    tap_ids = set()
     for tap in session.get("taps", []):
+        tap_id = tap.get("id")
+        if not isinstance(tap_id, str) or not tap_id:
+            raise ValueError("tap: missing id")
+        if tap_id in tap_ids:
+            raise ValueError(f"duplicate tap id: {tap_id}")
+        tap_ids.add(tap_id)
         frame = frames.get(tap.get("keyframe"))
         if frame is None:
             raise ValueError(f"tap {tap.get('id')}: unknown keyframe")
@@ -190,6 +202,24 @@ def reject_constant(value):
     raise ValueError(f"invalid JSON numeric constant: {value}")
 
 
+def write_result(path, result):
+    try:
+        mode = stat.S_IMODE(path.stat().st_mode)
+    except FileNotFoundError:
+        mode = None
+    # Ordinary creation inside a private directory applies umask for a new result.
+    # Existing results retain their mode when the completed inode replaces them.
+    with tempfile.TemporaryDirectory(dir=path.parent) as stage:
+        temporary = Path(stage) / "result.json"
+        with temporary.open("w") as output:
+            output.write(json.dumps(result, indent=2, allow_nan=False) + "\n")
+            output.flush()
+            if mode is not None:
+                os.fchmod(output.fileno(), mode)
+            os.fsync(output.fileno())
+        temporary.replace(path)
+
+
 def main():
     parser = argparse.ArgumentParser(description=__doc__)
     parser.add_argument("session", type=Path)
@@ -205,7 +235,7 @@ def main():
         result["source_manifest_sha256"] = sha256(raw).hexdigest()
         result["image_reference_base"] = str(args.session.resolve().parent)
         args.out.parent.mkdir(parents=True, exist_ok=True)
-        args.out.write_text(json.dumps(result, indent=2, allow_nan=False) + "\n")
+        write_result(args.out, result)
     except (ValueError, TypeError, KeyError, AttributeError, OSError) as error:
         parser.exit(2, f"capture contract: {error}\n")
     print(f"Checked {len(result['keyframes'])} frame(s), {len(result['tap_checks'])} tap(s). "