diff --git a/experiments/autodetect/README.md b/experiments/autodetect/README.md index 06d6a227c..fade53d27 100644 --- a/experiments/autodetect/README.md +++ b/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 diff --git a/experiments/autodetect/autodetect/dfine.py b/experiments/autodetect/autodetect/dfine.py index 26ca6ebdc..fd1c1d31d 100644 --- a/experiments/autodetect/autodetect/dfine.py +++ b/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 = [] diff --git a/experiments/autodetect/autodetect/extent_gt.py b/experiments/autodetect/autodetect/extent_gt.py index 73d68ffd4..11688beea 100644 --- a/experiments/autodetect/autodetect/extent_gt.py +++ b/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) diff --git a/experiments/autodetect/autodetect/sets.py b/experiments/autodetect/autodetect/sets.py index cabbef6de..30bc0e6cf 100644 --- a/experiments/autodetect/autodetect/sets.py +++ b/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() diff --git a/experiments/autodetect/meter_brand/README.md b/experiments/autodetect/meter_brand/README.md index 4c98d889d..077177d0f 100644 --- a/experiments/autodetect/meter_brand/README.md +++ b/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. diff --git a/experiments/autodetect/meter_brand/brands.py b/experiments/autodetect/meter_brand/brands.py index 67091cfc6..2ce912d39 100644 --- a/experiments/autodetect/meter_brand/brands.py +++ b/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): diff --git a/experiments/autodetect/meter_brand/score.py b/experiments/autodetect/meter_brand/score.py index 62caf8889..290518bb2 100644 --- a/experiments/autodetect/meter_brand/score.py +++ b/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: diff --git a/experiments/autodetect/tests/test_review_regressions.py b/experiments/autodetect/tests/test_review_regressions.py new file mode 100644 index 000000000..99cb91869 --- /dev/null +++ b/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() diff --git a/experiments/capture-contract/capture_contract.py b/experiments/capture-contract/capture_contract.py index 9e2ea70bc..558d51273 100644 --- a/experiments/capture-contract/capture_contract.py +++ b/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). " diff --git a/experiments/capture-contract/test_contract.py b/experiments/capture-contract/test_contract.py index 38709b6a7..42872cd51 100644 --- a/experiments/capture-contract/test_contract.py +++ b/experiments/capture-contract/test_contract.py @@ -1,14 +1,22 @@ """Known-answer controls. These verify adapters, not device measurement accuracy.""" -from copy import deepcopy import json -from pathlib import Path -import tempfile +import os import subprocess import sys +import tempfile import unittest +from copy import deepcopy +from pathlib import Path +from unittest.mock import patch -from capture_contract import (camera_pose, crop_resize_intrinsics, - no_duplicates, ray_for_pixel, validate_session) +from capture_contract import ( + camera_pose, + crop_resize_intrinsics, + no_duplicates, + ray_for_pixel, + validate_session, + write_result, +) ROOT = Path(__file__).parent @@ -103,7 +111,7 @@ def test_duplicate_json_keys_are_rejected(self): json.loads('{"format": 1, "format": 2}',object_pairs_hook=no_duplicates) def test_taps_outside_saved_image_are_rejected(self): - for pixel in [[-100,10000],[-1,400],[500,801]]: + for pixel in [[-100,10000],[-1,400],[500,801],[1000,400],[500,800]]: session=deepcopy(self.session) session["taps"][0]["pixel"]=pixel with self.assertRaisesRegex(ValueError,"outside its saved image"): @@ -123,24 +131,73 @@ def test_integer_overflow_is_a_validation_error(self): with self.assertRaisesRegex(ValueError,"floating-point range"): validate_session(self.session) + def test_missing_and_duplicate_tap_ids_are_rejected(self): + for invalid in [None, "", 1]: + session = deepcopy(self.session) + session["taps"][0]["id"] = invalid + with self.assertRaisesRegex(ValueError, "missing id"): + validate_session(session) + self.session["taps"].append(deepcopy(self.session["taps"][0])) + with self.assertRaisesRegex(ValueError, "duplicate tap id"): + validate_session(self.session) + + def test_resizing_overflow_is_rejected(self): + with self.assertRaisesRegex(ValueError, "resized intrinsics.*finite"): + crop_resize_intrinsics([1e200, 1, 0, 0], [0, 0], [1e200, 1]) + + def test_failed_output_write_preserves_previous_result(self): + with tempfile.TemporaryDirectory() as temp: + out = Path(temp) / "converted.json" + out.write_text("previous result") + with (patch("capture_contract.os.fsync", side_effect=OSError("disk full")), + self.assertRaisesRegex(OSError, "disk full")): + write_result(out, {"new": "result"}) + self.assertEqual(out.read_text(), "previous result") + self.assertEqual(list(Path(temp).iterdir()), [out]) + write_result(out, {"new": "result"}) + self.assertEqual(json.loads(out.read_text()), {"new": "result"}) + + def test_atomic_replacement_preserves_existing_permissions(self): + with tempfile.TemporaryDirectory() as temp: + out = Path(temp) / "converted.json" + for mode in (0o644, 0o640, 0o600): + with self.subTest(mode=mode): + out.write_text("previous result") + out.chmod(mode) + write_result(out, {"new": "result"}) + self.assertEqual(out.stat().st_mode & 0o777, mode) + self.assertEqual(json.loads(out.read_text()), {"new": "result"}) + + def test_new_result_permissions_follow_umask(self): + with tempfile.TemporaryDirectory() as temp: + for mask in (0o022, 0o027): + with self.subTest(mask=mask): + out = Path(temp) / f"converted-{mask}.json" + previous = os.umask(mask) + try: + write_result(out, {"new": "result"}) + finally: + os.umask(previous) + self.assertEqual(out.stat().st_mode & 0o777, 0o666 & ~mask) + def test_cli_writes_only_after_validation(self): with tempfile.TemporaryDirectory() as temp: out=Path(temp)/"converted.json" cmd=[sys.executable,str(ROOT/"capture_contract.py"), str(ROOT/"fixtures/synthetic-session.json"),"--out",str(out)] - result=subprocess.run(cmd,capture_output=True,text=True) + result=subprocess.run(cmd,capture_output=True,text=True,check=False) self.assertEqual(result.returncode,0,result.stderr) converted=json.loads(out.read_text()) self.assertEqual(len(converted["source_manifest_sha256"]),64) invalid=Path(temp)/"bad.json";invalid.write_text('{"format": 5}') badout=Path(temp)/"not-created.json" result=subprocess.run([sys.executable,str(ROOT/"capture_contract.py"), - str(invalid),"--out",str(badout)],capture_output=True,text=True) + str(invalid),"--out",str(badout)],capture_output=True,text=True,check=False) self.assertEqual(result.returncode,2) self.assertFalse(badout.exists()) original=invalid.read_bytes() result=subprocess.run([sys.executable,str(ROOT/"capture_contract.py"), - str(invalid),"--out",str(invalid)],capture_output=True,text=True) + str(invalid),"--out",str(invalid)],capture_output=True,text=True,check=False) self.assertEqual(result.returncode,2) self.assertEqual(invalid.read_bytes(),original) diff --git a/experiments/device-field-test/README.md b/experiments/device-field-test/README.md index fbd383786..730febbbf 100644 --- a/experiments/device-field-test/README.md +++ b/experiments/device-field-test/README.md @@ -75,8 +75,9 @@ The 20 ft "Is this the … end of the wall?" prompt never appeared. `reach` ([`GuidancePlanner.swift`](https://github.com/SamGu-NRX/house-scanning-master/blob/ff95f1cb571e2f3ef56d9faae85880665ad0ca30/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GuidancePlanner.swift#L101-L117)) counts 6-inch cells outward from the meter. It stops at the first cell whose wall and ground are not both -covered. A cell is covered when each of its three sample rows has been seen from two positions: the wall -at 0, 3.25 and 6.5 ft up, and the ground at 0, 2 and 4 ft out. So if the ground right in front of the +covered or explicitly skipped. The replay ignores skipped cells and depth, so it cannot establish the +app's reach in every state. A cell is covered when each of its three sample rows has been seen from two +positions: the wall at 0, 3.25 and 6.5 ft up, and the ground at 0, 2 and 4 ft out. So if the ground right in front of the meter hasn't been seen twice, the tap puts the end at the meter. After that, the coverage map ignores everything past the ends: @@ -93,8 +94,8 @@ which is the ±0.328 ft the server echoes. This code is unchanged between the bu branch head `ff95f1c`. The replay shows how long the scan was exposed to this. Replaying the 44 photos in order through the -coverage rules, with no ends marked, the covered stretch next to the meter stays at 0 ft on both sides -until photo 43: +simplified no-depth, no-skip coverage rules, with no ends marked, the covered stretch next to the meter +stays at 0 ft on both sides until photo 43: ``` photo x (ft) reach left reach right @@ -143,7 +144,7 @@ These were visible: |---|---|---| | O-1 | Not reached | The camera opened with no iOS prompt; permission was granted in an earlier run | | F-2 | Pass | "Step a little closer to the wall" after an early tap | -| C-1 | Pass | The ring filled in under a second; the meter-number question followed | +| C-1 | Partially verified, haptic unverified | The ring filled in under a second; the meter-number question followed | | W-1 | Pass | Left, right, "Tilt down…" and "Take a step back" all appeared; the fog cleared after about 3 s | | W-2 | Pass | The photo counter flashes | | W-5 | Fail, B-16 | "Tap the ac unit" | @@ -269,7 +270,7 @@ The tester said the build and run, and "no haptic" once, but no test IDs. The ot |---|---|---| | O-1 | Not reached | No camera prompt; permission was granted earlier. A new Motion & Fitness prompt appeared on "Find your electric meter" (#43) | | F-2 | Unclear | Early taps were narrated, but no "Step a little closer to the wall" appeared | -| C-1 | Pass | The ring filled in about a second; "Meter number saved" followed | +| C-1 | Partially verified, haptic unverified | The ring filled in about a second; "Meter number saved" followed | | W-1 | Pass, confusing | All instructions appeared. After the right end was set, "Around 7 ft right" requests couldn't be met | | W-2 | Pass | The counter flashes; "there is no haptic" | | W-5 | Fail, B-16 | "Tap the ac unit" (#25) | @@ -278,7 +279,7 @@ The tester said the build and run, and "no haptic" once, but no test IDs. The ot | G-2 | Pass, copy bug | "I can't get there" uploaded at once, but "2 more views to finish" showed only one (#39) | | U-1 | Measured | 0.7 s for the first upload, 1.2 s after the gap request | | U-2 | Fail, known | The final upload stayed on "Sending measurements, 99%"; "Check clearances" never ticked (#31) | -| R-1 | Pass (haptic unverified) | First result with a spot: "An installer will take a look", 13 check lines | +| R-1 | Partially verified, haptic unverified | First result with a spot: "An installer will take a look", 13 check lines | | R-3 | Stayed put, wrong spot | Steady relative to the meter; drawn through the post at the wall's end | | R-4 | Fail, known | The battery is drawn over a person (#41) | @@ -309,6 +310,8 @@ Other things the recording showed: would unblock `reach` early. The strip could also show that an end will land at the meter before the tap. - **For the next runs:** the runbook now says to fill the strip next to the meter before any "Can't get there". Testers should say test IDs and haptics aloud, and share the scan off the phone before closing the app: a - relaunch deletes it. Runs 2 and 3 are still to do. + new-store cleanup on relaunch keeps the two newest completed scans with valid bundles, but can delete + older completed scans and unfinished scans without a bundle after pending bundle writes finish. Partial + or unreadable bundles are left alone. Runs 2 and 3 are still to do. Raw material stays out of git: the recording, the scan and the tape readings. diff --git a/experiments/device-field-test/RUNBOOK.md b/experiments/device-field-test/RUNBOOK.md index f547341a0..f5aa02fe4 100644 --- a/experiments/device-field-test/RUNBOOK.md +++ b/experiments/device-field-test/RUNBOOK.md @@ -32,17 +32,19 @@ for the `B-` numbers. "Known bug" means the triage already predicts the failure. ## Run 1: full flow, clean wall +O-3 Skip, F-3 a premature tap and F-2 a successful tap can leave their screens. Run each in a separate attempt, then restart before continuing the full flow. + | ID | Screen | Do | Expect | |---|---|---|---| -| O-1 | Onboarding | Page through with Next, tap Allow camera, then Allow | The iOS camera prompt appears. Note when | | O-2 | Onboarding | VoiceOver on, swipe the pages | Unknown: is the page number announced? | | O-3 | Onboarding | Reduce Motion on, tap Next and Skip | Known: Skip still slides | -| F-2 | Find meter | Tap the meter the moment the camera opens, then again | "Step a little closer to the wall"; known: no haptic on the second refusal | -| F-1 | Find meter | Pan across the wall and ground for 5 s, tap the meter | A ring blooms, a success haptic, then the close-up | +| O-1 | Onboarding | Page through with Next, tap Allow camera, then Allow | The iOS camera prompt appears. Note when | | F-3 | Find meter | Tap before the wall is detected | Question: does it pin at the wrong depth? | | F-4 | Find meter | Point at the sky until relocalizing | Question: "Point at the meter like this." shows no picture | -| C-1 | Close-up | Hold the meter in the circle | The ring fills in about 0.6 s, a light haptic, the walk begins 1.2 s later | +| F-2 | Find meter | Tap the meter the moment the camera opens, then again | "Step a little closer to the wall"; known: no haptic on the second refusal | +| F-1 | Find meter | Pan across the wall and ground for 5 s, tap the meter | A ring blooms, a success haptic, then the close-up | | C-2 | Close-up | Put the meter at the circle's edge | The drawn circle matches the captured area (device-only check) | +| C-1 | Close-up | Hold the meter in the circle | The ring fills in about 0.6 s, a light haptic, the walk begins 1.2 s later | | W-1 | Walk | Follow each instruction | "Walk slowly to your left", "Tilt down…", "Take a step back". Is the fog readable? | | W-2 | Walk | Watch the photo counter | It flashes green with no haptic | | W-3 | Walk | Mark something, pick a kind, tap | A firm haptic on accept | @@ -89,7 +91,7 @@ Put an obstacle in front of the meter or panel and repeat run 1. Add these: Group the tests into short recordings: W-12 then M-3; W-10 then W-11; U-4 then U-3; O-4 alone. A call or a lock can end a screen recording, and this way it takes only its own tests with it. O-4 deletes the app and -every scan on it, so export scans first. +every scan on it, so share each completed scan before O-4 and confirm the exported file exists. | ID | Do | Expect | |---|---|---| @@ -103,6 +105,10 @@ every scan on it, so export scans first. ## After the run +- On relaunch, a new scan store keeps the two newest completed scans with valid `scan.zip` bundles. + After pending bundle writes finish, cleanup can delete older completed scans and unfinished scans + without a bundle. Partial or unreadable bundles are left alone; export a run before reinstalling. + - Save the scan's `scene.json` and replay its coverage: ```sh diff --git a/experiments/device-field-test/build-4.1/run1-2000.md b/experiments/device-field-test/build-4.1/run1-2000.md index 60ead2559..5223ca695 100644 --- a/experiments/device-field-test/build-4.1/run1-2000.md +++ b/experiments/device-field-test/build-4.1/run1-2000.md @@ -31,7 +31,7 @@ Checks run: 1. **The app used a table top as the ground.** `groundBelow` takes the *highest* horizontal plane at least 0.3 m below the meter, with no check on its classification. It picked an ARKit plane classified `table`, 0.51 m below the meter, over the detected `floor` plane 1.47 m below it. So **every height in the scan is - about 3 ft 2 in too high above the real ground.** + about 3 ft 2 in too low because the selected ground plane is too high.** - The meter is exported at 1 ft 8 in up. The real height is about 4 ft 10 in. - The window's bottom is exported at 0 ft 2 in. The real height is about 3 ft 4 in. - The ground targets are drawn on the wall. The gap patches are drawn at exactly the folding table's top @@ -78,7 +78,7 @@ Checks run: | ID | Sev | Lane | Title | Existing issue | Evidence (video time) | |---|---|---|---|---|---| -| A41-1-01 | P0 | app | Ground height taken from a `table` plane: every height is ~3 ft 2 in too high | new | 3:13 narration; 3:44–3:46; 4:44–4:47; 5:58; packet planes | +| A41-1-01 | P0 | app | Ground height taken from a `table` plane: every exported height is ~3 ft 2 in too low | new | 3:13 narration; 3:44–3:46; 4:44–4:47; 5:58; packet planes | | A41-1-02 | P0 | app | AR "See it on your wall" draws nothing (regression from #41's RealityKit path) | new (regression of #41) | 6:10–6:25; 6:05 narration | | A41-1-03 | P1 | app | "Tilt down… at your meter" can't finish, blocks the walk and never escalates (151 s of 225 s) | #28 inverted | 0:33–4:13 | | A41-1-04 | P1 | app | Ground task re-asked 3 s after it was met: a ground re-estimate rebuilds coverage | new | 0:52–0:58 (`hi/ev_ground_reset.jpg`) | diff --git a/experiments/device-field-test/build-4.1/run2-2015.md b/experiments/device-field-test/build-4.1/run2-2015.md index c07f5ad70..d601621d2 100644 --- a/experiments/device-field-test/build-4.1/run2-2015.md +++ b/experiments/device-field-test/build-4.1/run2-2015.md @@ -4,7 +4,7 @@ Commit-safe copy: the meter serial is removed. Frames, transcript, scan zip and This is one of three threads analysing TestFlight 4.1. A later session compares all three and files issues. The **issue list** is the next section and uses stable IDs (`A41-2015-…`, named after the recording time so they can't clash -with the other threads). The last column maps each finding to the earlier 20:00 analysis (`private/device-test-4.1-2000/REPORT.md`, +with the other threads). The last column maps each finding to the earlier 20:00 analysis ([run1-2000.md](run1-2000.md), IDs `A41-1-…`) where one matches. ## Inputs and how they were analysed diff --git a/experiments/device-field-test/build-4.1/run3-2055.md b/experiments/device-field-test/build-4.1/run3-2055.md index afeb57857..02593a5ea 100644 --- a/experiments/device-field-test/build-4.1/run3-2055.md +++ b/experiments/device-field-test/build-4.1/run3-2055.md @@ -4,8 +4,8 @@ Commit-safe copy: the meter numbers are removed. The photos and video show the s This is one of three threads analysing TestFlight 4.1. A later session compares the three and files issues. The **issue list** below uses stable IDs (`A41-2055-…`, named after the recording so they can't clash with other threads). The last -column maps each finding to the earlier reports where one matches: 20:00 (`private/device-test-4.1-2000/REPORT.md`, -`A41-1-…`) and 20:15 (`private/device-test-4.1-2015/REPORT.md`, `A41-2015-…`). +column maps each finding to the earlier reports where one matches: 20:00 ([run1-2000.md](run1-2000.md), +`A41-1-…`) and 20:15 ([run2-2015.md](run2-2015.md), `A41-2015-…`). **This run is short and unfinished.** The tester was stuck on the first task for 73 s and said *"I'm just gonna end the test here"*. They then tapped "Can't get there" through every remaining task, got a result and shared the zip. The diff --git a/experiments/device-field-test/coverage_replay.py b/experiments/device-field-test/coverage_replay.py index 86d5db9df..8c79358f3 100644 --- a/experiments/device-field-test/coverage_replay.py +++ b/experiments/device-field-test/coverage_replay.py @@ -5,7 +5,7 @@ """Replay a House Scan scene.json's kept photos through the app's coverage rules. For each kept photo, in order, prints how far the covered wall reaches on each side of the -meter: the same `reach` that "Can't get there" uses to place a wall end. Then prints, for +meter under the simplified no-depth/no-skip rules below. Then prints, for the cells either side of the meter, which photos saw each sample row. The rules mirror `CoverageMap` and `GuidancePlanner.reach` on `t3/ios-mvf` at ff95f1c: @@ -16,7 +16,9 @@ - a cell is covered when every row was seen from two positions at least 0.25 m apart. Limits: it uses the exported (refined) wall line and meter for every photo, cannot see -tracking state, and ignores marked ends, so it shows what coverage would have been with no ends. +tracking state, ignores per-photo depth and explicitly skipped cells, and ignores marked ends. +The app accepts covered OR skipped cells for reach and uses depth when supplied. This replay +therefore does not establish reach equivalence in those states. uv run coverage_replay.py path/to/scene.json """ @@ -58,11 +60,22 @@ def load(path: Path): return origin, along, outward, cameras +def reach(covered: set[int], step: int) -> float: + index, cells = (0 if step > 0 else -1), 0 + while index in covered: + cells, index = cells + 1, index + step + return cells * CELL / FT + + def main() -> None: parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument("scene", type=Path) - parser.add_argument("--cells", type=int, default=45, help="cells to track on the right; 3 are tracked on the left") + parser.add_argument("--cells", type=int, default=45, help="cells to track on the right") + # Match the existing right extent so left reach is not capped at 1.5 ft. + parser.add_argument("--left-cells", type=int, default=45, help="cells to track on the left") args = parser.parse_args() + if args.cells <= 0 or args.left_cells <= 0: + parser.error("cell counts must be positive") origin, along, outward, cameras = load(args.scene) up = np.array([0.0, 1.0, 0.0]) @@ -89,16 +102,10 @@ def rows_seen(camera, band: str, index: int) -> list[int]: normal = outward if band == "wall" else up return [r for r, offset in enumerate(ROWS[band]) if all(sees(camera, point(band, s, offset), normal) for s in samples)] - indices = range(-3, args.cells) + indices = range(-args.left_cells, args.cells) positions = {(band, i): [[] for _ in ROWS[band]] for band in ROWS for i in indices} covered: set[int] = set() - def reach(step: int) -> float: - index, cells = (0 if step > 0 else -1), 0 - while index in covered: - cells, index = cells + 1, index + step - return cells * CELL / FT - print("photo x (ft) reach left reach right") for camera in cameras: position = camera[1][:3, 3] @@ -113,7 +120,7 @@ def reach(step: int) -> float: if all(len(row) >= 2 for band in ROWS for row in positions[(band, i)]): covered.add(i) x = np.dot(position - origin, along) / FT - print(f"{camera[0]} {x:7.2f} {reach(-1):8.1f} ft {reach(1):9.1f} ft") + print(f"{camera[0]} {x:7.2f} {reach(covered, -1):8.1f} ft {reach(covered, 1):9.1f} ft") print("\nPhotos that saw each row of the cells either side of the meter") for i in (-1, 0): diff --git a/experiments/device-field-test/test_coverage_replay.py b/experiments/device-field-test/test_coverage_replay.py new file mode 100644 index 000000000..75ac50297 --- /dev/null +++ b/experiments/device-field-test/test_coverage_replay.py @@ -0,0 +1,74 @@ +import json +import sys + +import coverage_replay as replay +import numpy as np + + +def test_covered_only_reach_differs_from_skipped_app_cells(): + # GuidancePlanner.reach at ff95f1c accepts both covered and skipped bands. + levels = { + 0: ("covered", "covered"), + 1: ("skipped", "covered"), + 2: ("covered", "covered"), + } + covered = { + i for i, bands in levels.items() if all(level == "covered" for level in bands) + } + app_done = { + i + for i, bands in levels.items() + if all(level in ("covered", "skipped") for level in bands) + } + assert replay.reach(covered, 1) == 0.5 + assert replay.reach(app_done, 1) == 1.5 + + +def test_synthetic_replay_left_extent_is_configurable(tmp_path, monkeypatch, capsys): + pose = np.eye(4) + angle = np.radians(-35) + pose[:3, :3] = [ + [1, 0, 0], + [0, np.cos(angle), -np.sin(angle)], + [0, np.sin(angle), np.cos(angle)], + ] + frames = [] + for i, x in enumerate([-1, -0.6]): + pose[:3, 3] = np.array([x, 2.5, 3]) / replay.FT + frames.append( + { + "id": f"k{i}", + "pose": pose.T.ravel().tolist(), + "intrinsics": [250, 250, 600, 600], + "w": 1200, + "h": 1200, + } + ) + scene = tmp_path / "scene.json" + scene.write_text( + json.dumps( + { + "walls": [{"baseline": [[0.0, 0.0], [10.0, 0.0]]}], + "meter": {"pos": [0, 3, 0]}, + "keyframes": frames, + } + ) + ) + monkeypatch.setattr( + sys, + "argv", + ["coverage_replay.py", str(scene), "--cells", "8", "--left-cells", "8"], + ) + replay.main() + last_photo = next( + line for line in capsys.readouterr().out.splitlines() if line.startswith("k1") + ) + assert float(last_photo.split()[2]) == 4.0 + monkeypatch.setattr( + sys, "argv", ["coverage_replay.py", str(scene), "--left-cells", "3"] + ) + replay.main() + last_photo = next( + line for line in capsys.readouterr().out.splitlines() if line.startswith("k1") + ) + assert float(last_photo.split()[2]) == 1.5 diff --git a/experiments/edge-geometry/README.md b/experiments/edge-geometry/README.md index 36d3563d8..401f42a1f 100644 --- a/experiments/edge-geometry/README.md +++ b/experiments/edge-geometry/README.md @@ -1,5 +1,7 @@ # Edge geometry +The tracked tables predate the match-rate denominator correction. Their M3 match rates exclude unpaired points and need a new run before use. + ## Question Clearances start at edges, and learned depth is 8 in or worse there. How much of that error lands along the wall, and how does it depend on the angle a photo sees the edge from? It compares four ways to place an edge (M0 to M3), plus the app's plane from one patch (M1-anchor), on ETH3D electro's laser truth. [METHODS.md](METHODS.md) defines them. diff --git a/experiments/edge-geometry/run.py b/experiments/edge-geometry/run.py index 2d62dc943..9371e990e 100644 --- a/experiments/edge-geometry/run.py +++ b/experiments/edge-geometry/run.py @@ -135,11 +135,17 @@ def cam_dirs(K: np.ndarray, uv: np.ndarray) -> np.ndarray: return np.c_[(uv[:, 0] - K[0, 2]) / K[0, 0], (uv[:, 1] - K[1, 2]) / K[1, 1], np.ones(len(uv))] +EVALS_COMMIT = "190ed339f41924ae0244e0b88c5ebe647ddd9eb4" + + def evals_commit() -> str: out = subprocess.run( ["git", "-C", str(EVALS_DIR), "rev-parse", "HEAD"], capture_output=True, text=True ) - return out.stdout.strip() or "unknown" + head = out.stdout.strip() + if out.returncode or head != EVALS_COMMIT: + raise SystemExit(f"evals harness at {EVALS_DIR} is at {head or 'unknown'}, expected {EVALS_COMMIT}") + return head def plane_members(cands, normals, planes) -> np.ndarray: @@ -744,7 +750,7 @@ def summarise_multi(m: dict) -> dict: }, "M3": { "pairs": int(pair.sum()), - "match_rate": round(float(matched.sum() / max(pair.sum(), 1)), 3), + "match_rate": round(float(matched.sum() / max(len(pair), 1)), 3), "match_px": pct(r["m3_px"][c][matched]), "frame_3d": stats.summary(m3_frame["3d"], j), "frame_along": stats.summary(m3_frame["along"], j), @@ -829,7 +835,7 @@ def grade(single: dict, multi: dict) -> dict: m2 = p90_in(ma["m2_span"][ma["in_plane"], 0]) pair = ma["m3_pair"] matched = pair & (ma["m3_ncc"] >= NCC_MIN) & np.isfinite(ma["m3_frame"]).all(axis=1) - rate = float(matched.sum() / max(pair.sum(), 1)) + rate = float(matched.sum() / max(len(pair), 1)) m3 = p90_in(np.linalg.norm(ma["m3_span"], axis=1)) return { "P1": {"m0_along_p90_in_0_15": near, "m0_along_p90_in_over_30": wide, "pass": bool(near <= 0.5 * wide)}, diff --git a/experiments/edge-geometry/tests/test_run_contracts.py b/experiments/edge-geometry/tests/test_run_contracts.py new file mode 100644 index 000000000..c060f53b5 --- /dev/null +++ b/experiments/edge-geometry/tests/test_run_contracts.py @@ -0,0 +1,37 @@ +import ast +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import Mock + +import numpy as np +import pytest + +import geom + + +def function(name, namespace): + path = Path(__file__).resolve().parents[1] / "run.py" + tree = ast.parse(path.read_text()) + node = next(n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name == name) + exec(compile(ast.Module(body=[node], type_ignores=[]), str(path), "exec"), namespace) + return namespace[name] + + +def test_harness_must_match_full_pinned_commit(): + expected = "190ed339f41924ae0244e0b88c5ebe647ddd9eb4" + run = Mock(return_value=SimpleNamespace(stdout=expected + "\n", returncode=0)) + check = function("evals_commit", {"subprocess": SimpleNamespace(run=run), "EVALS_DIR": Path("harness"), "EVALS_COMMIT": expected}) + assert check() == expected + for head, code in [("190ed33different", 0), ("", 128)]: + run.return_value = SimpleNamespace(stdout=head, returncode=code) + with pytest.raises(SystemExit, match="expected"): + check() + + +def test_unpaired_points_reduce_p4_match_rate(): + grade = function("grade", {"np": np, "geom": geom, "p90_in": lambda x: 1.0, "NCC_MIN": .8}) + single = {"angle": np.array([5., 40.]), "m0": np.zeros((2, 3)), "m1": np.zeros((2, 3)), "in_plane": np.ones(2, bool)} + ma = {"m2_span": np.zeros((4, 3)), "in_plane": np.ones(4, bool), "m3_pair": np.array([True, True, False, False]), "m3_ncc": np.ones(4), "m3_frame": np.zeros((4, 3)), "m3_span": np.zeros((4, 3))} + result = grade(single, {"modern_assumed": ma})["P4"] + assert result["match_rate"] == .5 + assert result["pass"] is False diff --git a/experiments/evals/Makefile b/experiments/evals/Makefile index cf8b0b31b..89f14580e 100644 --- a/experiments/evals/Makefile +++ b/experiments/evals/Makefile @@ -66,7 +66,8 @@ pose-priors: # (the tape survey, the team's map for `score import-measure-lab`, the rules file) it also writes # the learned-depth results files, imports the rig's own row with the scoring harness at SCORING # (experiments/scoring, PR #4) and scores every row in one table. -FIELD_OUT ?= $(DATA)/evals/field/results/$(basename $(notdir $(SESSION))) +# Discard partial stdout on failure so it cannot become an output path. +FIELD_OUT ?= $(shell out=$$(uv run python -m evals.field result-dir "$(SESSION)") && printf '%s\n' "$$out") RULES ?= field/rules.json # PR #4's harness, checked out read-only at a pinned commit outside the repo. The full SHA is # fetched by name first, because a shallow or single-branch clone doesn't have it. @@ -75,9 +76,15 @@ SCORING_CHECKOUT := $(DATA)/evals/scoring-77ce830 SCORING ?= $(SCORING_CHECKOUT)/experiments/scoring STAMPED_MAP = $(FIELD_OUT)/inputs/map.json field: + @# Resolve once here: result-dir validates and hashes the capture's contents. + $(eval FIELD_OUT := $(FIELD_OUT)) + @if [ -z "$(strip $(FIELD_OUT))" ]; then \ + echo "FIELD_OUT lookup failed or returned no path; refusing field work" >&2; exit 1; \ + fi @# The default checkout is created at the pinned commit and, when reused, must still be there @# with the harness unmodified. An overridden SCORING is the caller's choice and must exist. - @if [ "$(SCORING)" = "$(SCORING_CHECKOUT)/experiments/scoring" ]; then \ + @if [ -z "$(TRUTH)" ]; then :; \ + elif [ "$(SCORING)" = "$(SCORING_CHECKOUT)/experiments/scoring" ]; then \ if [ ! -d "$(SCORING_CHECKOUT)" ]; then \ { git -C ../.. cat-file -e $(SCORING_COMMIT)^{commit} 2>/dev/null || \ git -C ../.. fetch --no-tags origin $(SCORING_COMMIT); } && \ diff --git a/experiments/evals/README.md b/experiments/evals/README.md index 6357eea27..946d374dc 100644 --- a/experiments/evals/README.md +++ b/experiments/evals/README.md @@ -1,5 +1,7 @@ # Evals on real data +Saved photo-geometry and pose-prior summaries predate the edge-cohort, fixed-camera-center, and shared-pose-noise fixes. They need dataset and model regeneration before use. The old Map3D report's CoverageMap comparison lacks a validated matching baseline and is no longer generated. + **Question.** Can a phone's photos and AR tracking measure a house wall well enough for placement: about 4 in at p90 on 1 to 3 m spans? And does the app's coverage map claim only wall it saw (false-observed at most 0.5 ft)? Each bar was fixed before its run. **Run.** `cd experiments/evals && uv sync && make test`; each result has its own target (`make drift`, `make recon`, `make coverage`, …). Model runs need an Apple-silicon Mac, and the app-code checks need macOS 26 with Swift 6.2. [METHODS.md](METHODS.md) lists every target, prerequisite and dataset sha256. diff --git a/experiments/evals/evals/ar_poses.py b/experiments/evals/evals/ar_poses.py index 3cd6e2546..be932b422 100644 --- a/experiments/evals/evals/ar_poses.py +++ b/experiments/evals/evals/ar_poses.py @@ -4,7 +4,7 @@ - `exact`: the true poses, the best any pose prior can do. - `advio_2018`: the errors measured in `evals.drift` on ADVIO's 2018 iPhone 6s. Scale: each - group of photos takes one walk's ARKit scale against the ground truth rescaled to GPS, in turn + seed takes one walk's ARKit scale against the ground truth rescaled to GPS, by seed identity (`arkit_scale_vs_truth_gps` in results/advio_drift.json, walks 20 to 22: 0.838, 0.943, 0.951; the median displacement ratio over 10 to 30 ft windows). ARCore is not the reference: on walk 20 it reads 4% short of GPS itself (0.802 vs 0.835 of the truth). Position noise: ARKit's own @@ -17,7 +17,7 @@ Scale error stretches every camera's offset from the group's first camera; noise is then added independently per camera. Rotation noise turns each camera about its own centre. Each setting with -noise is drawn `NOISE_DRAWS` times (draw 0 is the one MapAnything was run on); the scale a group +noise is drawn `NOISE_DRAWS` times (draw 0 is used for new MapAnything runs); the scale a group gets does not change between draws. """ @@ -30,7 +30,7 @@ @dataclass(frozen=True) class PoseError: - scales: tuple[float, ...] # one per group, taken in turn + scales: tuple[float, ...] # selected by seed identity, shared across view counts position_sigma_m: float rotation_sigma_deg: float # How far a correct match may reproject with these poses: 2 px of feature noise plus what the @@ -87,22 +87,21 @@ def degrade( def group_poses( views: dict, groups: dict[str, list[list[str]]], setting: str, draw: int = 0 ) -> dict[str, dict]: - """{group id: {"scale": applied scale, "poses": {view: 4x4 cam-to-world}}} for every group, - deterministic per setting and draw. Draw 0 keeps the generator the first version used, so it - reproduces the poses MapAnything was given.""" + """Pose noise keyed by setting, draw, seed and camera, shared by nested view groups.""" error = SETTINGS[setting] - rng = np.random.default_rng(_seed(setting) if draw == 0 else [_seed(setting), draw]) out: dict[str, dict] = {} - k = 0 for n, members_list in groups.items(): for members in members_list: - scale = error.scales[k % len(error.scales)] - k += 1 - poses = degrade([views[m].cam_to_world for m in members], scale, error, rng) - out[f"n{n}-{members[0]}"] = { - "scale": scale, - "poses": {m: T.tolist() for m, T in zip(members, poses, strict=True)}, - } + seed = members[0] + scale = error.scales[_seed(seed) % len(error.scales)] + poses = {} + for member in members: + rng = np.random.default_rng([_seed(setting), draw, _seed(seed), _seed(member)]) + T = views[member].cam_to_world.copy() + origin = views[seed].cam_to_world[:3, 3] + T[:3, 3] = origin + scale * (T[:3, 3] - origin) + poses[member] = degrade([T], 1.0, error, rng)[0].tolist() + out[f"n{n}-{seed}"] = {"scale": scale, "poses": poses} return out diff --git a/experiments/evals/evals/datasets.py b/experiments/evals/evals/datasets.py index 14c8d4739..70e8bf9fe 100644 --- a/experiments/evals/evals/datasets.py +++ b/experiments/evals/evals/datasets.py @@ -178,7 +178,11 @@ def record_path(archive: Archive) -> Path: def write_record(archive: Archive, names: list[str]) -> None: """Record the unpacked files once every expected one is present: the marker, and at least one file under each requested member.""" - files = {n: (archive.dest / n).stat().st_size for n in names if (archive.dest / n).is_file()} + files = { + n: {"bytes": (archive.dest / n).stat().st_size, "sha256": sha256_of(archive.dest / n)} + for n in names + if (archive.dest / n).is_file() + } missing = [m for m in archive.members if not any(n.startswith(m) for n in files)] if not (archive.dest / archive.marker).exists(): missing.append(archive.marker) @@ -191,19 +195,34 @@ def write_record(archive: Archive, names: list[str]) -> None: def is_complete(archive: Archive) -> bool: """True when this archive's record matches its pinned sha256 and every recorded file exists at - its recorded size. Sizes catch a truncated or partly deleted copy without rehashing gigabytes; - an edit that keeps a file's size is not caught.""" - path = record_path(archive) - if not path.exists(): - return False - record = json.loads(path.read_text()) - if record.get("sha256") != archive.sha256 or not record.get("files"): - return False - for name, size in record["files"].items(): - f = archive.dest / name - if not f.is_file() or f.stat().st_size != size: + its recorded size and content hash. Old size-only records require a fresh verified extraction.""" + try: + record = json.loads(record_path(archive).read_text()) + if not isinstance(record, dict) or record.get("sha256") != archive.sha256: + return False + files = record.get("files") + if not isinstance(files, dict) or not files: return False - return True + for name, identity in files.items(): + if ( + Path(name).is_absolute() + or ".." in Path(name).parts + or not isinstance(identity, dict) + or type(identity.get("bytes")) is not int + or identity["bytes"] < 0 + or not isinstance(identity.get("sha256"), str) + ): + return False + f = archive.dest / name + if ( + not f.is_file() + or f.stat().st_size != identity["bytes"] + or sha256_of(f) != identity["sha256"] + ): + return False + return True + except (OSError, ValueError): + return False def main() -> None: @@ -216,7 +235,7 @@ def main() -> None: ) args = ap.parse_args() for a in ADVIO if args.dataset == "advio" else ETH3D: - if is_complete(a) and not args.force: + if not args.force and is_complete(a): print(f"{a.name}: already unpacked and recorded complete ({record_path(a)})") continue fetch(a) diff --git a/experiments/evals/evals/field.py b/experiments/evals/evals/field.py index 42f010cd9..3a582ebe3 100644 --- a/experiments/evals/evals/field.py +++ b/experiments/evals/evals/field.py @@ -32,6 +32,7 @@ import re import shutil import sys +import tempfile import time import zipfile from dataclasses import dataclass @@ -111,6 +112,13 @@ def file_sha256(path: Path) -> str: return h.hexdigest() +def write_cache_manifest(path: Path, manifest: dict) -> None: + with tempfile.TemporaryDirectory(dir=path.parent) as directory: + temporary = Path(directory) / "manifest.json" + temporary.write_text(json.dumps(manifest), encoding="utf-8") + temporary.replace(path) + + def check_zip_directory(session: Path) -> None: """Refuse a zip whose central directory is too large or lists too many entries, reading only its end record. `zipfile.ZipFile` reads the whole directory and builds every entry while it is @@ -149,6 +157,7 @@ def unpack(session: Path) -> tuple[Path, str | None]: check_zip_directory(session) digest = file_sha256(session) out = FIELD_DIR / digest[:16] + manifest_path = out.with_suffix(".members.json") with zipfile.ZipFile(session) as z: infos = z.infolist() if len(infos) > MAX_ZIP_ENTRIES: @@ -171,7 +180,25 @@ def unpack(session: Path) -> tuple[Path, str | None]: if len(roots) != 1: raise ValueError(f"{session}: expected one session.json, found {roots}") root = out / roots[0] - if not root.exists(): + try: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + except (OSError, ValueError): + manifest = None + # Hashes recorded while extracting avoid decompressing the ZIP in every field stage. + # Bind them to the full archive digest and still hash cached files to detect local edits. + hashes = manifest.get("files") if isinstance(manifest, dict) else None + intact = ( + isinstance(hashes, dict) + and manifest.get("archive_sha256") == digest + and set(hashes) == {str(rel) for _, rel in members} + and root.exists() + and all( + (out.joinpath(*rel.parts)).is_file() + and file_sha256(out.joinpath(*rel.parts)) == hashes[str(rel)] + for _, rel in members + ) + ) + if not intact: FIELD_DIR.mkdir(parents=True, exist_ok=True) free = shutil.disk_usage(FIELD_DIR).free if free - declared < MIN_FREE_AFTER_UNPACK: @@ -183,17 +210,22 @@ def unpack(session: Path) -> tuple[Path, str | None]: shutil.rmtree(tmp, ignore_errors=True) try: written = 0 + hashes = {} for info, rel in members: dest = tmp.joinpath(*rel.parts) dest.parent.mkdir(parents=True, exist_ok=True) + member_hash = hashlib.sha256() with z.open(info) as src, dest.open("wb") as dst: while block := src.read(1 << 20): written += len(block) if written > MAX_ZIP_BYTES: raise ValueError(f"{session}: expands past {MAX_ZIP_BYTES} bytes") dst.write(block) + member_hash.update(block) + hashes[str(rel)] = member_hash.hexdigest() shutil.rmtree(out, ignore_errors=True) tmp.rename(out) + write_cache_manifest(manifest_path, {"archive_sha256": digest, "files": hashes}) except BaseException: shutil.rmtree(tmp, ignore_errors=True) raise @@ -261,6 +293,9 @@ def safe_id(value: str, what: str) -> str: return value +# A 1 MiB header budget bounds malformed JPEG work. No dataset calibration supports this limit. +MAX_JPEG_HEADER_BYTES = 1024 * 1024 + # JPEG start-of-frame markers carry the image size; C4 (DHT), C8 (JPG) and CC (DAC) share the range # but are not frames. _JPEG_SOF = set(range(0xC0, 0xD0)) - {0xC4, 0xC8, 0xCC} @@ -276,14 +311,14 @@ def image_size(path: Path) -> tuple[int, int]: if head[:2] != b"\xff\xd8": raise ValueError(f"{path}: not a JPEG or PNG image") f.seek(2) - while True: + while f.tell() < MAX_JPEG_HEADER_BYTES: byte = f.read(1) if not byte: break if byte != b"\xff": - continue + break marker = f.read(1) - while marker == b"\xff": # fill bytes before a marker + while marker == b"\xff" and f.tell() < MAX_JPEG_HEADER_BYTES: # bounded fill bytes marker = f.read(1) if not marker: break @@ -292,8 +327,12 @@ def image_size(path: Path) -> tuple[int, int]: continue if m in (0xD9, 0xDA): # end of image, or start of scan before any frame header break + if f.tell() + 2 > MAX_JPEG_HEADER_BYTES: + break length = int.from_bytes(f.read(2), "big") if m in _JPEG_SOF: + if f.tell() + 5 > MAX_JPEG_HEADER_BYTES: + break frame = f.read(5) if len(frame) == 5: return int.from_bytes(frame[3:5], "big"), int.from_bytes(frame[1:3], "big") @@ -309,9 +348,11 @@ def read_session_image(path: Path, flags: int = cv2.IMREAD_COLOR) -> np.ndarray: w, h = image_size(path) if w <= 0 or h <= 0 or w * h > MAX_IMAGE_PIXELS: raise ValueError(f"{path}: {w} x {h} pixels, more than {MAX_IMAGE_PIXELS} or empty") - img = cv2.imread(str(path), flags) + img = cv2.imread(str(path), flags | cv2.IMREAD_IGNORE_ORIENTATION) if img is None: raise ValueError(f"{path}: OpenCV could not decode it") + if img.shape[:2] != (h, w): + raise ValueError(f"{path}: decoded size differs from header {w} x {h}") return img @@ -759,11 +800,43 @@ def score( out_dir: Path, tap_error_in: float = TAP_ERROR_IN, ) -> str: + session_path = Path(session_path).resolve() + truth_path, map_path, rules_path = ( + Path(path).resolve() if path is not None else None + for path in (truth_path, map_path, rules_path) + ) + out_dir = out_dir.resolve() + scored = (out_dir / "scored").resolve() + generated = {(out_dir / name).resolve() for name in (*ROW_FILES, "field_report.md")} + # Check before any cleanup: callers can keep an archive or survey under the output root. + for path in (session_path, truth_path, map_path, rules_path): + if ( + path is not None + and path.is_dir() + and any( + path.is_relative_to(target) or target.is_relative_to(path) + for target in (out_dir, scored, *generated) + ) + ): + raise ValueError( + f"directory input {path} overlaps scoring output cleanup; choose another out-dir" + ) + if path is not None and (path.is_relative_to(scored) or path in generated): + raise ValueError( + f"input {path} overlaps scoring output cleanup; choose another out-dir" + ) # First: a re-run, even one that stops early below, must not leave an earlier run's rows # for `score` to pick up. out_dir.mkdir(parents=True, exist_ok=True) for name in ROW_FILES: (out_dir / name).unlink(missing_ok=True) + (out_dir / "field_report.md").unlink(missing_ok=True) + scored_path = out_dir / "scored" + if scored_path.is_symlink(): + scored_path.unlink() + else: + shutil.rmtree(scored_path, ignore_errors=True) + scored_path.mkdir(parents=True, exist_ok=True) folder, capture = unpack(session_path) session = load_session(folder) out = work_dir(folder) @@ -829,8 +902,10 @@ def depth_of(k): f"{map_path}: map is for session {mapping.get('session')!r}, the capture is " f"{session['session']['id']!r}; run `stamp` first" ) - if capture not in truth.get("captures", []): - raise ValueError(f"{truth_path}: survey does not list capture {capture}; run `stamp` first") + if set(truth.get("captures", [])) != {capture}: + raise ValueError( + f"{truth_path}: survey captures must contain only {capture}; run `stamp` first" + ) rules_sha = hashlib.sha256(Path(rules_path).read_bytes()).hexdigest() ref = mapping["measurements"].get(truth["scale_reference"]) tape = {m["id"]: m for m in truth["measurements"]}.get(truth["scale_reference"]) @@ -880,11 +955,16 @@ def depth_of(k): return "\n".join(lines) + "\n" +def result_dir(session: Path) -> Path: + folder, digest = unpack(session) + return FIELD_DIR / "results" / capture_id(folder, digest) + + def main() -> None: ap = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter ) - ap.add_argument("step", choices=["stamp", "prepare", "score"]) + ap.add_argument("step", choices=["stamp", "prepare", "score", "result-dir"]) ap.add_argument("session", type=Path, help="session zip shared from Measure Lab, or its folder") ap.add_argument("--truth", type=Path) ap.add_argument("--map", type=Path) @@ -892,13 +972,16 @@ def main() -> None: ap.add_argument("--out-dir", type=Path, help="default: a folder per session under the data dir") ap.add_argument("--tap-error-in", type=float, default=TAP_ERROR_IN) args = ap.parse_args() - out_dir = args.out_dir or FIELD_DIR / "results" / args.session.stem - if args.step == "stamp": - print(stamp(args.session, args.truth, args.map, out_dir)) + if args.step == "result-dir": + print(result_dir(args.session)) return if args.step == "prepare": print(prepare(*unpack(args.session))) return + out_dir = args.out_dir or result_dir(args.session) + if args.step == "stamp": + print(stamp(args.session, args.truth, args.map, out_dir)) + return report = score(args.session, args.truth, args.map, args.rules, out_dir, args.tap_error_in) out_dir.mkdir(parents=True, exist_ok=True) (out_dir / "field_report.md").write_text(report) diff --git a/experiments/evals/evals/frames.py b/experiments/evals/evals/frames.py index 19edd4e23..2a7c8befe 100644 --- a/experiments/evals/evals/frames.py +++ b/experiments/evals/evals/frames.py @@ -52,7 +52,8 @@ def normals_from_depth(depth: np.ndarray, K: np.ndarray) -> np.ndarray: def view_geometry(scene: Scene, name: str) -> tuple[float, float]: """Median distance (m) to the view's scored points and median incidence angle (degrees).""" v = scene.views[name] - index, uv, _ = scene.visible(name) + index, uv, edge = scene.visible(name) + index, uv = index[~edge], uv[~edge] Xc = scene.cands[index] @ v.R_wc.T + v.t_wc dist = np.linalg.norm(Xc, axis=1) depth, K = scene.oracle_depth(name) diff --git a/experiments/evals/evals/map3d.py b/experiments/evals/evals/map3d.py index e8ea8dc18..27dc97d91 100644 --- a/experiments/evals/evals/map3d.py +++ b/experiments/evals/evals/map3d.py @@ -169,7 +169,7 @@ def span_columns(lo: float, hi: float) -> tuple[np.ndarray, np.ndarray]: def within_bar(false_observed_ft: float) -> bool: """The pre-registered 0.5 ft bar, inclusive. Rounding to a micro-foot keeps a claim of exactly the bar from failing on floating-point sums of column widths.""" - return round(false_observed_ft, 6) <= PASS_FT + return bool(round(false_observed_ft, 6) <= PASS_FT) def space_claims(setup: Setup, spans: list[dict], kind: str, tol_abs: float = HIDE_ABS_M) -> dict: @@ -381,7 +381,6 @@ def evaluate() -> dict: def markdown(r: dict) -> str: b = r["bands"] - cm = json.loads((RESULTS / "coverage.json").read_text())[0]["bands"]["wall"] c = r["chain"] lines = [ "# The app's 3D map against laser visibility (generated by `make map3d`)", @@ -394,8 +393,6 @@ def markdown(r: dict) -> str: "", "| Band | Claimed | False-observed (share) | Pass | Missed | Seen from 2 positions |", "| --- | --- | --- | --- | --- | --- |", - f"| wall, CoverageMap (section 7) | {cm['claimed_ft']:.1f} | {cm['false_observed_ft']:.1f} " - f"({100 * cm['false_observed_share']:.0f}%) | no | {cm['missed_ft']:.1f} | {cm['seen_two_view_ft']:.1f} |", ] for label, bands in (("", b), (", 5 m truth", r["bands_5m_truth"])): for band in ("wall", "ground"): diff --git a/experiments/evals/evals/pose_priors.py b/experiments/evals/evals/pose_priors.py index 7622cbef0..3095d5b6d 100644 --- a/experiments/evals/evals/pose_priors.py +++ b/experiments/evals/evals/pose_priors.py @@ -243,7 +243,9 @@ def pv(name): label = mapanything_label( root / f"ma392_{setting}", setting, pose_file(scene.name, setting) ) - methods[f"mapanything@392 + {label}"] = [model_frame_method(root / f"ma392_{setting}")] + methods[f"mapanything@392 + {label}"] = [ + model_frame_method(root / f"ma392_{setting}", centred=True) + ] for setting in SETTINGS: draws = noise_draws(setting) methods[f"MoGe-2 placed with {setting} poses"] = [ diff --git a/experiments/evals/evals/recon.py b/experiments/evals/evals/recon.py index bbe6f52ec..8344b569d 100644 --- a/experiments/evals/evals/recon.py +++ b/experiments/evals/evals/recon.py @@ -387,7 +387,7 @@ def pv(name): return methods -def model_frame_method(root: Path) -> Method: +def model_frame_method(root: Path, centred: bool = False) -> Method: """A multi-view model's own output per group, in the frame and scale it chose.""" def method(gid, members): @@ -397,7 +397,7 @@ def method(gid, members): def pv(name): d, K, T = load_prediction(run / f"{name}.npz") - return d, K, T, False + return d, K, T, centred return pv diff --git a/experiments/evals/models/README.md b/experiments/evals/models/README.md index 33f9f84f9..f0321fda8 100644 --- a/experiments/evals/models/README.md +++ b/experiments/evals/models/README.md @@ -1,6 +1,8 @@ # Depth and geometry model runners -Runs three commercially licensed models on a list of photos and writes metric depth in one format, so the wall-reconstruction eval (question 2 in `../README.md`) can score each against ETH3D's laser scans. +Runs three models on a list of photos and writes metric depth in one format, so the wall-reconstruction eval (question 2 in `../README.md`) can score each against ETH3D's laser scans. + +MoGe-2 is excluded from commercial-use claims until its checkpoint licence is verified; see the licence table below. ## Install @@ -51,7 +53,7 @@ Network outputs are resampled to the input grid bilinearly. A pixel is valid onl | `--model` | Checkpoint (revision) | License | Code (commit) | | --- | --- | --- | --- | -| `moge2` | [`Ruicheng/moge-2-vitl-normal`](https://huggingface.co/Ruicheng/moge-2-vitl-normal) (`cb0e8bb`) | MIT | microsoft/MoGe `0744441` | +| `moge2` | [`Ruicheng/moge-2-vitl-normal`](https://huggingface.co/Ruicheng/moge-2-vitl-normal) (`cb0e8bb`) | weights unverified; MoGe code MIT; DINOv2 code Apache-2.0 | [microsoft/MoGe `0744441`](https://github.com/microsoft/MoGe/blob/07444410f1e33f402353b99d6ccd26bd31e469e8/README.md#license) | | `da3metric` | [`depth-anything/DA3METRIC-LARGE`](https://huggingface.co/depth-anything/DA3METRIC-LARGE) (`4010e39`) | Apache-2.0 | ByteDance-Seed/Depth-Anything-3 `3d835ec` | | `mapanything` | [`facebook/map-anything-apache`](https://huggingface.co/facebook/map-anything-apache) (`00f9c24`) | Apache-2.0 | facebookresearch/map-anything `3d10cf7` | diff --git a/experiments/evals/models/common.py b/experiments/evals/models/common.py index 060c94887..55a5b5457 100644 --- a/experiments/evals/models/common.py +++ b/experiments/evals/models/common.py @@ -162,8 +162,10 @@ def read_poses(path: Path, images: list[Path]) -> list[np.ndarray]: T = np.asarray(entry, dtype=np.float64) if T.shape != (4, 4) or not np.all(np.isfinite(T)): raise ValueError(f"{path}: pose for {p.stem} must be a 4x4 matrix") - if not np.allclose(T[3], [0, 0, 0, 1]) or not np.allclose( - T[:3, :3].T @ T[:3, :3], np.eye(3), atol=1e-4 + if ( + not np.allclose(T[3], [0, 0, 0, 1]) + or not np.allclose(T[:3, :3].T @ T[:3, :3], np.eye(3), atol=1e-4) + or not np.isclose(np.linalg.det(T[:3, :3]), 1.0, atol=1e-4) ): raise ValueError(f"{path}: pose for {p.stem} is not a rigid transform: {T.tolist()}") out.append(T) diff --git a/experiments/evals/models/moge2.py b/experiments/evals/models/moge2.py index bb2624f01..f054ef4a3 100644 --- a/experiments/evals/models/moge2.py +++ b/experiments/evals/models/moge2.py @@ -34,7 +34,7 @@ "repo": "https://github.com/microsoft/MoGe", "commit": "07444410f1e33f402353b99d6ccd26bd31e469e8", } -LICENSE = "MIT" +LICENSE = "weights unverified; code MIT" RESOLUTION_LEVEL = 9 # MoGe's default: 3600 ViT tokens, the top of its 1200-3600 range. diff --git a/experiments/evals/models/run.py b/experiments/evals/models/run.py index db89aa64a..15e904da2 100644 --- a/experiments/evals/models/run.py +++ b/experiments/evals/models/run.py @@ -13,12 +13,15 @@ from __future__ import annotations import argparse +import fcntl import importlib import json import platform import shutil import sys +import tempfile import time +from contextlib import contextmanager from pathlib import Path from models.common import ( @@ -112,6 +115,11 @@ def pick_device(requested: str) -> str: def main(argv: list[str] | None = None) -> None: args = parse_args(argv) + with output_lock(args.out): + run_model(args) + + +def run_model(args) -> None: caches = set_cache_dirs() images = read_image_list(args.images) @@ -133,44 +141,138 @@ def main(argv: list[str] | None = None) -> None: # Everything is written to a staging folder and swapped in only when the whole run succeeds, # so a run that stops midway (out of memory, cancelled) leaves the previous run whole instead of # a mix of its depth maps and new ones under the old run.json. - stage = args.out.with_name(args.out.name + ".staging") - shutil.rmtree(stage, ignore_errors=True) - stage.mkdir(parents=True) - try: + with staging_directory(args.out) as stage: run_into(args, stage, images, intrinsics, poses, caches, module, device, torch) - except BaseException: - shutil.rmtree(stage, ignore_errors=True) - raise - publish(stage, args.out) + publish(stage, args.out) print(f"wrote the run's npz files and run.json to {args.out}", file=sys.stderr) -def publish(stage: Path, out: Path) -> None: - """Replace `out` with the complete run in `stage`, all or nothing. +def directory_identity(path: Path) -> list[int] | None: + try: + stat = path.lstat() + except FileNotFoundError: + return None + return [stat.st_dev, stat.st_ino] + + +@contextmanager +def output_lock(out: Path, *, wait: bool = False): + """One writer per output; a hard exit releases the OS lock before the next recovery.""" + out.parent.mkdir(parents=True, exist_ok=True) + # Keep this file: unlinking it lets two processes lock different inodes at the same path. + with out.with_name("." + out.name + ".runner.lock").open("a") as lock: + try: + fcntl.flock(lock, fcntl.LOCK_EX | (0 if wait else fcntl.LOCK_NB)) + except BlockingIOError as e: + raise RuntimeError(f"another model runner is writing {out}") from e + recover_publication(out) + recover_staging(out) + yield + + +def recover_staging(out: Path) -> None: + owner = out.with_name("." + out.name + ".staging.json") + if not owner.exists(): + return + try: + record = json.loads(owner.read_text(encoding="utf-8")) + name, identity = record["name"], record["identity"] + if ( + not isinstance(name, str) + or Path(name).name != name + or not name.startswith(out.name + ".staging-") + or not isinstance(identity, list) + or len(identity) != 2 + or any(type(value) is not int for value in identity) + ): + raise ValueError("invalid staged directory identity") + except (OSError, ValueError, KeyError, TypeError) as e: + raise RuntimeError(f"invalid staging record {owner}; inspect it by hand") from e + stage = out.parent / name + current = directory_identity(stage) + if current is not None: + if current != identity: + raise RuntimeError(f"staging directory differs from {owner}; inspect it by hand") + shutil.rmtree(stage) + owner.unlink() + - The previous run is set aside as `out.previous` and put back if the swap fails. A - `.previous` is deleted only when a complete run (with its run.json) is published at `out`. A - `.previous` with nothing at `out` is a run a killed publication set aside, and is restored - before anything else, so a failed publication never loses the last complete run.""" +@contextmanager +def staging_directory(out: Path): + """Called under output_lock; record ownership before any prediction data is written.""" + stage = Path(tempfile.mkdtemp(prefix=out.name + ".staging-", dir=out.parent)) + owner = out.with_name("." + out.name + ".staging.json") + try: + record = {"name": stage.name, "identity": directory_identity(stage)} + with tempfile.TemporaryDirectory(dir=out.parent) as tmp: + temporary = Path(tmp) / "owner.json" + temporary.write_text(json.dumps(record), encoding="utf-8") + owner.hardlink_to(temporary) + except BaseException: + shutil.rmtree(stage) + raise + try: + yield stage + finally: + if stage.exists(): + shutil.rmtree(stage) + owner.unlink() + + +def recover_publication(out: Path) -> None: + """Restore or clean a runner-owned backup after interruption, preserving unknown siblings.""" previous = out.with_name(out.name + ".previous") - if previous.exists(): - if (out / "run.json").exists(): - shutil.rmtree(previous) # a later publication completed; this one is superseded - elif not out.exists(): + owner = out.with_name(out.name + ".publication.json") + if directory_identity(owner) is None: + if directory_identity(previous) is not None: + raise RuntimeError(f"refusing to modify existing backup directory {previous}") + return + try: + record = json.loads(owner.read_text(encoding="utf-8")) + old, new = record["old"], record["new"] + identities = (new,) if old is None else (old, new) + for identity in identities: + if ( + not isinstance(identity, list) + or len(identity) != 2 + or any(type(value) is not int for value in identity) + ): + raise ValueError("invalid directory identity") + except (OSError, ValueError, KeyError, TypeError) as e: + raise RuntimeError(f"invalid publication record {owner}; inspect it by hand") from e + current_id, previous_id = directory_identity(out), directory_identity(previous) + if previous_id is not None: + if previous_id != old or current_id not in (None, new): + raise RuntimeError(f"publication directories differ from {owner}; inspect them by hand") + if current_id is None: previous.rename(out) else: - raise RuntimeError( - f"both {out} (without run.json) and {previous} exist; inspect them by hand" - ) - if out.exists(): - out.rename(previous) + shutil.rmtree(previous) + elif current_id not in (old, new): + raise RuntimeError(f"publication directory differs from {owner}; inspect it by hand") + owner.unlink() + + +def publish(stage: Path, out: Path) -> None: + """Publish a complete run, preserving the old directory if its replacement fails.""" + recover_publication(out) + previous = out.with_name(out.name + ".previous") + owner = out.with_name(out.name + ".publication.json") + # Record directory identities before either rename, so recovery cannot mistake a caller's + # .previous sibling for the runner's backup. Publish the record atomically as well. + record = {"old": directory_identity(out), "new": directory_identity(stage)} + with tempfile.TemporaryDirectory(prefix=out.name + ".publication-", dir=out.parent) as tmp: + temporary = Path(tmp) / "directories.json" + temporary.write_text(json.dumps(record), encoding="utf-8") + owner.hardlink_to(temporary) try: + if out.exists(): + out.rename(previous) stage.rename(out) except BaseException: - if previous.exists() and not out.exists(): - previous.rename(out) + recover_publication(out) raise - shutil.rmtree(previous, ignore_errors=True) + recover_publication(out) def run_into(args, out: Path, images, intrinsics, poses, caches, module, device, torch) -> None: diff --git a/experiments/evals/models/run_groups.py b/experiments/evals/models/run_groups.py index e0e46afcf..643daba3c 100644 --- a/experiments/evals/models/run_groups.py +++ b/experiments/evals/models/run_groups.py @@ -17,7 +17,6 @@ import argparse import hashlib import json -import shutil import sys import time from pathlib import Path @@ -35,17 +34,20 @@ write_npz, ) from models.map_anything import load, run -from models.run import pick_device, publish +from models.run import output_lock, pick_device, publish, staging_directory def write_group(out: Path, results, summary: dict) -> None: """Write one group's depth maps and run.json, all or nothing: they go to a staging folder that `publish` swaps in for `out` only once every file is written, so a failure partway leaves the previous outputs and run.json as they were, never a mix of old and new views.""" - stage = out.with_name(out.name + ".staging") - shutil.rmtree(stage, ignore_errors=True) - stage.mkdir(parents=True) - try: + with output_lock(out, wait=True): + write_locked_group(out, results, summary) + + +def write_locked_group(out: Path, results, summary: dict) -> None: + """Called with the group lock held through inference and publication.""" + with staging_directory(out) as stage: for res in results: write_npz( stage, @@ -56,11 +58,35 @@ def write_group(out: Path, results, summary: dict) -> None: res.cam_to_world, res.arrays, ) + summary = {**summary, "outputs": {p.name: file_hash(p) for p in stage.glob("*.npz")}} (stage / "run.json").write_text(json.dumps(summary, indent=1) + "\n") - except BaseException: - shutil.rmtree(stage, ignore_errors=True) - raise - publish(stage, out) + publish(stage, out) + + +def file_hash(path: Path) -> str: + h = hashlib.sha256() + with path.open("rb") as src: + while block := src.read(1 << 20): + h.update(block) + return h.hexdigest() + + +def cached_group(out: Path, key: str, members: list[str]) -> bool: + try: + record = json.loads((out / "run.json").read_text()) + if not isinstance(record, dict): + return False + expected = {f"{m}.npz" for m in members} + hashes = record.get("outputs", {}) + if not isinstance(hashes, dict) or any(not isinstance(sha, str) for sha in hashes.values()): + return False + return ( + record.get("fingerprint") == key + and set(hashes) == expected + and all(file_hash(out / name) == sha for name, sha in hashes.items()) + ) + except (OSError, ValueError): + return False def main() -> None: @@ -86,27 +112,22 @@ def main() -> None: "growing: the weights take 2.64 GB, and 2- and 4-view groups at 392 px fit under 3.8", ) args = ap.parse_args() - set_cache_dirs() - import torch + # One model stays loaded across all groups, so serialize the whole output-root run. + with output_lock(args.out, wait=True): + run_scene(args) + +def run_scene(args) -> None: + set_cache_dirs() groups = json.loads((args.scene_dir / "subsets.json").read_text()) if args.sizes: groups = {n: g for n, g in groups.items() if n in args.sizes} pose_sets = json.loads(args.poses_file.read_text()) if args.poses_file else None images_dir = args.scene_dir / "images_1024" intr_file = args.scene_dir / "model_inputs" / "intrinsics.json" - device = pick_device(args.device) - ckpt = (map_anything.REPO, map_anything.FILENAME, map_anything.REVISION) - require_free_space_for_download(*ckpt) - t0 = time.perf_counter() - model = load(device) - checkpoint = checkpoint_record(*ckpt, map_anything.SHA256) - if device == "mps": - torch.mps.empty_cache() # loading leaves about 0.6 GB of freed blocks cached - torch.mps.set_per_process_memory_fraction( - args.mps_cap_gb * 1e9 / torch.mps.recommended_max_memory() - ) - print(f"loaded in {time.perf_counter() - t0:.1f} s on {device}", file=sys.stderr) + model = None + device = None + checkpoint = None for n, members_list in groups.items(): for members in members_list: gid = f"n{n}-{members[0]}" @@ -117,42 +138,58 @@ def main() -> None: images = [images_dir / f"{m}.jpg" for m in members] intrinsics = read_intrinsics(intr_file, images) key = fingerprint( - members, images, intrinsics, poses, args.max_side, checkpoint["sha256"] - ) - done = out / "run.json" - # Reuse only an output made from these exact inputs and this checkpoint. - if done.exists() and json.loads(done.read_text()).get("fingerprint") == key: - continue - inputs = RunInputs( - images=images, - intrinsics=intrinsics, - intrinsics_mode="known", - poses=poses, - max_side=args.max_side, - device=device, - fp32=False, + members, images, intrinsics, poses, args.max_side, map_anything.SHA256 ) - results = run(model, inputs) - summary = { - "fingerprint": key, - "checkpoint": checkpoint, - "members": members, - "seconds_per_view": round(results[0].seconds, 3), - "network_wh": list(results[0].network_wh), - "weights": "transformer stacks bf16, rest fp32 (models/map_anything.py load)", - "metric_scaling_factor": [r.extra["metric_scaling_factor"] for r in results], - "poses_file": str(args.poses_file) if args.poses_file else None, - # What evals.pose_priors labels the row by: the exact pose file this run used. - "poses_sha256": ( - hashlib.sha256(args.poses_file.read_bytes()).hexdigest() - if args.poses_file - else None - ), - } - write_group(out, results, summary) - print(f"n={n} {members[0]}: {results[0].seconds:.2f} s/view", file=sys.stderr) - if device == "mps": - torch.mps.empty_cache() + with output_lock(out, wait=True): + if cached_group(out, key, members): + continue + if model is None: + import torch + + device = pick_device(args.device) + ckpt = (map_anything.REPO, map_anything.FILENAME, map_anything.REVISION) + require_free_space_for_download(*ckpt) + t0 = time.perf_counter() + model = load(device) + checkpoint = checkpoint_record(*ckpt, map_anything.SHA256) + if device == "mps": + torch.mps.empty_cache() + torch.mps.set_per_process_memory_fraction( + args.mps_cap_gb * 1e9 / torch.mps.recommended_max_memory() + ) + print( + f"loaded in {time.perf_counter() - t0:.1f} s on {device}", file=sys.stderr + ) + inputs = RunInputs( + images=images, + intrinsics=intrinsics, + intrinsics_mode="known", + poses=poses, + max_side=args.max_side, + device=device, + fp32=False, + ) + results = run(model, inputs) + summary = { + "fingerprint": key, + "checkpoint": checkpoint, + "members": members, + "seconds_per_view": round(results[0].seconds, 3), + "network_wh": list(results[0].network_wh), + "weights": "transformer stacks bf16, rest fp32 (models/map_anything.py load)", + "metric_scaling_factor": [r.extra["metric_scaling_factor"] for r in results], + "poses_file": str(args.poses_file) if args.poses_file else None, + # What evals.pose_priors labels the row by: the exact pose file this run used. + "poses_sha256": ( + hashlib.sha256(args.poses_file.read_bytes()).hexdigest() + if args.poses_file + else None + ), + } + write_locked_group(out, results, summary) + print(f"n={n} {members[0]}: {results[0].seconds:.2f} s/view", file=sys.stderr) + if device == "mps": + torch.mps.empty_cache() if __name__ == "__main__": diff --git a/experiments/evals/models/tests/test_common.py b/experiments/evals/models/tests/test_common.py index e6d6ef04c..0df9b5459 100644 --- a/experiments/evals/models/tests/test_common.py +++ b/experiments/evals/models/tests/test_common.py @@ -3,7 +3,10 @@ from __future__ import annotations import json +import select import shutil +import subprocess +import sys from pathlib import Path import numpy as np @@ -296,6 +299,212 @@ def _snapshot(folder): return {p.name: p.read_bytes() for p in sorted(folder.iterdir())} +@pytest.mark.parametrize("writer", ["single", "group"]) +def test_next_run_reclaims_staging_after_a_hard_exit(tmp_path, monkeypatch, writer): + from models import run, run_groups + + invoke, out = _fake_model_run(tmp_path, monkeypatch, fail_on=None) + _complete_run(out, "previous") + before = _snapshot(out) + script = """ +import os, sys +from pathlib import Path +import pytest +sys.path.insert(0, 'models/tests') +from test_common import _fake_model_run +from models import run, run_groups +root, writer = Path(sys.argv[1]), sys.argv[2] +def crash(stage): + (stage / 'partial.npz').write_bytes(b'partial prediction') + os._exit(137) +if writer == 'single': + invoke, out = _fake_model_run(root, pytest.MonkeyPatch(), fail_on=None) + run.run_into = lambda args, stage, *rest: crash(stage) + invoke() +else: + def results(): + stage = next(root.glob('out.staging-*')) + crash(stage) + yield + run_groups.write_group(root / 'out', results(), {}) +""" + result = subprocess.run( + [sys.executable, "-c", script, str(tmp_path), writer], + cwd=Path(run.__file__).resolve().parents[1], + capture_output=True, + text=True, + ) + assert result.returncode == 137, result.stderr + abandoned = list(tmp_path.glob("out.staging-*")) + assert len(abandoned) == 1 + assert (abandoned[0] / "partial.npz").read_bytes() == b"partial prediction" + assert _snapshot(out) == before + # A caller's similarly named folder must not be swept up with the recorded one. + unowned = tmp_path / "out.staging-caller" + _complete_run(unowned, "caller") + unowned_before = _snapshot(unowned) + if writer == "single": + invoke() + else: + run_groups.write_group(out, [], {"run": "retry"}) + assert not abandoned[0].exists() + assert _snapshot(unowned) == unowned_before + assert not (tmp_path / ".out.staging.json").exists() + assert (out / "run.json").is_file() + + +def test_staging_recovery_preserves_an_active_runner(tmp_path): + from models import run + + script = """ +import sys +from pathlib import Path +from models.run import output_lock, staging_directory +with output_lock(Path(sys.argv[1])), staging_directory(Path(sys.argv[1])) as stage: + (stage / 'partial.npz').write_bytes(b'active prediction') + print(stage, flush=True) + sys.stdin.readline() +""" + out = tmp_path / "out" + with subprocess.Popen( + [sys.executable, "-c", script, str(out)], + cwd=Path(run.__file__).resolve().parents[1], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) as process: + try: + stage = Path(process.stdout.readline().strip()) + assert (stage / "partial.npz").read_bytes() == b"active prediction" + with pytest.raises(RuntimeError, match="another model runner"), run.output_lock(out): + pytest.fail("acquired an active runner's lock") + assert (stage / "partial.npz").read_bytes() == b"active prediction" + finally: + process.communicate(input="\n", timeout=10) + assert process.returncode == 0 + assert not stage.exists() + + +def test_staging_recovery_preserves_a_replaced_directory(tmp_path): + from models import run + + out = tmp_path / "out" + stage = tmp_path / "out.staging-old" + _complete_run(stage, "old") + record = {"name": stage.name, "identity": run.directory_identity(stage)} + stage.rename(tmp_path / "saved-stage") + _complete_run(stage, "caller") + owner = tmp_path / ".out.staging.json" + owner.write_text(json.dumps(record)) + before = _snapshot(stage) + with pytest.raises(RuntimeError, match="staging directory differs"), run.output_lock(out): + pytest.fail("accepted a replaced staging directory") + assert _snapshot(stage) == before + assert json.loads(owner.read_text()) == record + + +def test_concurrent_group_runs_load_once_across_multiple_groups(tmp_path): + from models import run_groups + + (tmp_path / "images_1024").mkdir() + (tmp_path / "images_1024/a.jpg").write_bytes(b"image") + (tmp_path / "images_1024/b.jpg").write_bytes(b"image b") + (tmp_path / "model_inputs").mkdir() + (tmp_path / "model_inputs/intrinsics.json").write_text( + json.dumps({"a": [1, 1, 0, 0], "b": [1, 1, 0, 0]}) + ) + (tmp_path / "subsets.json").write_text(json.dumps({"1": [["a"], ["b"]]})) + script = """ +import fcntl, sys, types +from pathlib import Path +import numpy as np +from models import run_groups +root, role = Path(sys.argv[1]), sys.argv[2] +sys.modules['torch'] = types.SimpleNamespace() +run_groups.pick_device = lambda device: 'cpu' +run_groups.set_cache_dirs = lambda: None +run_groups.require_free_space_for_download = lambda *args: None +run_groups.checkpoint_record = lambda *args: {'verified': True} +def load(device): + with (root / 'loads.txt').open('a') as file: + file.write(role + '\\n') + print('loaded', flush=True) + sys.stdin.readline() + return object() +def infer(model, inputs): + with (root / 'inferences.txt').open('a') as file: + file.write(role + ':' + inputs.images[0].stem + '\\n') + return [types.SimpleNamespace( + path=inputs.images[0], depth=np.ones((2, 2), np.float32), + valid=np.ones((2, 2), bool), intrinsics=np.array([1., 1., 0., 0.]), + cam_to_world=None, arrays={}, seconds=0., network_wh=(2, 2), + extra={'metric_scaling_factor': 1.})] +run_groups.load = load +run_groups.run = infer +flock = fcntl.flock +def announce_wait(file, operation): + if role == 'second': + print('root' if Path(file.name).name == '.outputs.runner.lock' else 'group', flush=True) + elif Path(file.name).name == '.n1-b.runner.lock': + print('next-group', flush=True) + sys.stdin.readline() + return flock(file, operation) +fcntl.flock = announce_wait +sys.argv = ['run_groups', '--scene-dir', str(root), '--out', str(root / 'outputs')] +run_groups.main() +""" + processes = [] + + def launch(role): + process = subprocess.Popen( + [sys.executable, "-u", "-c", script, str(tmp_path), role], + cwd=Path(run_groups.__file__).resolve().parents[1], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + processes.append(process) + return process + + def await_line(process): + assert select.select([process.stdout], [], [], 10)[0], "runner did not reach the barrier" + return process.stdout.readline().strip() + + try: + first = launch("first") + assert await_line(first) == "loaded" + second = launch("second") + lock_scope = await_line(second) + assert lock_scope in ("root", "group") + first.stdin.write("\n") + first.stdin.flush() + assert await_line(first) == "next-group" + if lock_scope == "group": + assert await_line(second) == "group" + # Allow the old per-group design to overtake the first runner at group b. + assert await_line(second) == "loaded" + _, errors = second.communicate(input="\n", timeout=10) + assert second.returncode == 0, errors + _, errors = first.communicate(input="\n", timeout=10) + assert first.returncode == 0, errors + if lock_scope == "root": + _, errors = second.communicate(input="\n", timeout=10) + assert second.returncode == 0, errors + finally: + for process in processes: + if process.poll() is None: + process.kill() + process.communicate(timeout=10) + assert (tmp_path / "inferences.txt").read_text().splitlines() == ["first:a", "first:b"] + assert (tmp_path / "loads.txt").read_text().splitlines() == ["first"] + for member in ("a", "b"): + group = tmp_path / f"outputs/n1-{member}" + record = json.loads((group / "run.json").read_text()) + assert run_groups.cached_group(group, record["fingerprint"], [member]) + + def test_a_run_that_fails_midway_leaves_the_previous_run_intact(tmp_path, monkeypatch): main, out = _fake_model_run(tmp_path, monkeypatch, fail_on="b") out.mkdir() @@ -349,34 +558,184 @@ def failing_rename(self, target): shutil.rmtree(stage) -def test_a_run_left_aside_by_a_killed_publication_is_restored_not_deleted(tmp_path, monkeypatch): +@pytest.mark.parametrize("out_exists", [False, True]) +def test_publication_refuses_an_existing_backup_directory(tmp_path, out_exists): from models import run - # A kill between setting the old run aside and moving the new one in leaves only .previous. - _complete_run(tmp_path / "out.previous", "previous") + if out_exists: + _complete_run(tmp_path / "out", "current") + _complete_run(tmp_path / "out.previous", "caller-owned") before = _snapshot(tmp_path / "out.previous") - stage = tmp_path / "out.staging" + stage = tmp_path / "stage" _complete_run(stage, "new") - run.publish(stage, tmp_path / "out") - assert json.loads((tmp_path / "out" / "run.json").read_text()) == {"run": "new"} - assert not (tmp_path / "out.previous").exists() - # Restored first, then superseded by a complete publication: never deleted while alone. - _complete_run(tmp_path / "out.previous", "previous") - shutil.rmtree(tmp_path / "out") - stage = tmp_path / "out.staging" - _complete_run(stage, "new again") + with pytest.raises(RuntimeError, match="existing backup"): + run.publish(stage, tmp_path / "out") + assert _snapshot(tmp_path / "out.previous") == before + assert stage.exists() - real = Path.rename - def refuse_staging(self, target): - if self.name == "out.staging": - raise OSError("injected") - return real(self, target) +@pytest.mark.parametrize("record", ["{", "[]", '{"old": null, "new": null}']) +def test_publication_preserves_an_invalid_ownership_record(tmp_path, record): + from models import run - monkeypatch.setattr(Path, "rename", refuse_staging) - with pytest.raises(OSError, match="injected"): + owner = tmp_path / "out.publication.json" + owner.write_text(record) + backup, stage = tmp_path / "out.previous", tmp_path / "stage" + _complete_run(backup, "caller-owned") + _complete_run(stage, "new") + before = _snapshot(backup) + with pytest.raises(RuntimeError, match="invalid publication record"): run.publish(stage, tmp_path / "out") - assert _snapshot(tmp_path / "out") == before + assert _snapshot(backup) == before + assert owner.read_text() == record + assert stage.exists() + + +@pytest.mark.parametrize("after_rename", ["out", "stage", "cleanup"]) +def test_publication_recovers_after_a_process_is_killed(tmp_path, monkeypatch, after_rename): + from models import run + + out, stage = tmp_path / "out", tmp_path / "stage" + _complete_run(out, "previous") + _complete_run(stage, "interrupted") + previous_bytes, new_bytes = _snapshot(out), _snapshot(stage) + # Hard exit after the real rename bypasses publish's exception rollback. + script = """ +import os, shutil, sys +from pathlib import Path +from models.run import publish +out, stage, after_rename = Path(sys.argv[1]), Path(sys.argv[2]), sys.argv[3] +rename = Path.rename +def interrupted(self, target): + result = rename(self, target) + if self.name == after_rename: + os._exit(137) + return result +Path.rename = interrupted +rmtree = shutil.rmtree +def interrupted_cleanup(path, *args, **kwargs): + result = rmtree(path, *args, **kwargs) + if after_rename == "cleanup" and Path(path) == out.with_name(out.name + ".previous"): + os._exit(137) + return result +shutil.rmtree = interrupted_cleanup +publish(stage, out) +""" + result = subprocess.run( + [sys.executable, "-c", script, str(out), str(stage), after_rename], + cwd=Path(run.__file__).resolve().parents[1], + capture_output=True, + text=True, + ) + assert result.returncode == 137, result.stderr + backup = tmp_path / "out.previous" + if after_rename == "cleanup": + assert not backup.exists() + else: + assert _snapshot(backup) == previous_bytes + retry = tmp_path / "retry" + _complete_run(retry, "retry") + rename = Path.rename + + def fail_retry(self, target): + if self == retry: + raise OSError("injected replacement failure") + return rename(self, target) + + with monkeypatch.context() as patch: + patch.setattr(Path, "rename", fail_retry) + with pytest.raises(OSError, match="injected replacement failure"): + run.publish(retry, out) + assert _snapshot(out) == (previous_bytes if after_rename == "out" else new_bytes) + assert not backup.exists() + assert not (tmp_path / "out.publication.json").exists() + expected = _snapshot(retry) + run.publish(retry, out) + assert _snapshot(out) == expected + + +def test_publication_record_cannot_claim_a_replaced_backup(tmp_path): + from models import run + + out, stage = tmp_path / "out", tmp_path / "stage" + _complete_run(out, "previous") + _complete_run(stage, "new") + owner = tmp_path / "out.publication.json" + owner.write_text( + json.dumps({"old": run.directory_identity(out), "new": run.directory_identity(stage)}) + ) + out.rename(tmp_path / "saved-original") + backup = tmp_path / "out.previous" + _complete_run(backup, "caller-owned") + before = _snapshot(backup) + with pytest.raises(RuntimeError, match="directories differ"): + run.publish(stage, out) + assert _snapshot(backup) == before + assert stage.exists() + assert owner.exists() + + +@pytest.mark.parametrize("writer", ["single", "group"]) +def test_runners_recover_before_a_new_model_failure(tmp_path, monkeypatch, writer): + from models import run, run_groups + + main, out = _fake_model_run(tmp_path, monkeypatch, fail_on="a") + _complete_run(out, "previous") + before = _snapshot(out) + stage = tmp_path / "interrupted-stage" + _complete_run(stage, "interrupted") + owner = tmp_path / "out.publication.json" + owner.write_text( + json.dumps({"old": run.directory_identity(out), "new": run.directory_identity(stage)}) + ) + out.rename(tmp_path / "out.previous") + + def fail_results(): + raise MemoryError("injected") + yield # the failure happens when the group writer starts consuming results + + with pytest.raises(MemoryError): + if writer == "single": + main() + else: + run_groups.write_group(out, fail_results(), {}) + assert _snapshot(out) == before + assert not (tmp_path / "out.previous").exists() + assert not owner.exists() + + +def test_run_preserves_a_caller_owned_staging_sibling(tmp_path, monkeypatch): + main, _out = _fake_model_run(tmp_path, monkeypatch, fail_on=None) + sibling = tmp_path / "out.staging" + _complete_run(sibling, "caller-owned") + before = _snapshot(sibling) + main() + assert _snapshot(sibling) == before + + +def test_read_poses_refuses_a_reflection(tmp_path): + from models.common import read_poses + + pose = np.eye(4) + pose[0, 0] = -1 + path = tmp_path / "poses.json" + path.write_text(json.dumps({"a": pose.tolist()})) + with pytest.raises(ValueError, match="rigid transform"): + read_poses(path, [tmp_path / "a.jpg"]) + + +def test_group_cache_requires_unchanged_member_outputs(tmp_path): + from models.run_groups import cached_group, file_hash + + output = tmp_path / "a.npz" + output.write_bytes(b"prediction") + record = {"fingerprint": "k", "outputs": {"a.npz": file_hash(output)}} + (tmp_path / "run.json").write_text(json.dumps(record)) + assert cached_group(tmp_path, "k", ["a"]) + output.write_bytes(b"substitute") + assert not cached_group(tmp_path, "k", ["a"]) + output.unlink() + assert not cached_group(tmp_path, "k", ["a"]) def test_a_group_write_that_fails_midway_keeps_the_previous_outputs(tmp_path, monkeypatch): @@ -410,8 +769,98 @@ def fail_on_b(folder, stem, *args): with pytest.raises(OSError, match="injected"): run_groups.write_group(out, results, {"run": "new"}) # a is written, then b fails assert _snapshot(out) == before - assert sorted(p.name for p in tmp_path.iterdir()) == ["n2-a"] + assert sorted(p.name for p in tmp_path.iterdir()) == [".n2-a.runner.lock", "n2-a"] monkeypatch.setattr(run_groups, "write_npz", real_write) run_groups.write_group(out, results, {"run": "new"}) assert sorted(p.name for p in out.iterdir()) == ["a.npz", "b.npz", "run.json"] - assert json.loads((out / "run.json").read_text()) == {"run": "new"} + assert json.loads((out / "run.json").read_text())["run"] == "new" + + +def test_group_cache_hit_does_not_load_the_model(tmp_path, monkeypatch): + import sys + + from models import run_groups + from models.common import fingerprint + + (tmp_path / "images_1024").mkdir() + image = tmp_path / "images_1024" / "a.jpg" + image.write_bytes(b"image") + (tmp_path / "model_inputs").mkdir() + (tmp_path / "model_inputs" / "intrinsics.json").write_text(json.dumps({"a": [1, 1, 0, 0]})) + (tmp_path / "subsets.json").write_text(json.dumps({"1": [["a"]]})) + out = tmp_path / "outputs" / "n1-a" + out.mkdir(parents=True) + prediction = out / "a.npz" + prediction.write_bytes(b"prediction") + key = fingerprint(["a"], [image], [[1, 1, 0, 0]], None, None, run_groups.map_anything.SHA256) + (out / "run.json").write_text( + json.dumps({"fingerprint": key, "outputs": {"a.npz": run_groups.file_hash(prediction)}}) + ) + + def refuse(*args): + raise AssertionError("cached group tried to load a model") + + monkeypatch.setattr(run_groups, "load", refuse) + monkeypatch.setattr(run_groups, "require_free_space_for_download", refuse) + monkeypatch.setattr( + sys, + "argv", + ["run_groups", "--scene-dir", str(tmp_path), "--out", str(tmp_path / "outputs")], + ) + run_groups.main() + + +@pytest.mark.parametrize("writer", ["single", "group"]) +@pytest.mark.parametrize("failure", ["existing-backup", "rename"]) +def test_publication_failure_removes_only_owned_staging(tmp_path, monkeypatch, writer, failure): + from models import run_groups + + if writer == "single": + invoke, out = _fake_model_run(tmp_path, monkeypatch, fail_on=None) + else: + out = tmp_path / "out" + + def invoke(): + run_groups.write_group(out, [], {"run": "new"}) + + _complete_run(out, "previous") + before = _snapshot(out) + sibling = out.with_name(out.name + ".staging") + _complete_run(sibling, "caller-owned") + sibling_before = _snapshot(sibling) + backup = out.with_name(out.name + ".previous") + if failure == "existing-backup": + _complete_run(backup, "caller-owned-backup") + backup_before = _snapshot(backup) + error, message = RuntimeError, "existing backup" + else: + rename = Path.rename + + def refuse_staging(self, target): + if self.name.startswith(out.name + ".staging-"): + raise OSError("injected publication failure") + return rename(self, target) + + monkeypatch.setattr(Path, "rename", refuse_staging) + error, message = OSError, "injected publication failure" + with pytest.raises(error, match=message): + invoke() + assert not list(tmp_path.glob(out.name + ".staging-*")) + assert _snapshot(out) == before + assert _snapshot(sibling) == sibling_before + if failure == "existing-backup": + assert _snapshot(backup) == backup_before + else: + assert not backup.exists() + + +@pytest.mark.parametrize( + "record", [None, [], {"outputs": None}, {"outputs": ["a.npz"]}, {"outputs": {"a.npz": None}}] +) +def test_malformed_group_cache_is_a_miss(tmp_path, record): + from models.run_groups import cached_group + + if isinstance(record, dict): + record["fingerprint"] = "k" + (tmp_path / "run.json").write_text(json.dumps(record)) + assert not cached_group(tmp_path, "k", ["a"]) diff --git a/experiments/evals/results/map3d.json b/experiments/evals/results/map3d.json index 78626271e..24e0a2c8a 100644 --- a/experiments/evals/results/map3d.json +++ b/experiments/evals/results/map3d.json @@ -53,7 +53,7 @@ "facing_3cm_truth": { "claimed_ft": 0.5, "false_observed_ft": 0.5, - "passes": 1.0, + "passes": true, "spans_with_unseen_ft": [ [ 8.5, @@ -76,7 +76,7 @@ "facing": { "claimed_ft": 0.5, "false_observed_ft": 0.5, - "passes": 1.0, + "passes": true, "spans_with_unseen_ft": [ [ 8.5, diff --git a/experiments/evals/tests/test_field.py b/experiments/evals/tests/test_field.py index 85d9ae643..e29ba2f6c 100644 --- a/experiments/evals/tests/test_field.py +++ b/experiments/evals/tests/test_field.py @@ -651,3 +651,259 @@ def test_stamp_refuses_a_survey_already_bound_to_another_capture(case): survey["captures"] = [capture] # its own capture, stamped before: accepted again paths["truth"].write_text(json.dumps(survey)) field.stamp(archive, paths["truth"], paths["map"], out) + + +def test_cached_extraction_skips_zip_decompression(tmp_path, monkeypatch): + monkeypatch.setattr(field, "FIELD_DIR", tmp_path / "cache") + archive = tmp_path / "session.zip" + with zipfile.ZipFile(archive, "w", compression=zipfile.ZIP_DEFLATED) as z: + z.writestr("s/session.json", SESSION_JSON) + z.writestr("s/keyframe.jpg", b"photo") + folder, digest = field.unpack(archive) + + def no_decompression(*args, **kwargs): + pytest.fail("cached field stages must not open ZIP members") + + monkeypatch.setattr(zipfile.ZipFile, "open", no_decompression) + for _ in range(3): + assert field.unpack(archive) == (folder, digest) + + manifest = json.loads((field.FIELD_DIR / f"{digest[:16]}.members.json").read_text()) + assert manifest == { + "archive_sha256": digest, + "files": { + "s/session.json": hashlib.sha256(SESSION_JSON.encode()).hexdigest(), + "s/keyframe.jpg": hashlib.sha256(b"photo").hexdigest(), + }, + } + + +@pytest.mark.parametrize("invalid", [None, "{", "[]", "wrong-archive", "missing-member"]) +def test_cached_extraction_rebuilds_without_valid_manifest(tmp_path, monkeypatch, invalid): + monkeypatch.setattr(field, "FIELD_DIR", tmp_path / "cache") + archive = tmp_path / "session.zip" + _zip(archive, [("session.json", SESSION_JSON), ("keyframe.jpg", b"photo")]) + folder, digest = field.unpack(archive) + reference = field.FIELD_DIR / f"{digest[:16]}.members.json" + expected = json.loads(reference.read_text()) + if invalid is None: + reference.unlink() + elif invalid in ("{", "[]"): + reference.write_text(invalid) + else: + broken = json.loads(reference.read_text()) + if invalid == "wrong-archive": + broken["archive_sha256"] = "0" * 64 + else: + del broken["files"]["keyframe.jpg"] + reference.write_text(json.dumps(broken)) + opened = [] + original_open = zipfile.ZipFile.open + + def record_open(self, member, *args, **kwargs): + opened.append(member.filename) + return original_open(self, member, *args, **kwargs) + + monkeypatch.setattr(zipfile.ZipFile, "open", record_open) + assert field.unpack(archive) == (folder, digest) + assert opened == ["session.json", "keyframe.jpg"] + assert json.loads(reference.read_text()) == expected + + +def test_cached_extraction_recreates_modified_files(tmp_path, monkeypatch): + monkeypatch.setattr(field, "FIELD_DIR", tmp_path / "cache") + archive = tmp_path / "session.zip" + with zipfile.ZipFile(archive, "w") as z: + z.writestr("session.json", "original") + z.writestr("keyframe.jpg", b"photo") + folder, _ = field.unpack(archive) + (folder / "session.json").write_text("modified") + (folder / "keyframe.jpg").write_bytes(b"other") + again, _ = field.unpack(archive) + assert (again / "session.json").read_text() == "original" + assert (again / "keyframe.jpg").read_bytes() == b"photo" + + +def test_score_refuses_a_survey_with_another_capture(case): + archive, paths, _capture, out = case + truth = json.loads(paths["truth"].read_text()) + truth["captures"].append("another capture") + paths["truth"].write_text(json.dumps(truth)) + with pytest.raises(ValueError, match="survey captures must contain only"): + field.score(archive, paths["truth"], paths["map"], paths["rules"], out) + + +def test_image_decoder_keeps_sensor_orientation(tmp_path): + import struct + + path = tmp_path / "oriented.jpg" + assert cv2.imwrite(str(path), np.zeros((8, 12, 3), dtype=np.uint8)) + # A little-endian TIFF directory with one SHORT orientation tag, rotate 90 degrees. + exif = b"Exif\0\0" + struct.pack("<2sHIH", b"II", 42, 8, 1) + exif += struct.pack("H", len(exif) + 2) + exif + jpeg[2:]) + assert cv2.imread(str(path)).shape[:2] == (12, 8) + assert field.read_session_image(path).shape[:2] == (8, 12) + + +def test_jpeg_header_rejects_a_frame_beyond_the_scan_budget(tmp_path): + path = tmp_path / "invalid.jpg" + # Valid APP segments lead to a valid frame beyond the budget; an unbounded scan accepts it. + segment = b"\xff\xe0\xff\xff" + b"a" * 65533 + path.write_bytes(b"\xff\xd8" + segment * 17 + b"\xff\xc0\x00\x07\x08\x00\x08\x00\x0c") + with pytest.raises(ValueError, match="frame header"): + field.image_size(path) + + +def test_jpeg_frame_header_cannot_cross_scan_budget(tmp_path, monkeypatch): + monkeypatch.setattr(field, "MAX_JPEG_HEADER_BYTES", 32) + path = tmp_path / "crosses-budget.jpg" + path.write_bytes( + b"\xff\xd8\xff\xe0\x00\x1a" + b"a" * 24 + b"\xff\xc0\x00\x07\x08\x00\x08\x00\x0c" + ) + with pytest.raises(ValueError, match="frame header"): + field.image_size(path) + + +@pytest.mark.parametrize("input_name", ["session", "truth", "map", "rules"]) +@pytest.mark.parametrize("symlink", [False, True]) +def test_score_preserves_inputs_inside_cleanup_directory(case, input_name, symlink): + archive, paths, _capture, out = case + scored = out / "scored" + scored.mkdir(parents=True) + inputs = {"session": archive, **paths} + original = inputs[input_name].read_bytes() + inside = scored / inputs[input_name].name + inside.write_bytes(original) + if symlink: + alias = out.parent / f"{input_name}-alias" + alias.symlink_to(inside) + inputs[input_name] = alias + else: + inputs[input_name] = inside + report = out / "field_report.md" + report.write_text("previous report") + with pytest.raises(ValueError, match="overlaps scoring output cleanup"): + field.score(inputs["session"], inputs["truth"], inputs["map"], inputs["rules"], out) + assert inside.read_bytes() == original + assert report.read_text() == "previous report" + + +def test_score_clears_old_reports_before_a_failure(case): + archive, paths, _capture, out = case + out.mkdir() + (out / "field_report.md").write_text("stale report") + (out / "scored").mkdir() + (out / "scored" / "scores.json").write_text("stale scores") + truth = json.loads(paths["truth"].read_text()) + truth["captures"] = ["wrong capture"] + paths["truth"].write_text(json.dumps(truth)) + with pytest.raises(ValueError): + field.score(archive, paths["truth"], paths["map"], paths["rules"], out) + assert not (out / "field_report.md").exists() + assert (out / "scored").is_dir() + assert not list((out / "scored").iterdir()) + + +@pytest.mark.parametrize("layout", ["inside", "equal", "ancestor", "alias", "cleanup-alias"]) +def test_score_preserves_directory_sessions_overlapping_outputs(case, layout): + archive, paths, _capture, _out = case + folder, _ = field.unpack(archive) + out = folder / "results" + if layout == "equal": + out = folder + elif layout == "ancestor": + out = folder.parent + elif layout in ("alias", "cleanup-alias"): + out = folder.parent / "output-alias" + if layout == "alias": + (folder / "results").mkdir() + out.symlink_to(folder / "results", target_is_directory=True) + out.mkdir(exist_ok=True) + scored = out / "scored" + if layout == "cleanup-alias": + scored.symlink_to(folder, target_is_directory=True) + else: + scored.mkdir() + doc = json.loads((folder / "session.json").read_text()) + (scored / "session-input.jpg").write_bytes( + field.session_file(folder, doc["keyframes"][0]["img"]).read_bytes() + ) + if scored.resolve().is_relative_to(folder): + doc["keyframes"][0]["img"] = str( + (scored / "session-input.jpg").resolve().relative_to(folder) + ) + (folder / "session.json").write_text(json.dumps(doc)) + report = out / "field_report.md" + report.write_text("previous report") + before = {str(p.relative_to(folder)): p.read_bytes() for p in folder.rglob("*") if p.is_file()} + with pytest.raises(ValueError, match=r"directory input .* overlaps scoring output cleanup"): + field.score(folder, paths["truth"], paths["map"], paths["rules"], out) + assert { + str(p.relative_to(folder)): p.read_bytes() for p in folder.rglob("*") if p.is_file() + } == before + assert report.read_text() == "previous report" + + +def test_prepare_without_output_directory_skips_result_lookup(tmp_path, monkeypatch, capsys): + import sys + + archive = tmp_path / "session.zip" + folder, prepared = tmp_path / "unpacked", tmp_path / "prepared" + calls = [] + + def unpack(session): + assert session == archive + calls.append("unpack") + return folder, "capture-hash" + + def prepare(session_folder, digest): + assert (session_folder, digest) == (folder, "capture-hash") + calls.append("prepare") + return prepared + + def unused_lookup(*args): + pytest.fail("prepare must not look up a results directory") + + monkeypatch.setattr(sys, "argv", ["field", "prepare", str(archive)]) + monkeypatch.setattr(field, "unpack", unpack) + monkeypatch.setattr(field, "prepare", prepare) + monkeypatch.setattr(field, "result_dir", unused_lookup) + field.main() + assert calls == ["unpack", "prepare"] + assert capsys.readouterr().out == f"{prepared}\n" + + +def test_equal_archive_basenames_get_distinct_result_directories(tmp_path, monkeypatch): + monkeypatch.setattr(field, "FIELD_DIR", tmp_path / "cache") + archives = [] + for name in ("a", "b"): + folder = tmp_path / name + folder.mkdir() + archive = folder / "session.zip" + with zipfile.ZipFile(archive, "w") as z: + z.writestr("session.json", name) + archives.append(archive) + assert field.result_dir(archives[0]) != field.result_dir(archives[1]) + + +@pytest.mark.parametrize("dangling", [False, True]) +def test_score_replaces_scored_symlink_without_touching_its_target(case, dangling): + archive, paths, _capture, out = case + unrelated = out.parent / "unrelated-scores" + if not dangling: + unrelated.mkdir() + (unrelated / "keep.json").write_bytes(b"caller-owned scores") + out.mkdir() + scored = out / "scored" + scored.symlink_to(unrelated, target_is_directory=True) + field.score(archive, paths["truth"], paths["map"], paths["rules"], out) + assert scored.is_dir() + assert not scored.is_symlink() + (scored / "new.json").write_bytes(b"new scores") + if dangling: + assert not unrelated.exists() + else: + assert (unrelated / "keep.json").read_bytes() == b"caller-owned scores" + assert {path.name for path in unrelated.iterdir()} == {"keep.json"} diff --git a/experiments/evals/tests/test_makefile.py b/experiments/evals/tests/test_makefile.py new file mode 100644 index 000000000..bc1b7dfe3 --- /dev/null +++ b/experiments/evals/tests/test_makefile.py @@ -0,0 +1,117 @@ +"""The field recipe resolves capture identity once and preserves output overrides.""" + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +MAKEFILE = Path(__file__).resolve().parents[1] / "Makefile" + + +def run_make(tmp_path, target, override=None, lookup="success", check=True): + binary = tmp_path / "bin" + binary.mkdir() + log = tmp_path / "commands.jsonl" + uv = binary / "uv" + uv.write_text( + f"#!{sys.executable}\n" + "import json, os, sys\n" + "from pathlib import Path\n" + "args = sys.argv[1:]\n" + "log = Path(os.environ['COMMAND_LOG'])\n" + "with log.open('a') as handle:\n" + " handle.write(json.dumps(args) + '\\n')\n" + "if args[:5] == ['run', 'python', '-m', 'evals.field', 'result-dir']:\n" + " mode = os.environ['LOOKUP_MODE']\n" + " if mode in ['success', 'failed-with-path']:\n" + " print(os.environ['DEFAULT_OUT'])\n" + " elif mode == 'whitespace':\n" + " print(' ')\n" + " if mode in ['failed', 'failed-with-path']:\n" + " sys.exit(1)\n" + "elif args[:5] == ['run', 'python', '-m', 'evals.field', 'prepare']:\n" + " print(os.environ['WORK_DIR'])\n" + ) + uv.chmod(0o755) + scoring = tmp_path / "scoring" + scoring.mkdir() + args = [ + "make", + "-f", + str(MAKEFILE), + target, + "SESSION=session.zip", + "TRUTH=survey.json", + "MAP=map.json", + f"SCORING={scoring}", + ] + env = { + **os.environ, + "PATH": f"{binary}:{os.environ['PATH']}", + "COMMAND_LOG": str(log), + "DEFAULT_OUT": str(tmp_path / "default"), + "WORK_DIR": str(tmp_path / "work"), + "LOOKUP_MODE": lookup, + } + env.pop("FIELD_OUT", None) + if override == "command-line": + args.append(f"FIELD_OUT={tmp_path / 'override'}") + elif override == "environment": + env["FIELD_OUT"] = str(tmp_path / "override") + elif override == "empty": + args.append("FIELD_OUT=") + result = subprocess.run( + args, cwd=tmp_path, env=env, check=check, capture_output=True, text=True + ) + commands = [json.loads(line) for line in log.read_text().splitlines()] if log.exists() else [] + return result, commands + + +@pytest.mark.parametrize("override", [None, "command-line", "environment"]) +def test_field_output_lookup_runs_once_unless_overridden(tmp_path, override): + _result, commands = run_make(tmp_path, "field", override) + lookups = [ + args + for args in commands + if args[:5] == ["run", "python", "-m", "evals.field", "result-dir"] + ] + assert len(lookups) == (1 if override is None else 0) + output = str(tmp_path / ("default" if override is None else "override")) + for step in ("stamp", "score"): + [args] = [ + args for args in commands if args[:5] == ["run", "python", "-m", "evals.field", step] + ] + assert args[args.index("--out-dir") + 1] == output + [import_args] = [ + args for args in commands if args[:3] == ["run", "score", "import-measure-lab"] + ] + assert import_args[import_args.index("--map") + 1] == output + "/inputs/map.json" + assert import_args[import_args.index("--out") + 1] == output + "/measure-lab.json" + [score_args] = [args for args in commands if args[:3] == ["run", "score", "--rules"]] + assert score_args[score_args.index("--out") + 1] == output + "/scored" + + +def test_unrelated_target_does_not_resolve_a_capture(tmp_path): + _result, commands = run_make(tmp_path, "test") + assert not any(args[:4] == ["run", "python", "-m", "evals.field"] for args in commands) + + +@pytest.mark.parametrize("lookup", ["failed", "failed-with-path", "empty", "whitespace"]) +def test_failed_or_empty_lookup_stops_before_field_work(tmp_path, lookup): + report = tmp_path / "field_report.md" + report.write_bytes(b"previous report") + result, commands = run_make(tmp_path, "field", lookup=lookup, check=False) + assert result.returncode != 0 + assert "FIELD_OUT" in result.stderr + assert commands == [["run", "python", "-m", "evals.field", "result-dir", "session.zip"]] + assert report.read_bytes() == b"previous report" + + +def test_explicit_empty_output_stops_without_a_lookup(tmp_path): + result, commands = run_make(tmp_path, "field", override="empty", check=False) + assert result.returncode != 0 + assert "FIELD_OUT" in result.stderr + assert commands == [] diff --git a/experiments/evals/tests/test_pose_priors.py b/experiments/evals/tests/test_pose_priors.py index c220ae63d..7295f2b90 100644 --- a/experiments/evals/tests/test_pose_priors.py +++ b/experiments/evals/tests/test_pose_priors.py @@ -7,7 +7,7 @@ import numpy as np import pytest -from evals.ar_poses import ADVIO_ARKIT_SCALES, SETTINGS, _seed, degrade, group_poses, noise_draws +from evals.ar_poses import ADVIO_ARKIT_SCALES, SETTINGS, group_poses, noise_draws from evals.pairs import INCH from evals.pose_priors import ( SENTINEL, @@ -103,20 +103,14 @@ def __init__(self, x): self.cam_to_world[0, 3] = x -def test_draw_zero_keeps_the_original_generator_and_other_draws_differ(): +def test_pose_draws_are_repeatable_and_differ_between_draws(): views = {"a": _V(0.0), "b": _V(2.0)} groups = {"2": [["a", "b"]]} d0 = group_poses(views, groups, "modern_assumed", 0)["n2-a"] - ref = degrade( - [views["a"].cam_to_world, views["b"].cam_to_world], - 0.98, - SETTINGS["modern_assumed"], - np.random.default_rng(_seed("modern_assumed")), - ) - np.testing.assert_allclose(d0["poses"]["b"], ref[1]) + assert d0 == group_poses(views, groups, "modern_assumed", 0)["n2-a"] d1 = group_poses(views, groups, "modern_assumed", 1)["n2-a"] - assert d1["scale"] == d0["scale"] == 0.98 - assert not np.allclose(d1["poses"]["b"], d0["poses"]["b"]) + assert d0["poses"] != d1["poses"] + assert d0["scale"] == d1["scale"] def test_fit_cache_refits_only_when_inputs_change(tmp_path): diff --git a/experiments/evals/tests/test_review_fixes.py b/experiments/evals/tests/test_review_fixes.py index 8312f5962..b5049e229 100644 --- a/experiments/evals/tests/test_review_fixes.py +++ b/experiments/evals/tests/test_review_fixes.py @@ -232,6 +232,60 @@ def test_an_interrupted_unpack_is_not_complete(tmp_path: Path): assert not datasets.is_complete(archive) +@pytest.mark.parametrize( + "files", + [ + None, + [], + ["d/a.csv"], + {"d/a.csv": None}, + {"d/a.csv": {"bytes": 1}}, + {"d/a.csv": {"sha256": "f" * 64}}, + {"d/a.csv": {"bytes": "1", "sha256": "f" * 64}}, + ], +) +def test_malformed_dataset_file_records_are_cache_misses(tmp_path, files): + from evals import datasets + + archive = _archive(tmp_path) + (tmp_path / "d").mkdir() + (tmp_path / "d/a.csv").write_text("a") + path = datasets.record_path(archive) + path.parent.mkdir() + path.write_text(json.dumps({"sha256": archive.sha256, "files": files})) + assert not datasets.is_complete(archive) + + +@pytest.mark.parametrize("record", ["null", "[]", "{"]) +def test_malformed_dataset_record_is_a_cache_miss(tmp_path, record): + from evals import datasets + + archive = _archive(tmp_path) + path = datasets.record_path(archive) + path.parent.mkdir() + path.write_text(record) + assert not datasets.is_complete(archive) + + +def test_forced_dataset_fetch_never_checks_completion(tmp_path, monkeypatch): + import sys + + from evals import datasets + + archive = _archive(tmp_path) + monkeypatch.setattr(datasets, "ADVIO", [archive]) + monkeypatch.setattr(sys, "argv", ["datasets", "advio", "--force"]) + + def unexpected_check(_archive): + raise AssertionError("--force must skip cache validation") + + monkeypatch.setattr(datasets, "is_complete", unexpected_check) + fetched = [] + monkeypatch.setattr(datasets, "fetch", fetched.append) + datasets.main() + assert fetched == [archive] + + def test_a_recorded_unpack_is_complete_until_a_file_goes_missing_or_changes_size(tmp_path: Path): from evals import datasets @@ -337,3 +391,73 @@ def factory(gid, members): with pytest.raises(recon.PartialRun, match="n2-b"): recon.score_groups(scene, {"model": factory}, sizes=(1, 2)) + + +def test_dataset_cache_rejects_same_size_edits(tmp_path): + from evals import datasets + + archive = _archive(tmp_path, members=("d/a.csv",)) + (tmp_path / "d").mkdir() + path = tmp_path / "d/a.csv" + path.write_bytes(b"original") + datasets.write_record(archive, ["d/a.csv"]) + path.write_bytes(b"modified") + assert not datasets.is_complete(archive) + + +def test_within_bar_returns_a_python_boolean(): + assert type(within_bar(np.float64(0.5))) is bool + + +def test_nested_groups_share_camera_pose_errors(): + from types import SimpleNamespace + + from evals.ar_poses import group_poses + + views = {} + for i, name in enumerate("abcd"): + T = np.eye(4) + T[0, 3] = i + views[name] = SimpleNamespace(cam_to_world=T) + groups = {"2": [["a", "b"]], "4": [["a", "b", "c", "d"]]} + for setting in ("advio_2018", "modern_assumed"): + for draw in range(5): + poses = group_poses(views, groups, setting, draw) + assert poses["n2-a"]["scale"] == poses["n4-a"]["scale"] + for name in "ab": + np.testing.assert_array_equal( + poses["n2-a"]["poses"][name], poses["n4-a"]["poses"][name] + ) + + +def test_photo_geometry_excludes_edge_candidates(monkeypatch): + from types import SimpleNamespace + + from evals import frames + + scene = SimpleNamespace( + views={"v": SimpleNamespace(R_wc=np.eye(3), t_wc=np.zeros(3))}, + cands=np.array([[0, 0, 3.0], [0, 0, 100.0]]), + visible=lambda name: ( + np.array([0, 1]), + np.array([[2, 2], [2, 2]]), + np.array([False, True]), + ), + oracle_depth=lambda name: (np.full((5, 5), 3.0), np.eye(3)), + ) + monkeypatch.setattr(frames, "WIDTH", 5) + monkeypatch.setattr(frames, "_height", lambda view: 5) + distance, _angle = frames.view_geometry(scene, "v") + assert distance == 3.0 + + +def test_pose_conditioned_method_keeps_camera_centres(tmp_path, monkeypatch): + from evals import recon + + (tmp_path / "g").mkdir() + (tmp_path / "g" / "run.json").write_text("{}") + pose = np.eye(4) + pose[0, 3] = 5 + monkeypatch.setattr(recon, "load_prediction", lambda path: (np.ones((2, 2)), np.eye(3), pose)) + assert recon.model_frame_method(tmp_path, centred=True)("g", ["a"])("a")[3] is True + assert recon.model_frame_method(tmp_path)("g", ["a"])("a")[3] is False diff --git a/experiments/live-dots/.gitignore b/experiments/live-dots/.gitignore index 05dff4040..d8e79b81f 100644 --- a/experiments/live-dots/.gitignore +++ b/experiments/live-dots/.gitignore @@ -1,3 +1,4 @@ Fixtures/ +.fixtures-refresh.lock .build/ .swiftpm/ diff --git a/experiments/live-dots/fetch-fixtures.sh b/experiments/live-dots/fetch-fixtures.sh index 0370cda39..38ed99d99 100755 --- a/experiments/live-dots/fetch-fixtures.sh +++ b/experiments/live-dots/fetch-fixtures.sh @@ -3,9 +3,44 @@ # without adding ios/ to a sparse checkout. Fixtures/ is git-ignored. set -eu here="$(cd "$(dirname "$0")" && pwd)" +if [ "${1-}" != --lock-held ]; then + exec python3 "$here/rename-fixtures.py" --refresh "$here/fetch-fixtures.sh" +fi +# A hard kill skips the trap; recover our recorded backup before any git/network work. +python3 "$here/rename-fixtures.py" --recover "$here" root="$(git -C "$here" rev-parse --show-toplevel)" git -C "$root" fetch --quiet origin t3/ios-mvf -rm -rf "$here/Fixtures" -# ios/HouseScanUITests/Fixtures//... becomes Fixtures//... -git -C "$root" archive origin/t3/ios-mvf ios/HouseScanUITests/Fixtures | tar -x -C "$here" --strip-components=2 +stage="$(mktemp -d "$here/.fixtures.XXXXXX")" +published=0 +cleanup() { + status=$? + trap - EXIT HUP INT TERM + if [ "$published" -eq 0 ] && [ -d "$stage/previous" ]; then + if [ ! -e "$here/Fixtures" ] && [ ! -L "$here/Fixtures" ] && \ + python3 "$here/rename-fixtures.py" "$stage/previous" "$here/Fixtures"; then + : + else + echo "Could not restore fixtures; backup preserved at $stage/previous" >&2 + exit 1 + fi + fi + rm -rf "$stage" + exit "$status" +} +trap cleanup EXIT +trap 'exit 1' HUP INT TERM +python3 "$here/rename-fixtures.py" --mark-stage "$stage" +# Keep archive creation separate from extraction so either failure preserves the old fixtures. +git -C "$root" archive --output="$stage/fixtures.tar" origin/t3/ios-mvf ios/HouseScanUITests/Fixtures +tar -xf "$stage/fixtures.tar" -C "$stage" --strip-components=2 +if [ ! -f "$stage/Fixtures/synthetic-wall-lidar/session.json" ]; then + echo "Fetched fixtures lack synthetic-wall-lidar/session.json; keeping existing fixtures" >&2 + exit 1 +fi +if [ -e "$here/Fixtures" ]; then + python3 "$here/rename-fixtures.py" --mark-backup "$stage" "$here/Fixtures" + python3 "$here/rename-fixtures.py" "$here/Fixtures" "$stage/previous" +fi +python3 "$here/rename-fixtures.py" "$stage/Fixtures" "$here/Fixtures" +published=1 ls "$here/Fixtures" diff --git a/experiments/live-dots/rename-fixtures.py b/experiments/live-dots/rename-fixtures.py new file mode 100644 index 000000000..2eb1cf3e7 --- /dev/null +++ b/experiments/live-dots/rename-fixtures.py @@ -0,0 +1,133 @@ +"""Rename a fixture directory without nesting or replacing an existing destination.""" + +import ctypes +import fcntl +import json +import os +import shutil +import sys +from pathlib import Path + + +def refresh(script: Path) -> None: + # Exec keeps the lock in the shell itself, including its cleanup trap. A hard + # exit releases it without leaving a supervisor that can block recovery. + with (script.parent / ".fixtures-refresh.lock").open("a") as lock: + fcntl.flock(lock.fileno(), fcntl.LOCK_EX) + os.set_inheritable(lock.fileno(), True) + os.execv("/bin/sh", ["/bin/sh", str(script), "--lock-held"]) + + +def identity(path: Path) -> list[int]: + stat = path.lstat() + if path.is_symlink() or not path.is_dir(): + raise ValueError(f"{path}: expected a fixture directory") + return [stat.st_dev, stat.st_ino] + + +def mark_backup(stage: Path, fixtures: Path) -> None: + (stage / "ownership.json").write_text( + json.dumps({"stage": identity(stage), "previous": identity(fixtures)}) + ) + + +def mark_stage(stage: Path) -> None: + (stage / "ownership.json").write_text(json.dumps({"stage": identity(stage)})) + + +def recover_backup(here: Path) -> None: + fixtures = here / "Fixtures" + stages = [] + for stage in here.glob(".fixtures.*"): + try: + marker = stage / "ownership.json" + if marker.is_symlink(): + raise ValueError("linked ownership marker") + record = json.loads(marker.read_text()) + if not isinstance(record, dict) or not valid_identity(record.get("stage")): + raise ValueError("invalid stage identity") + if record["stage"] != identity(stage): + raise ValueError("stage identity changed") + previous = stage / "previous" + if "previous" in record and not valid_identity(record["previous"]): + raise ValueError("invalid backup identity") + if (previous.exists() or previous.is_symlink()) and record.get( + "previous" + ) != identity(previous): + raise ValueError("backup identity changed") + stages.append(stage) + except (OSError, ValueError): + print(f"Unverified fixture stage left untouched: {stage}", file=sys.stderr) + backups = [ + stage / "previous" + for stage in stages + if (stage / "previous").exists() or (stage / "previous").is_symlink() + ] + if backups and (fixtures.exists() or fixtures.is_symlink()): + paths = ", ".join(str(backup.parent) for backup in backups) + raise RuntimeError( + f"Fixtures is occupied; backup preserved in {paths}; inspect before refreshing" + ) + if not fixtures.exists() and not fixtures.is_symlink(): + if len(backups) > 1: + raise RuntimeError( + "multiple fixture backups; inspect them before refreshing" + ) + if backups: + identity(backups[0]) + rename_exclusive(str(backups[0]), str(fixtures)) + for stage in stages: + shutil.rmtree(stage) + + +def valid_identity(value) -> bool: + return ( + isinstance(value, list) + and len(value) == 2 + and all(type(n) is int for n in value) + ) + + +def rename_exclusive(source: str, destination: str) -> None: + libc = ctypes.CDLL(None, use_errno=True) + if sys.platform == "darwin": + rename = libc.renamex_np + rename.argtypes = [ctypes.c_char_p, ctypes.c_char_p, ctypes.c_uint] + args = (os.fsencode(source), os.fsencode(destination), 4) # RENAME_EXCL + elif sys.platform.startswith("linux"): + rename = libc.renameat2 + rename.argtypes = [ + ctypes.c_int, + ctypes.c_char_p, + ctypes.c_int, + ctypes.c_char_p, + ctypes.c_uint, + ] + args = ( + -100, + os.fsencode(source), + -100, + os.fsencode(destination), + 1, + ) # AT_FDCWD, NOREPLACE + else: + raise RuntimeError( + "fixture replacement requires macOS or Linux exclusive rename" + ) + rename.restype = ctypes.c_int + if rename(*args) != 0: + error = ctypes.get_errno() + raise OSError(error, os.strerror(error), destination) + + +if __name__ == "__main__": + if sys.argv[1] == "--refresh": + refresh(Path(sys.argv[2])) + elif sys.argv[1] == "--recover": + recover_backup(Path(sys.argv[2])) + elif sys.argv[1] == "--mark-backup": + mark_backup(Path(sys.argv[2]), Path(sys.argv[3])) + elif sys.argv[1] == "--mark-stage": + mark_stage(Path(sys.argv[2])) + else: + rename_exclusive(*sys.argv[1:]) diff --git a/experiments/live-dots/test_fetch_fixtures.py b/experiments/live-dots/test_fetch_fixtures.py new file mode 100644 index 000000000..7ca44aa76 --- /dev/null +++ b/experiments/live-dots/test_fetch_fixtures.py @@ -0,0 +1,384 @@ +import json +import os +import runpy +import subprocess +import sys +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).resolve().parent / "fetch-fixtures.sh" + + +@pytest.mark.parametrize( + "ownership", [None, "malformed", "wrong-stage", "wrong-backup"] +) +def test_recovery_preserves_unknown_fixture_stages(tmp_path, ownership, capsys): + import fcntl + + helper = runpy.run_path(str(SCRIPT.with_name("rename-fixtures.py"))) + stage = tmp_path / ".fixtures.foo" + previous = stage / "previous" + previous.mkdir(parents=True) + (previous / "session.json").write_text("caller data") + if ownership is not None: + record = { + "stage": helper["identity"](stage), + "previous": helper["identity"](previous), + } + if ownership == "wrong-stage": + record["stage"] = [-1, -1] + elif ownership == "wrong-backup": + record["previous"] = [-1, -1] + (stage / "ownership.json").write_text( + "{" if ownership == "malformed" else json.dumps(record) + ) + before = contents(stage) + with (tmp_path / ".fixtures-refresh.lock").open("a") as lock: + fcntl.flock(lock.fileno(), fcntl.LOCK_EX) + helper["recover_backup"](tmp_path) + assert contents(stage) == before + assert not (tmp_path / "Fixtures").exists() + assert ( + f"Unverified fixture stage left untouched: {stage}" in capsys.readouterr().err + ) + + +def contents(folder): + return { + str(path.relative_to(folder)): path.read_bytes() + for path in folder.rglob("*") + if path.is_file() + } + + +@pytest.mark.parametrize("published", [False, True]) +def test_recovery_preserves_a_stage_without_a_marker(tmp_path, published, capsys): + import fcntl + + helper = runpy.run_path(str(SCRIPT.with_name("rename-fixtures.py"))) + stage = tmp_path / ".fixtures.unmarked" + stage.mkdir() + (stage / "fixtures.tar").write_bytes(b"unfinished archive") + if published: + (tmp_path / "Fixtures").mkdir() + (tmp_path / "Fixtures/session.json").write_text("current") + with (tmp_path / ".fixtures-refresh.lock").open("a") as lock: + fcntl.flock(lock.fileno(), fcntl.LOCK_EX) + helper["recover_backup"](tmp_path) + assert (stage / "fixtures.tar").read_bytes() == b"unfinished archive" + assert str(stage) in capsys.readouterr().err + if published: + assert (tmp_path / "Fixtures/session.json").read_text() == "current" + + +@pytest.mark.parametrize( + "failure", + [ + "archive", + "extract", + "empty", + "missing-session", + "move", + "occupied", + "reappeared", + "reappeared-empty", + "restore", + "hard-kill", + "kill-extract", + "kill-published", + "success", + ], +) +def test_failed_refresh_preserves_existing_fixtures(tmp_path, failure): + script = tmp_path / "fetch-fixtures.sh" + script.write_text(SCRIPT.read_text()) + (tmp_path / "rename-fixtures.py").write_text( + SCRIPT.with_name("rename-fixtures.py").read_text() + ) + fixtures = tmp_path / "Fixtures" + fixtures.mkdir() + (fixtures / "old.json").write_text("working replay") + (fixtures / "synthetic-wall-lidar").mkdir() + (fixtures / "synthetic-wall-lidar/session.json").write_text('{"working": true}') + original = contents(fixtures) + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + git = bin_dir / "git" + git.write_text("""#!/bin/sh +case "$3" in +rev-parse) echo "$TEST_ROOT" ;; +fetch) exit 0 ;; +archive) + [ "$FAILURE" != archive ] || exit 1 + : > "${4#--output=}" + ;; +esac +""") + tar = bin_dir / "tar" + tar.write_text("""#!/bin/sh +[ "$FAILURE" != extract ] || exit 1 +mkdir -p "$4/Fixtures" +case "$FAILURE" in +missing-session) echo unrelated > "$4/Fixtures/new.json" ;; +success|move|occupied|restore|reappeared|reappeared-empty|hard-kill|kill-extract|kill-published) + mkdir -p "$4/Fixtures/synthetic-wall-lidar" + echo replacement > "$4/Fixtures/synthetic-wall-lidar/session.json" + ;; +esac +if [ "$FAILURE" = kill-extract ]; then + kill -KILL "$PPID" +fi +""") + mv = bin_dir / "mv" + mv.write_text("""#!/bin/sh +if [ "${1##*/}" = Fixtures ] && [ "$1" != "$TEST_ROOT/Fixtures" ]; then + case "$FAILURE" in + move|occupied|restore) + test -d "${1%/*}/previous" || exit 2 + echo failed > "$TEST_ROOT/final-move-failed" + if [ "$FAILURE" = occupied ]; then + mkdir "$TEST_ROOT/Fixtures" + echo concurrent > "$TEST_ROOT/Fixtures/other.json" + fi + exit 1 + ;; + esac +fi +if [ "$FAILURE" = restore ] && [ "${1##*/}" = previous ]; then + echo failed > "$TEST_ROOT/restore-failed" + exit 1 +fi +if [ "${1##*/}" = Fixtures ] && [ "$1" != "$TEST_ROOT/Fixtures" ]; then + case "$FAILURE" in + reappeared|reappeared-empty) + mkdir "$TEST_ROOT/Fixtures" + if [ "$FAILURE" = reappeared ]; then + echo concurrent > "$TEST_ROOT/Fixtures/other.json" + fi + ;; + esac +fi +exec /bin/mv "$@" +""") + python = bin_dir / "python3" + python.write_text( + '#!/bin/sh\nhelper="$1"\nshift\n' + + mv.read_text() + .removeprefix("#!/bin/sh\n") + .replace( + 'exec /bin/mv "$@"', + '''if [ "$FAILURE" = hard-kill ] && [ "$1" = "$TEST_ROOT/Fixtures" ]; then + "$TEST_PYTHON" "$helper" "$@" || exit 1 + kill -KILL "$PPID" + exit 0 +fi +if [ "$FAILURE" = kill-published ] && [ "${1##*/}" = Fixtures ] && [ "$1" != "$TEST_ROOT/Fixtures" ]; then + "$TEST_PYTHON" "$helper" "$@" || exit 1 + kill -KILL "$PPID" + exit 0 +fi +exec "$TEST_PYTHON" "$helper" "$@"''', + ) + ) + git.chmod(0o755) + tar.chmod(0o755) + mv.chmod(0o755) + python.chmod(0o755) + env = dict( + os.environ, + PATH=f"{bin_dir}:/usr/bin:/bin", + TEST_ROOT=str(tmp_path), + FAILURE=failure, + TEST_PYTHON=sys.executable, + ) + result = subprocess.run( + ["/bin/sh", str(script)], env=env, capture_output=True, text=True, check=False + ) + if failure in ("hard-kill", "kill-extract", "kill-published"): + assert result.returncode == -9, result.stderr + stages = list(tmp_path.glob(".fixtures.*")) + assert len(stages) == 1 + assert (stages[0] / "ownership.json").is_file() + if failure == "hard-kill": + assert not fixtures.exists() + if failure != "kill-extract": + assert contents(stages[0] / "previous") == original + else: + assert not (stages[0] / "previous").exists() + expected = contents(fixtures) if fixtures.exists() else original + # The first git invocation must already see the restored fixtures. Then fail + # archive creation, so neither fetching nor a successful swap can mask recovery. + git.write_text( + git.read_text().replace( + 'rev-parse) echo "$TEST_ROOT" ;;', + 'rev-parse) test -d "$TEST_ROOT/Fixtures" || exit 2; ' + 'for stage in "$TEST_ROOT"/.fixtures.*; do test ! -d "$stage" || exit 2; done; ' + 'echo "$TEST_ROOT" ;;', + ) + ) + again = subprocess.run( + ["/bin/sh", str(script)], + env={**env, "FAILURE": "archive"}, + capture_output=True, + text=True, + check=False, + ) + assert again.returncode == 1, again.stderr + assert contents(fixtures) == expected + if failure == "kill-published": + assert "Fixtures is occupied; backup preserved" in again.stderr + assert str(stages[0]) in again.stderr + assert contents(stages[0] / "previous") == original + elif failure == "success": + assert result.returncode == 0, result.stderr + assert ( + fixtures / "synthetic-wall-lidar/session.json" + ).read_text() == "replacement\n" + assert not (fixtures / "old.json").exists() + elif failure in ("occupied", "restore", "reappeared", "reappeared-empty"): + assert result.returncode != 0 + if failure in ("occupied", "restore"): + assert (tmp_path / "final-move-failed").read_text() == "failed\n" + stages = list(tmp_path.glob(".fixtures.*")) + assert len(stages) == 1 + backup = stages[0] / "previous" + assert contents(backup) == original + assert f"backup preserved at {backup}" in result.stderr + if failure in ("occupied", "reappeared"): + assert (fixtures / "other.json").read_text() == "concurrent\n" + assert list(fixtures.iterdir()) == [fixtures / "other.json"] + elif failure == "reappeared-empty": + assert list(fixtures.iterdir()) == [] + else: + assert (tmp_path / "restore-failed").read_text() == "failed\n" + assert not fixtures.exists() + if failure != "restore": + destination_before = contents(fixtures) + again = subprocess.run( + ["/bin/sh", str(script)], + env={**env, "FAILURE": "archive"}, + capture_output=True, + text=True, + check=False, + ) + assert again.returncode != 0 + assert "Fixtures is occupied; backup preserved" in again.stderr + assert str(stages[0]) in again.stderr + assert contents(backup) == original + assert contents(fixtures) == destination_before + else: + assert result.returncode != 0 + assert contents(fixtures) == original + if failure == "move": + assert (tmp_path / "final-move-failed").read_text() == "failed\n" + if failure == "missing-session": + assert "lack synthetic-wall-lidar/session.json" in result.stderr + if failure not in ( + "occupied", + "restore", + "reappeared", + "reappeared-empty", + "kill-published", + ): + assert not list(tmp_path.glob(".fixtures.*")) + + +def test_overlapping_refresh_waits_before_recovery(tmp_path): + import fcntl + import select + + script = tmp_path / SCRIPT.name + script.write_text(SCRIPT.read_text()) + helper = tmp_path / "rename-fixtures.py" + source = SCRIPT.with_name(helper.name).read_text() + # Pause the first process after backup creation, before publication. Announce + # the second lock attempt so the test does not depend on process startup speed. + source = source.replace( + "def rename_exclusive(source: str, destination: str) -> None:", + """def rename_exclusive(source: str, destination: str) -> None: + if os.environ.get("REFRESH") == "first" and Path(source).name == "Fixtures" and Path(destination).name == "Fixtures" and Path(source).parent != Path(destination).parent: + print("first paused", flush=True) + input() +""", + ).replace( + "fcntl.flock(lock.fileno(), fcntl.LOCK_EX)", + """if os.environ.get("REFRESH") == "second": + print("second waiting", flush=True) + fcntl.flock(lock.fileno(), fcntl.LOCK_EX) + if os.environ.get("REFRESH") == "second": + print("second locked", flush=True)""", + ) + helper.write_text(source) + fixtures = tmp_path / "Fixtures" + fixtures.mkdir() + (fixtures / "old.json").write_text("original") + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + for name, text in { + "git": """#!/bin/sh +case "$3" in +rev-parse) echo "$TEST_ROOT" ;; +archive) : > "${4#--output=}" ;; +esac +""", + "tar": """#!/bin/sh +mkdir -p "$4/Fixtures/synthetic-wall-lidar" +echo "$REFRESH" > "$4/Fixtures/synthetic-wall-lidar/session.json" +""", + "python3": f'#!/bin/sh\nexec "{sys.executable}" "$@"\n', + }.items(): + path = bin_dir / name + path.write_text(text) + path.chmod(0o755) + env = {**os.environ, "PATH": f"{bin_dir}:/usr/bin:/bin", "TEST_ROOT": str(tmp_path)} + processes = [] + + def start(label): + process = subprocess.Popen( + ["/bin/sh", str(script)], + env={**env, "REFRESH": label}, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + processes.append(process) + return process + + def line(process): + assert select.select([process.stdout], [], [], 5)[0], ( + "refresh did not reach barrier" + ) + return process.stdout.readline().strip() + + try: + first = start("first") + assert line(first) == "first paused" + (stage,) = tmp_path.glob(".fixtures.*") + assert not fixtures.exists() + before = contents(stage) + second = start("second") + assert line(second) == "second waiting" + with ( + (tmp_path / ".fixtures-refresh.lock").open("a") as lock, + pytest.raises(BlockingIOError), + ): + fcntl.flock(lock.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + assert contents(stage) == before + first.stdin.write("continue\n") + first.stdin.flush() + _, err = first.communicate(timeout=5) + assert first.returncode == 0, err + output, err = second.communicate(timeout=5) + assert second.returncode == 0, err + assert "second locked" in output + assert ( + fixtures / "synthetic-wall-lidar/session.json" + ).read_text() == "second\n" + assert not list(tmp_path.glob(".fixtures.*")) + finally: + for process in processes: + if process.poll() is None: + process.kill() + process.communicate(timeout=5) diff --git a/experiments/meter-closeup/FIELD_TEST.md b/experiments/meter-closeup/FIELD_TEST.md index 8aec0fce7..1087400e2 100644 --- a/experiments/meter-closeup/FIELD_TEST.md +++ b/experiments/meter-closeup/FIELD_TEST.md @@ -4,13 +4,13 @@ The thresholds in [PORTING.md](PORTING.md) come from processed Commons JPEGs, no 1. Take the photos below with the app's capture path, or the iPhone camera if the app is not ready. 2. Copy them into one folder. JPEG and HEIC both work. The command measures each photo's own pixels and never re-encodes them, because a JPEG round trip can lift sharpness across the threshold. -3. From this folder, run: +3. From `experiments/meter-closeup` in the repository, run with the absolute path to the photo folder: ```sh uv run python -m meter_eval.fieldtest PHOTO_DIR --number "" ``` -The command prints each photo's read result, the candidate's rank and the check values. It then counts retakes asked for photos that read, and photos accepted that did not. Like the app, its checks use the ranking's top candidate; `--number` only scores the outcome and is never written to disk. +The command prints each photo's read result, the candidate's rank and the check values. It then counts retakes asked for photos that read, and photos accepted that did not. Ranking and the top candidate used for retake checks depend only on blind heuristic features. The supplied number scores the read and its position in that order; it never changes the selected candidate. The number is never written to disk. | Threshold | Photos to take | The threshold holds if | |---|---|---| diff --git a/experiments/meter-closeup/README.md b/experiments/meter-closeup/README.md index 700882ab6..dc1e9f4e1 100644 --- a/experiments/meter-closeup/README.md +++ b/experiments/meter-closeup/README.md @@ -15,4 +15,11 @@ - Two phone-side retake checks work: whole-photo sharpness of 6.68 or less, and a top candidate 33.9 px tall or less. Glare, framing and hand shake have none. - The labels come from two AI readers, not people. +These figures use the earlier matcher that discarded arbitrary leading serial letters. The current +ranking and read scorer share a row matcher: labels and ratings are boundaries, adjacent +numeric groups cannot be omitted, and ranking uses only blind heuristic features. A read means +an admissible candidate matches the target; its rank is its position in that blind order. Rebuild the private number-core +labels and rerun the ranking evaluation before attributing those figures to the current code; +that dataset rerun has not been performed. + **What it changed.** The app offers three candidates to tap, barcode match first, and asks for a retake when the photo is blurry or the number too small ([PORTING.md](PORTING.md), [FIELD_TEST.md](FIELD_TEST.md)). Tables are in [results/](results/). diff --git a/experiments/meter-closeup/src/meter_eval/clean.py b/experiments/meter-closeup/src/meter_eval/clean.py index 677877538..c3befbbaa 100644 --- a/experiments/meter-closeup/src/meter_eval/clean.py +++ b/experiments/meter-closeup/src/meter_eval/clean.py @@ -41,7 +41,8 @@ def evaluate(row: dict, results: dict[str, dict]) -> dict: if config == PRIMARY and strict is not None: out["number_box"] = json.dumps([round(v, 5) for v in union_box(strict)]) # Is the tallest line with 4+ digits, the phone's guess at the label, the number? - out["tallest_is_number"] = int(tallest_digit_line(lines)["box"] in strict) + tallest = tallest_digit_line(lines) + out["tallest_is_number"] = int(tallest is not None and tallest["box"] in strict) if row["class_label"]: out[f"class_{config}"] = int(class_read(lines, row["class_label"])) return out diff --git a/experiments/meter-closeup/src/meter_eval/fieldtest.py b/experiments/meter-closeup/src/meter_eval/fieldtest.py index 955a7d19b..8c8909e87 100644 --- a/experiments/meter-closeup/src/meter_eval/fieldtest.py +++ b/experiments/meter-closeup/src/meter_eval/fieldtest.py @@ -3,11 +3,11 @@ uv run python -m meter_eval.fieldtest PHOTO_DIR --number "12 345 678" For each photo: whether Vision read the number, where the number-finding ranking put it, the -checks the app would run, and whether they would have asked for a retake. As in the app, the -checks use the ranking's top candidate, not the true number; --number only scores the -outcome. The summary counts the two costly outcomes: a retake asked for a photo that read (a -wasted retake) and a photo accepted that did not read (a re-request later). The meter number -is taken from the command line and is never written to disk. +retake check values and whether they would ask for a retake. Candidates and the retake box +come from the blind heuristic ranking; --number only scores the read and the number's rank. +The summary counts a retake asked for a photo that read (a wasted retake) and a photo +accepted that did not read (a re-request later). The meter number is taken from the command +line and is never written to disk. """ import argparse @@ -19,13 +19,18 @@ from meter_eval import retake from meter_eval.locate import candidates, ranked, top_candidate -from meter_eval.match import core, digest, normalize, number_boxes +from meter_eval.match import match_serial, number_core, serial_groups from meter_eval.ocr import Reader from meter_eval.quality import gray SUFFIXES = {".jpg", ".jpeg", ".png", ".heic"} +def reads_number(text: str | list[dict], target: str) -> bool: + lines = [{"text": text, "box": [0, 0, 1, 1]}] if isinstance(text, str) else text + return any(candidate["matches_target"] for candidate in match_serial(lines, target)) + + def upright_pixels(photo: Path, scratch: Path) -> Image.Image: """The photo's own decoded pixels, turned upright by its orientation tag. @@ -54,8 +59,15 @@ def main() -> None: parser.add_argument("--number", required=True, help="the meter number as printed") args = parser.parse_args() - target, length = digest(normalize(args.number)), len(normalize(args.number)) - target_core = digest(core(args.number)) + target_core = number_core(args.number) + if target_core is None: + parser.error("--number contains an unsupported character in a serial group") + if not target_core: + parser.error("--number must contain letters or digits") + if any(kind == "rating" for _, _, _, kind in serial_groups(args.number, ocr_ratings=True)): + parser.error( + "--number contains a rating-shaped part; such a serial is not supported by the OCR contract" + ) photos = sorted(p for p in args.folder.iterdir() if p.suffix.lower() in SUFFIXES) if not photos: raise SystemExit(f"no .jpg, .png or .heic photos in {args.folder}") @@ -77,9 +89,9 @@ def main() -> None: image.save(upright) g = gray(image) result = reader.read(upright, barcodes=True) - read = number_boxes(result["lines"], target, length, lenient=False) is not None + read = reads_number(result["lines"], args.number) order = ranked(candidates(result)) - rank = next((i + 1 for i, c in enumerate(order) if digest(c) == target_core), None) + rank = next((i + 1 for i, c in enumerate(order) if c == target_core), None) guess = top_candidate(result) box = guess and guess["box"] why = retake.reasons(g, box) diff --git a/experiments/meter-closeup/src/meter_eval/labels.py b/experiments/meter-closeup/src/meter_eval/labels.py index d148175ac..aae71f7b3 100644 --- a/experiments/meter-closeup/src/meter_eval/labels.py +++ b/experiments/meter-closeup/src/meter_eval/labels.py @@ -27,7 +27,7 @@ import re import secrets -from meter_eval.match import core, digest, lenient_digest, normalize, normalize_class +from meter_eval.match import core, digest, lenient_digest, normalize, normalize_class, number_core from meter_eval.paths import DATA_DIR, EXPERIMENT_DIR, KEY_PATH, MANIFEST READER1 = DATA_DIR / "labels_reader1.csv" @@ -152,12 +152,13 @@ def build() -> list[dict]: and r2["number_sure"] == "sure" ) kept = verdict == "keep" + serial = number_core(number) row |= { "number_hmac": digest(normalize(number)), "number_hmac_lenient": lenient_digest(number), "number_len": len(normalize(number)), - "number_core_hmac": digest(core(number)), - "number_core_len": len(core(number)), + "number_core_hmac": digest(serial) if serial else "", + "number_core_len": len(serial) if serial else 0, "number_agreed": "yes" if (agreed and verdict != "fix") or kept else "no", "number_agreed_strict": "yes" if (strict and verdict != "fix") or kept else "no", "human_check": verdict, diff --git a/experiments/meter-closeup/src/meter_eval/locate.py b/experiments/meter-closeup/src/meter_eval/locate.py index 3524ffeb6..93182bbaa 100644 --- a/experiments/meter-closeup/src/meter_eval/locate.py +++ b/experiments/meter-closeup/src/meter_eval/locate.py @@ -1,9 +1,9 @@ """Q4 and Q5: find the meter number among everything Vision reads on a close-up. Candidates are the digit-bearing tokens of each recognized line plus decoded barcode -payloads. A candidate is right when its core (see `match.core`) equals the labelled number's -core, so "NO. 12345678" and "ABC 123456" count, while a longer string that merely contains -the number, such as a barcode line "*XYZ1234567890…*", does not. +payloads. A candidate is right when its label-stripped alphanumeric text equals the labelled +number. Recognised printed labels such as "NO." are optional; serial letters such as ABC +remain part of the number. A longer string that merely contains the number does not match. The rules and the ranking were written from the odd-numbered photos only (`--split dev`) and committed before the even-numbered photos were scored. The held-out set is split by @@ -18,15 +18,13 @@ import csv import json import re +from collections import Counter -from meter_eval.match import core, digest, number_read +from meter_eval.match import KEYWORD, digest, match_serial, number_core from meter_eval.ocr import Reader from meter_eval.paths import DATA_DIR, MANIFEST, RESULTS_DIR from meter_eval.stats import share -# A label word in front of the number: No., Nr., №, #, S/N, Serial. -# "#:" is how Vision renders the Taiwanese label 電號: in front of the number. -KEYWORD = re.compile(r"^\s*(?:N[O0R]\.?|N°|№|#\s*:|S/?N|SERIAL\s*#?)\s*[.:#]?\s*", re.IGNORECASE) # Lines that describe the meter rather than identify it: ratings, voltages, constants, forms. SPEC = re.compile( r"\d\s*\(\s*\d+\s*\)|\d\s*V\b|HZ|\bKH\b|KH\s*\d|\bKT\b|\bTA\s*=?\s*\d|\bCL\s*\d|\d\s*CL\b|" @@ -66,60 +64,49 @@ def tokens(text: str) -> list[str]: return [t for t in unique if digits(t) >= 4] +def payload_fields(barcodes: list[dict]) -> list[str]: + """Validate each delimited barcode field independently, skipping unsupported fields.""" + return [ + token + for barcode in barcodes + for field in re.split(r"[;*,{}]+", barcode.get("payload") or "") + if (token := number_core(field)) + ] + + def payload_tokens(barcodes: list[dict]) -> set[str]: - """Cores of the digit-bearing parts of every decoded payload.""" - parts = set() - for barcode in barcodes: - for part in re.split(r"[\s;*,{}]+", barcode.get("payload") or ""): - if digits(part) >= 4: - parts.add(core(part)) - return parts - - -def candidates(result: dict) -> list[dict]: - """Every candidate with the features the rules use. - - A text token that equals a barcode payload part is confirmed. One that sits inside a - longer payload part (at least 6 characters) is a partial read of it, as when Vision drops - a digit or a prefix printed apart; the candidate then takes the payload's full string. If - two payloads of the same length both contain it, the read is ambiguous and not promoted. - """ + """Cores of the digit-bearing fields of every decoded payload.""" + return {token for token in payload_fields(barcodes) if digits(token) >= 4} + + +def candidates( + result: dict, target: str | None = None, *, target_digest: str | None = None +) -> list[dict]: + """Admissible OCR candidates under the shared serial contract, with ranking features.""" payloads = payload_tokens(result.get("barcodes") or []) - found = [] - for line in result["lines"]: - text = line["text"] - _, _, w, h = line["box"] - for token in tokens(text): - token_core = core(token) - confirmed = token_core in payloads - if not confirmed and len(token_core) >= 6: - longer = [p for p in payloads if token_core in p] - shortest = min((len(p) for p in longer), default=0) - closest = [p for p in longer if len(p) == shortest] - # Promote only when one payload is the closest match; a tie is ambiguous. - if len(closest) == 1: - token_core, confirmed = closest[0], True - found.append( - { - "core": token_core, - "box": line["box"], - "barcode_confirmed": confirmed, - "keyword": bool(KEYWORD.match(text)) - and not re.match(r"\s*CAT", text, re.IGNORECASE), - "alone": core(KEYWORD.sub("", text)) == token_core, - "spec": bool(SPEC.search(text)), - "vertical": h > w, - "zeros": set(token_core) <= {"0"}, - "length_ok": 6 <= len(token_core) <= 14 and digits(token_core) >= 5, - "height": h, - } - ) + found = match_serial(result["lines"], target, target_digest=target_digest) + # Ranking features describe the source row; retake checks use the candidate's own box. + row_counts = Counter(candidate["row_id"] for candidate in found) + for candidate in found: + token = candidate["core"] + _, _, width, height = candidate["row_box"] + candidate.update( + { + "barcode_confirmed": token in payloads, + "alone": row_counts[candidate["row_id"]] == 1, + "spec": bool(SPEC.search(candidate["text"])), + "vertical": height > width, + "zeros": set(token) <= {"0"}, + "length_ok": 6 <= len(token) <= 14 and digits(token) >= 5, + "height": height, + } + ) return found def near_barcode(candidate: dict, barcodes: list[dict]) -> bool: """The candidate's line sits within 1.5 line heights above or below a barcode it overlaps.""" - x, y, w, h = candidate["box"] + x, y, w, h = candidate.get("row_box", candidate["box"]) for b in barcodes: bx, by, bw, bh = b["box"] overlaps = min(x + w, bx + bw) - max(x, bx) > 0.3 * min(w, bw) @@ -147,9 +134,9 @@ def score(c: dict) -> float: ) -def top_candidate(result: dict) -> dict | None: - """The phone's best guess at the number's line: the highest-scoring candidate.""" - return best(candidates(result)) +def top_candidate(result: dict, target: str | None = None) -> dict | None: + """Best admissible candidate under the blind heuristic score.""" + return best(candidates(result, target)) def ranked(found: list[dict]) -> list[str]: @@ -188,6 +175,7 @@ def rules(found: list[dict], barcodes: list[dict]) -> dict[str, dict | None]: def scan(rows: list[dict]) -> dict[str, dict]: raw_path = DATA_DIR / "ocr" / "scan.jsonl" + raw_path.parent.mkdir(parents=True, exist_ok=True) results = {} with Reader() as reader, raw_path.open("w") as raw: for row in rows: @@ -201,12 +189,7 @@ def barcode_stats(row: dict, result: dict) -> dict: barcodes = result.get("barcodes") or [] payloads = [b["payload"] for b in barcodes if b.get("payload")] target = row["number_core_hmac"] - length = int(row["number_core_len"] or 0) - has_number = any( - digest(core(p)[i : i + length]) == target - for p in payloads - for i in range(len(core(p)) - length + 1) - ) + has_number = any(digest(token) == target for token in payload_fields(barcodes)) return { "barcode_found": int(bool(barcodes)), "barcode_decoded": int(bool(payloads)), @@ -244,14 +227,13 @@ def evaluate(row: dict, result: dict, split: str) -> dict: out |= barcode_stats(row, result) if row["number_agreed"] != "yes": return out - out["read"] = int( - number_read(result["lines"], row["number_hmac"], int(row["number_len"]), lenient=False) - ) - found = candidates(result) target = row["number_core_hmac"] + # Keep ground truth out of candidate generation, ranking and rule selection. + found = candidates(result) order = ranked(found) hits = [i for i, c in enumerate(order) if digest(c) == target] - out["rank"] = hits[0] + 1 if hits else "" + out["rank"] = hits[0] + 1 if hits else None + out["read"] = int(out["rank"] is not None) out["candidates"] = len(order) for name, pick in rules(found, result.get("barcodes") or []).items(): out[name] = "" if pick is None else int(digest(pick["core"]) == target) @@ -311,19 +293,19 @@ def rule_table(rows: list[dict], names: list[str]) -> str: f"{share(right, len(subset))} |" ) for k in (1, 3): - hit = sum(1 for r in read if r["rank"] != "" and r["rank"] <= k) + hit = sum(1 for r in read if r["rank"] is not None and r["rank"] <= k) out.append( f"| ranking, top {k}, over read photos only | {len(read)} | – | " f"{share(hit, len(read))} |" ) # End to end: a photo Vision did not read cannot offer the number, so it is a miss. for k in (1, 3): - hit = sum(1 for r in subset if r["rank"] != "" and r["rank"] <= k) + hit = sum(1 for r in subset if r["rank"] is not None and r["rank"] <= k) out.append( f"| ranking, top {k}, over every agreed photo (unread count as misses) | " f"{len(subset)} | – | {share(hit, len(subset))} |" ) - present = sum(1 for r in read if r["rank"] != "") + present = sum(1 for r in read if r["rank"] is not None) out.append( f"| number is any candidate, over read photos (ceiling) | {len(read)} | – | " f"{share(present, len(read))} |\n" diff --git a/experiments/meter-closeup/src/meter_eval/match.py b/experiments/meter-closeup/src/meter_eval/match.py index 13aaf85f2..a81a3ea24 100644 --- a/experiments/meter-closeup/src/meter_eval/match.py +++ b/experiments/meter-closeup/src/meter_eval/match.py @@ -15,11 +15,240 @@ from collections.abc import Sequence from meter_eval.paths import KEY_PATH +from meter_eval.quality import union_box # Lenient mode maps letters Vision commonly returns for digits. Applied to both the label and # the output, so it can only add matches. DIGIT_FIXUPS = str.maketrans({"O": "0", "I": "1", "L": "1"}) +# Printed labels shared by reading and candidate ranking. Compound labels precede +# shorter forms. Recognised prefixes split off even when attached to the serial. +NUMBER_LABELS = ( + "Serial No", + "Meter No", + "Meter #", + "S/N", + "Serial", + "Meter", + "SN", + "No", + "N0", + "Nr", + "N°", + "№", + "#", +) +# Label wrappers are explicit so format characters cannot disappear around a label. +_LABEL_SEPARATOR = r"[\s().:/#-]" +_LABEL_PATTERN = "|".join( + (_LABEL_SEPARATOR + "+").join(r"(?ai:" + re.escape(word) + ")" for word in label.split()) + for label in NUMBER_LABELS +) +_GROUP_LABEL = re.compile(rf"{_LABEL_SEPARATOR}*(?:" + _LABEL_PATTERN + rf"){_LABEL_SEPARATOR}*") +KEYWORD = re.compile("^" + _GROUP_LABEL.pattern) + + +_SERIAL_ALPHABET = frozenset("ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789") +# Only these punctuation marks and Unicode whitespace split a serial group. +_SERIAL_SEPARATORS = "-/.:" +_SERIAL_GROUP = re.compile(r"[^\s" + re.escape(_SERIAL_SEPARATORS) + r"]+") + + +def serial_text(text: str) -> str: + """Map only ASCII and fullwidth ASCII letters/digits, preserving invalid groups.""" + mapped = [] + for character in text: + code = ord(character) + if 0xFF10 <= code <= 0xFF19 or 0xFF21 <= code <= 0xFF3A or 0xFF41 <= code <= 0xFF5A: + character = chr(code - 0xFEE0) + if "a" <= character <= "z": + character = chr(ord(character) - 32) + mapped.append(character) + return "".join(mapped) + + +def read_target(text: str) -> str | None: + """Label-stripped serial text, or None for any unsupported group character.""" + groups = serial_groups(text) + if any(kind == "invalid" for _start, _end, _word, kind in groups): + return None + return "".join(word for _start, _end, word, kind in groups if kind != "boundary").lower() + + +def number_core(text: str) -> str | None: + target = read_target(text) + return target.upper() if target is not None else None + + +# Ratings are OCR boundaries: plate units and class/form prefixes. +SERIAL_RATINGS = { + "units": ( + "V", + "VAC", + "A", + "AMP", + "AMPS", + "HZ", + "W", + "KW", + "KWH", + "VA", + "KH", + "TA", + "PH", + "WIRE", + ), + "classes": (r"CL\s*[0-9]{1,3}", r"FM\s*[0-9]{1,3}[A-Z]?", "2S", "3W"), +} + + +# Ratings are short because voltage/current/frequency/Kh/class values have at most +# four digits; longer digit-bearing strings remain possible serials. +_SMALL_RATING_NUMBER = ( + r"(?:[0-9]{1,3}\.[0-9]|[0-9]{1,2}\.[0-9]{2}|[0-9]\.[0-9]{3}|\.[0-9]{1,4}|[0-9]{1,4})" +) +_RATING = re.compile( + r"(? list[tuple[int, int, str, str]]: + """Shared label stripping and validation, preserving supplied serial identities. + + OCR rating boundaries are opt-in so target and barcode suffixes are never removed. + """ + text = serial_text(text) + masked = list(text) + boundary_spans = [] + label_end = 0 + for token in _SERIAL_GROUP.finditer(text): + if token.start() < label_end: + continue + cursor = token.start() + while label := _GROUP_LABEL.match(text, cursor): + label_end = label.end() + boundary_spans.append((label.start(), label.end(), "boundary")) + masked[label.start() : label.end()] = [" "] * (label.end() - label.start()) + cursor = label.end() + labelled_text = "".join(masked) + for rating in _RATING.finditer(labelled_text) if ocr_ratings else (): + # Never extract a rating from inside a group containing an unsupported character. + neighbours = labelled_text[rating.start() - 1 : rating.start()] if rating.start() else "" + neighbours += labelled_text[rating.end() : rating.end() + 1] + if any( + not character.isspace() and character not in _SERIAL_SEPARATORS + for character in neighbours + ): + continue + boundary_spans.append((rating.start(), rating.end(), "rating")) + masked[rating.start() : rating.end()] = [" "] * (rating.end() - rating.start()) + spans = [(start, end, "", kind) for start, end, kind in boundary_spans] + spans.extend( + (token.start(), token.end(), token.group(), "") + for token in _SERIAL_GROUP.finditer("".join(masked)) + ) + groups = [] + for start, end, word, kind in sorted(spans): + if not kind and any(character not in _SERIAL_ALPHABET for character in word): + kind = "invalid" + elif not kind and word.isdigit(): + kind = "numeric" + elif not kind: + kind = "text" + groups.append((start, end, word, kind)) + return groups + + +def match_serial( + lines: Sequence[dict], target: str | None = None, *, target_digest: str | None = None +) -> list[dict]: + """Admissible row candidates in source order, with source and union boxes. + + Labels and ratings split runs. Numeric neighbours disallow a partial number; + alphabetic neighbours permit a serial inside surrounding printed words. + A separately printed label word always splits a run, so AB SN 12345 cannot + represent ABSN12345; the same serial printed as one group is supported. + """ + rows = [] + for line in sorted(lines, key=lambda item: item["box"][1]): + _, y, _, h = line["box"] + for row in rows: + _, ry, _, rh = row[0]["box"] + if min(y + h, ry + rh) - max(y, ry) >= 0.5 * min(h, rh): + row.append(line) + break + else: + rows.append([line]) + wanted = number_core(target) if target is not None else None + if target is not None and wanted is None: + return [] + found = [] + for row_id, row in enumerate(rows): + row_box = union_box([piece["box"] for piece in row]) + # No Unicode normalization/case folding or O/0, I/1, Cyrillic/Latin mapping. + ordered = sorted(row, key=lambda piece: piece["box"][0]) + normalized = [] + owners = [] + for piece in ordered: + text = serial_text(piece["text"]) + if normalized: + owners.append(None) + normalized.append(text) + owners.extend([piece["box"]] * len(text)) + row_text = " ".join(normalized) + groups = [] + kinds = [] + for start, end, word, kind in serial_groups(row_text, ocr_ratings=True): + boxes = [] + for box in owners[start:end]: + if box is not None and box not in boxes: + boxes.append(box) + groups.append((word, boxes)) + kinds.append("boundary" if kind == "rating" else kind) + words = [word for word, _ in groups] + for start in range(len(words)): + if kinds[start] in ("boundary", "invalid") or ( + start and kinds[start - 1] in ("numeric", "invalid") + ): + continue + joined = "" + boxes = [] + for end in range(start, len(words)): + if kinds[end] in ("boundary", "invalid"): + break + joined += words[end] + for box in groups[end][1]: + if box not in boxes: + boxes.append(box) + if end + 1 < len(words) and kinds[end + 1] in ("numeric", "invalid"): + continue + # Alphabetic fragments can join a serial; a maker word alone is not one. + if not any(character.isdigit() for character in joined): + continue + found.append( + { + "core": joined, + "boxes": boxes.copy(), + "box": union_box(boxes), + "row_id": row_id, + "row_box": row_box, + "keyword": bool(KEYWORD.match(row_text)), + "text": row_text, + "matches_target": joined == wanted + if wanted is not None + else target_digest is not None and digest(joined) == target_digest, + } + ) + return found + def normalize(text: str) -> str: """Uppercase and keep only A-Z and 0-9.""" @@ -109,7 +338,15 @@ def number_read(lines: Sequence[dict], target_digest: str, length: int, lenient: def class_read(lines: Sequence[dict], label: str) -> bool: target = normalize_class(label) - return any(target in normalize_class(text) for text, _ in rows_of_text(lines)) + if not target: + return False + # Keep token boundaries before dropping separators: CL20 must not match CL200. + pattern = r"(? str: diff --git a/experiments/meter-closeup/tests/test_fieldtest.py b/experiments/meter-closeup/tests/test_fieldtest.py index 360ed4c4a..69748e9e8 100644 --- a/experiments/meter-closeup/tests/test_fieldtest.py +++ b/experiments/meter-closeup/tests/test_fieldtest.py @@ -1,6 +1,7 @@ import io import numpy as np +import pytest from PIL import Image from meter_eval import retake @@ -53,3 +54,122 @@ def test_the_orientation_tag_is_applied(tmp_path): exif[0x0112] = 6 # rotate 90 degrees clockwise to display Image.new("RGB", (300, 100), "white").save(tmp_path / "turned.jpg", exif=exif.tobytes()) assert upright_pixels(tmp_path / "turned.jpg", tmp_path).size == (100, 300) + + +@pytest.mark.parametrize( + "number, text, expected, expected_rank", + [ + ("12-34567", "No. 1234567", "yes", 1), + ("No. 1234567", "No. 1234567", "yes", 1), + ("No. 1234567", "1234567", "yes", 1), + ("No. 1234567", "1234568", "no", None), + ("1234567", "12345678", "no", None), + ("1234567", "91234567", "no", None), + ("1234567", "No. 123 4567", "yes", 1), + ("1234567", "1234567 240 V", "yes", 1), + ("1234567", "123 4567 240 V", "yes", 1), + ("1234567", "1234567 8", "no", None), + ("1234567", "123 4567", "yes", 1), + ("1234567", "1234567", "yes", 1), + ("123ABC456789", "123ABC456789", "yes", 1), + ("123ABC456789", "123abc456789", "yes", 1), + ("123ABC456789", "123ABC 456789", "yes", 1), + ("ABC1234567", "ABC1234567", "yes", 1), + ("ABC1234567", "1234567", "no", None), + ("S/N: AB-12345", "AB12345", "yes", 1), + ("S/N: AB-12345", "ab 12345", "yes", 1), + ("S/N: AB-12345", "AB12346", "no", None), + ("Nr. 1234567", "1234567", "yes", 1), + ("N° 1234567", "1234567", "yes", 1), + ], +) +def test_fieldtest_scores_reads_and_ranks_without_a_dataset_key( + tmp_path, monkeypatch, capsys, number, text, expected, expected_rank +): + import sys + + from meter_eval import fieldtest, match + + class Reader: + def __enter__(self): + return self + + def __exit__(self, *exc): + pass + + def read(self, path, barcodes): + return {"lines": [{"text": text, "box": BOX}], "barcodes": []} + + def no_key(): + raise AssertionError("field test must not load the dataset key") + + Image.new("RGB", (256, 256), "white").save(tmp_path / "photo.jpg") + monkeypatch.setattr(fieldtest, "Reader", Reader) + monkeypatch.setattr(sys, "argv", ["fieldtest", str(tmp_path), "--number", number]) + with monkeypatch.context() as without_key: + without_key.setattr(match, "key", no_key) + fieldtest.main() + output = capsys.readouterr().out + assert f"| photo.jpg | {expected} | {expected_rank or '–'} |" in output + assert f"1 photos; {int(expected == 'yes')} retakes asked for photos that read" in output + + +@pytest.mark.parametrize("text", ["123 V 4567", "12345678"]) +def test_read_number_does_not_remove_letters_or_match_partial_groups(text): + from meter_eval.fieldtest import reads_number + + assert not reads_number(text, "1234567") + + +@pytest.mark.parametrize( + "text,target,expected", + [ + ("123-4567", "1234567", True), + ("123ABC 456789", "123abc-456789", True), + ("ABC1234567", "ABC1234567", True), + ("1234567", "ABC1234567", False), + ("X123ABC456789", "123ABC456789", False), + ], +) +def test_read_number_matches_joined_target_groups(text, target, expected): + from meter_eval.fieldtest import reads_number + + assert reads_number(text, target) is expected + + +@pytest.mark.parametrize( + "label", + [ + "No", + "No.", + "#", + "S/N", + "SN", + "Serial", + "Serial No", + "Meter", + "Meter No", + "Meter #", + "Nr.", + "N°", + "№", + "N0", + ], +) +@pytest.mark.parametrize("suffix", ["", ".", ":", "#"]) +def test_read_target_strips_only_recognised_label_words(label, suffix): + from meter_eval.fieldtest import reads_number + from meter_eval.match import read_target + + target = f" ({label.swapcase()}{suffix}) AB-12345 " + assert read_target(target) == "ab12345" + assert reads_number("AB12345", target) + assert reads_number("ab 12345", target) + assert not reads_number("AB12346", target) + + +@pytest.mark.parametrize("serial", ["ABC1234567", "ABSN12345", "XNO1234567", "XMETER1234567"]) +def test_read_target_preserves_letters_outside_a_recognised_label_prefix(serial): + from meter_eval.match import read_target + + assert read_target(serial) == serial.casefold() diff --git a/experiments/meter-closeup/tests/test_locate.py b/experiments/meter-closeup/tests/test_locate.py index 4aaebfd8a..f8d17ede7 100644 --- a/experiments/meter-closeup/tests/test_locate.py +++ b/experiments/meter-closeup/tests/test_locate.py @@ -2,6 +2,22 @@ from meter_eval.match import core +def test_serial_letters_distinguish_the_ranked_candidate(): + from meter_eval.fieldtest import reads_number + + result = { + "lines": [ + {"text": "1234567", "box": [0.1, 0.1, 0.6, 0.1]}, + {"text": "ABC1234567", "box": [0.1, 0.3, 0.6, 0.2]}, + ], + "barcodes": [], + } + order = ranked(candidates(result)) + assert order == ["ABC1234567", "1234567"] + assert reads_number(order[0], "ABC1234567") + assert not reads_number(order[1], "ABC1234567") + + def line(text, box=(0.4, 0.5, 0.2, 0.03)): return {"text": text, "box": list(box), "confidence": 1.0} @@ -35,14 +51,14 @@ def test_barcode_equal_to_a_text_token_confirms_it(): assert only["barcode_confirmed"] and only["core"] == "123ABC456789" -def test_partial_text_read_takes_the_barcodes_full_string(): +def test_partial_text_read_is_not_promoted_to_an_unread_barcode(): # Vision dropped the leading 1; the QR payload has the whole number. result = { "lines": [line("2345678")], "barcodes": [{"payload": "X0YZ01234567;12345678", "box": [0.8, 0.3, 0.1, 0.05]}], } [only] = candidates(result) - assert only["core"] == "12345678" and only["barcode_confirmed"] + assert only["core"] == "2345678" and not only["barcode_confirmed"] def test_short_token_inside_a_payload_is_not_confirmed(): @@ -116,7 +132,7 @@ def test_a_tie_between_two_payloads_is_not_promoted(): assert only["core"] == "2345678" and not only["barcode_confirmed"] -def test_a_single_closest_payload_is_still_promoted(): +def test_a_single_closest_payload_does_not_override_the_ocr_contract(): result = { "lines": [line("2345678")], "barcodes": [ @@ -125,4 +141,26 @@ def test_a_single_closest_payload_is_still_promoted(): ], } [only] = candidates(result) - assert only["core"] == "12345678" and only["barcode_confirmed"] + assert only["core"] == "2345678" and not only["barcode_confirmed"] + + +def test_scan_creates_its_output_directory(tmp_path, monkeypatch): + import json + + from meter_eval import locate + + class Reader: + def __enter__(self): + return self + + def __exit__(self, *exc): + pass + + def read(self, path, barcodes): + return {"lines": [], "barcodes": []} + + monkeypatch.setattr(locate, "DATA_DIR", tmp_path) + monkeypatch.setattr(locate, "Reader", Reader) + result = locate.scan([{"id": "m01"}]) + assert result == {"m01": {"lines": [], "barcodes": []}} + assert json.loads((tmp_path / "ocr" / "scan.jsonl").read_text()) == result["m01"] diff --git a/experiments/meter-closeup/tests/test_metrics.py b/experiments/meter-closeup/tests/test_metrics.py index d6e1b7493..3bc340583 100644 --- a/experiments/meter-closeup/tests/test_metrics.py +++ b/experiments/meter-closeup/tests/test_metrics.py @@ -170,3 +170,45 @@ def test_scrub_catches_digits_split_by_letters_and_keeps_short_codes(): assert scrub("utility plate No. 1234") == "utility plate No. ####" # Fewer than four digits within the leak check's window stay readable. assert scrub("a CL200 meter, two plates") == "a CL200 meter, two plates" + + +@pytest.mark.parametrize("text", ["CL200", "CL2000", "XCL20", "CL20A", "1200 CL"]) +def test_class_label_does_not_match_a_longer_token(text): + label = "200 CL" if text == "1200 CL" else "CL20" + assert not class_read([line(text, 0.1, 0.1)], label) + + +def test_class_label_can_be_one_token_on_a_specification_line(): + assert class_read([line("240V CL 20 60Hz", 0.1, 0.1)], "CL20") + assert not class_read([], "") + + +def test_clean_scores_a_number_split_into_short_observations(): + from meter_eval import clean + from meter_eval.ocr import PRIMARY + + target = digest("123456") + row = { + "id": "m01", + "number_agreed": "yes", + "number_agreed_strict": "yes", + "class_kind": "", + "class_label": "", + "number_hmac": target, + "number_hmac_lenient": target, + "number_len": "6", + } + result = clean.evaluate( + row, + { + PRIMARY: { + "lines": [ + line("123", 0.3, 0.5), + line("456", 0.4, 0.5), + ] + } + }, + ) + assert result[f"number_{PRIMARY}"] == 1 + assert result["number_box"] + assert result["tallest_is_number"] == 0 diff --git a/experiments/meter-closeup/tests/test_review_keep.py b/experiments/meter-closeup/tests/test_review_keep.py index 204d612cd..fa8619b78 100644 --- a/experiments/meter-closeup/tests/test_review_keep.py +++ b/experiments/meter-closeup/tests/test_review_keep.py @@ -82,6 +82,8 @@ def test_build_treats_a_kept_correction_as_human_confirmed(tmp_path, monkeypatch monkeypatch.setattr(labels, "HUMAN", tmp_path / "human.csv") [row] = labels.build() assert row["number_hmac"] == labels.digest("ABC7654321") + assert row["number_core_hmac"] == labels.digest("ABC7654321") + assert row["number_core_len"] == len("ABC7654321") assert row["number_agreed"] == row["number_agreed_strict"] == "yes" assert row["human_check"] == "keep" assert "ABC7654321" in labels.known_numbers() diff --git a/experiments/meter-closeup/tests/test_serial_contract.py b/experiments/meter-closeup/tests/test_serial_contract.py new file mode 100644 index 000000000..fbe3fde8c --- /dev/null +++ b/experiments/meter-closeup/tests/test_serial_contract.py @@ -0,0 +1,693 @@ +"""Sam's complete OCR contract: every row must agree on reading and ranking.""" + +import pytest + +from meter_eval.fieldtest import reads_number +from meter_eval.locate import candidates, evaluate, ranked +from meter_eval.match import digest, number_core + +CASES = [ + ("1234567", "1234567", True), + ("1234567", "123 4567", True), + ("1234567", "123-4567", True), + ("1234567", "123 4567 240 V", True), + ("1234567", "1234567 240V", True), + ("1234567", "CL200 1234567", True), + ("1234567", "SN 1234567 MADE IN USA", True), + ("1234567", "1234567 8", False), + ("1234567", "9 1234567", False), + ("1234567", "12345678", False), + ("1234567", "7654321", False), + ("ABC1234567", "ABC1234567", True), + ("ABC1234567", "ABC 1234567", True), + ("ABC1234567", "abc 1234567", True), + ("ABC1234567", "ABC|1234567", True), + ("ABC1234567", "1234567", False), + ("No. 1234567", "1234567", True), + ("Nr. 1234567", "1234567", True), + ("N° 1234567", "1234567", True), + ("S/N: AB-12345", "AB12345", True), + ("S/N: AB-12345", "ab 12345", True), + ("A1234567", "SN A 1234567", True), + ("1234567A", "SN 1234567A", True), + ("1234567", "1234567 240.0V", True), + ("1234567", "240.0V 1234567", True), + ("1234567", "٩1234567", False), + ("1234567", "12345678", False), + ("1234567", "А1234567", False), + ("1234567", "1234567", True), + ("1234567", "1234567", True), + ("ABSN12345", "AB SN 12345", False), + ("ABSN12345", "ABSN12345", True), +] + + +def observations(text): + pieces = text.split("|") + return [ + {"text": piece, "box": [0.1 + i * 0.3, 0.2, 0.25, 0.1]} for i, piece in enumerate(pieces) + ] + + +@pytest.mark.parametrize("target,text,expected", CASES) +def test_read_and_rank_follow_the_complete_contract(target, text, expected): + lines = observations(text) + read = reads_number(lines, target) + found = candidates({"lines": lines, "barcodes": []}, target) + order = ranked(found) + rank = next( + (i + 1 for i, candidate in enumerate(order) if candidate == number_core(target)), None + ) + ranked_read = rank is not None + assert read is expected + assert ranked_read is expected + assert read == ranked_read + result = evaluate( + { + "id": "synthetic", + "class_kind": "", + "number_agreed_strict": "yes", + "number_agreed": "yes", + "number_hmac": digest(number_core(target)), + "number_core_hmac": digest(number_core(target)), + "number_core_len": len(number_core(target)), + }, + {"lines": lines, "barcodes": []}, + "test", + ) + assert bool(result["read"]) is expected + assert (result["rank"] is not None) is expected + assert result["rank"] == rank + if text == "ABC|1234567": + exact = next(c for c in found if c["core"] == "ABC1234567") + assert exact["boxes"] == [line["box"] for line in lines] + assert exact["box"] == pytest.approx([0.1, 0.2, 0.55, 0.1]) + # Input order must not change the serial's x-order concatenation. + assert reads_number(list(reversed(lines)), target) + + +@pytest.mark.parametrize( + "rating", + [ + "240 V", + "240VAC", + "10 A", + "10AMP", + "10 AMPS", + "60HZ", + "5 W", + "2KW", + "3 KWH", + "240VA", + "1 KH", + "1TA", + "3 PH", + "4WIRE", + "CL200", + "CL 200", + "FM 12", + "2S", + "3W", + ], +) +def test_ratings_are_boundaries_on_both_sides(rating): + for text in (f"{rating} 1234567", f"1234567 {rating}"): + lines = observations(text) + assert reads_number(lines, "1234567") + assert ranked(candidates({"lines": lines, "barcodes": []}, "1234567"))[0] == "1234567" + + +def test_other_rows_do_not_complete_a_serial(): + lines = observations("ABC|1234567") + lines[1]["box"][1] = 0.6 + assert not reads_number(lines, "ABC1234567") + + +@pytest.mark.parametrize("text", ["CL 200", "FM 2S", "240V"]) +def test_class_or_unit_number_is_not_a_serial(text): + assert not candidates({"lines": observations(text), "barcodes": []}) + + +@pytest.mark.parametrize("target,text,expected", CASES) +def test_field_cli_reports_the_contract_table( + tmp_path, monkeypatch, capsys, target, text, expected +): + import sys + + from PIL import Image + + from meter_eval import fieldtest + + class Reader: + def __enter__(self): + return self + + def __exit__(self, *exc): + pass + + def read(self, path, barcodes): + return {"lines": observations(text), "barcodes": []} + + Image.new("RGB", (128, 128), "white").save(tmp_path / "photo.jpg") + monkeypatch.setattr(fieldtest, "Reader", Reader) + monkeypatch.setattr(sys, "argv", ["fieldtest", str(tmp_path), "--number", target]) + fieldtest.main() + output = capsys.readouterr().out + assert f"| photo.jpg | {'yes' if expected else 'no'} | {'1' if expected else '–'} |" in output + + +def test_maker_only_observation_requires_a_number_retake(): + import numpy as np + + from meter_eval import retake + from meter_eval.locate import top_candidate + + lines = observations("ITRON") + result = {"lines": lines, "barcodes": []} + assert top_candidate(result) is None + assert not reads_number(lines, "1234567") + sharp = np.random.default_rng(0).integers(0, 256, (128, 128), dtype=np.uint8) + assert retake.reasons(sharp, None) == ["no number found"] + + +def test_maker_on_another_row_cannot_be_the_number_candidate(): + from meter_eval.locate import top_candidate + + lines = [ + {"text": "ITRON", "box": [0.1, 0.1, 0.7, 0.2]}, + {"text": "1234567", "box": [0.1, 0.6, 0.4, 0.1]}, + ] + found = candidates({"lines": lines, "barcodes": []}) + assert [candidate["core"] for candidate in found] == ["1234567"] + assert top_candidate({"lines": lines, "barcodes": []})["core"] == "1234567" + + +def test_two_number_rows_are_each_alone_and_keep_the_prior_rule_pick(): + from meter_eval.locate import rules + + lines = [ + {"text": "1234567", "box": [0.1, 0.1, 0.4, 0.1]}, + {"text": "7654321", "box": [0.1, 0.6, 0.4, 0.2]}, + ] + found = candidates({"lines": lines, "barcodes": []}) + assert all(candidate["alone"] for candidate in found) + assert len({candidate["row_id"] for candidate in found}) == 2 + pick = rules(found, [])["alone on its line, 6-14 characters, not a spec line"] + assert pick["core"] == "7654321" + + +def test_row_features_use_the_source_row_and_keep_candidate_box_separate(): + from meter_eval.locate import near_barcode + + lines = [ + {"text": "SN", "box": [0.1, 0.2, 0.1, 0.1]}, + {"text": "ABC", "box": [0.25, 0.2, 0.1, 0.1]}, + {"text": "1234567", "box": [0.4, 0.2, 0.2, 0.1]}, + ] + found = candidates({"lines": lines, "barcodes": []}, "ABC1234567") + assert {candidate["core"] for candidate in found} == {"ABC1234567", "1234567"} + assert all(not candidate["alone"] for candidate in found) + assert all(candidate["keyword"] and not candidate["spec"] for candidate in found) + assert all( + candidate["height"] == pytest.approx(0.1) and not candidate["vertical"] + for candidate in found + ) + exact = next(candidate for candidate in found if candidate["matches_target"]) + assert exact["row_box"] == pytest.approx([0.1, 0.2, 0.5, 0.1]) + assert exact["box"] == pytest.approx([0.25, 0.2, 0.35, 0.1]) + assert near_barcode(exact, [{"payload": "x", "box": [0.1, 0.31, 0.1, 0.03]}]) + + +def test_keyword_scope_is_the_row_prefix_not_a_later_group(): + lines = observations("MADE SN 1234567") + assert all( + not candidate["keyword"] for candidate in candidates({"lines": lines, "barcodes": []}) + ) + + +@pytest.mark.parametrize("rating", ["240.0V", "240.0 V", "2.5A", "KH 7.2", "KH7.2", "CL200"]) +def test_complete_decimal_ratings_leave_no_partial_candidate(rating): + for text in (f"1234567 {rating}", f"{rating} 1234567"): + found = candidates({"lines": observations(text), "barcodes": []}, "1234567") + assert {candidate["core"] for candidate in found} == {"1234567"} + + +@pytest.mark.parametrize("text", ["٩ 1234567", "1234567 А", "Α 1234567"]) +def test_invalid_ocr_groups_are_numeric_like_neighbours(text): + assert not reads_number(observations(text), "1234567") + + +@pytest.mark.parametrize("target", ["٩1234567", "А1234567", "Α1234567"]) +def test_invalid_target_is_a_loud_argument_error(tmp_path, monkeypatch, capsys, target): + import sys + + from meter_eval import fieldtest + + monkeypatch.setattr(sys, "argv", ["fieldtest", str(tmp_path), "--number", target]) + with pytest.raises(SystemExit) as error: + fieldtest.main() + assert error.value.code == 2 + assert "--number contains an unsupported character" in capsys.readouterr().err + + +def test_standalone_label_shape_inside_a_split_serial_is_unsupported(): + # SN is always a label boundary when printed separately, even within a serial. + assert not reads_number(observations("AB SN 12345"), "ABSN12345") + assert reads_number(observations("ABSN12345"), "ABSN12345") + + +@pytest.mark.parametrize( + "target,text", + [ + ("ABO123456", "AB0123456"), + ("ABI123456", "AB1123456"), + ("A1234567", "А1234567"), + ], +) +def test_confusable_characters_are_not_mapped(target, text): + assert not reads_number(observations(text), target) + + +def test_decimal_rating_spanning_observations_keeps_the_serial_box(): + lines = observations("1234567|240.0|V") + found = candidates({"lines": lines, "barcodes": []}, "1234567") + assert {candidate["core"] for candidate in found} == {"1234567"} + assert found[0]["boxes"] == [lines[0]["box"]] + + +@pytest.mark.parametrize("text", ["SN.240V 1234567", "240V. 1234567", "SN:2.5A 1234567"]) +def test_complete_ratings_at_label_and_punctuation_boundaries(text): + found = candidates({"lines": observations(text), "barcodes": []}, "1234567") + assert {candidate["core"] for candidate in found} == {"1234567"} + + +@pytest.mark.parametrize("serial", ["12345V", "1234.5A", "CL1234", "FM12345"]) +def test_long_rating_shaped_tokens_remain_serials(serial): + assert reads_number(observations(f"SN {serial}"), serial) + + +@pytest.mark.parametrize("unsupported,ascii_serial", [("ß1234", "SS1234"), ("ff1234", "FF1234")]) +def test_unicode_expansions_are_invalid_as_targets_and_ocr(unsupported, ascii_serial): + from meter_eval.match import read_target + + assert number_core(unsupported) is None + assert read_target(unsupported) is None + assert not reads_number(observations(ascii_serial), unsupported) + assert not candidates({"lines": observations(ascii_serial), "barcodes": []}, unsupported) + assert not reads_number(observations(unsupported), ascii_serial) + assert not candidates({"lines": observations(unsupported), "barcodes": []}, ascii_serial) + + +@pytest.mark.parametrize( + "target,text", + [ + ("1234567", "1234567"), + ("1234567", "1234567"), + ("ABC1234567", "ABC1234567"), + ("abc1234567", "ABC1234567"), + ], +) +def test_only_fullwidth_ascii_is_mapped(target, text): + assert reads_number(observations(text), target) + assert ranked(candidates({"lines": observations(text), "barcodes": []}, target))[ + 0 + ] == number_core(target) + + +@pytest.mark.parametrize("payload", ["é1234567", "序號1234", "ß1234", "ff1234", "e\u03011234567"]) +def test_invalid_barcodes_do_not_abort_photo_scoring(tmp_path, monkeypatch, capsys, payload): + import sys + + from PIL import Image + + from meter_eval import fieldtest + from meter_eval.locate import barcode_stats, payload_tokens + + result = { + "lines": observations("1234567"), + "barcodes": [{"payload": payload, "box": [0.1, 0.5, 0.4, 0.1]}], + } + row = { + "id": "synthetic", + "class_kind": "", + "number_agreed_strict": "yes", + "number_agreed": "yes", + "number_hmac": digest("1234567"), + "number_core_hmac": digest("1234567"), + "number_core_len": 7, + } + assert payload_tokens(result["barcodes"]) == set() + assert barcode_stats(row, result) == { + "barcode_found": 1, + "barcode_decoded": 1, + "barcode_has_number": 0, + } + offline = evaluate(row, result, "test") + assert offline["read"] == 1 + assert offline["rank"] == 1 + + class Reader: + def __enter__(self): + return self + + def __exit__(self, *exc): + pass + + def read(self, path, barcodes): + return result + + Image.new("RGB", (128, 128), "white").save(tmp_path / "photo.jpg") + monkeypatch.setattr(fieldtest, "Reader", Reader) + monkeypatch.setattr(sys, "argv", ["fieldtest", str(tmp_path), "--number", "1234567"]) + fieldtest.main() + assert "| photo.jpg | yes | 1 |" in capsys.readouterr().out + + +@pytest.mark.parametrize("target", ["ß1234", "ff1234", "e\u03011234567", "1234567\u0301"]) +def test_unicode_expansion_target_is_a_cli_argument_error(tmp_path, monkeypatch, capsys, target): + import sys + + from meter_eval import fieldtest + + monkeypatch.setattr(sys, "argv", ["fieldtest", str(tmp_path), "--number", target]) + with pytest.raises(SystemExit) as error: + fieldtest.main() + assert error.value.code == 2 + assert "--number contains an unsupported character" in capsys.readouterr().err + + +@pytest.mark.parametrize( + "target,text", + [ + ("E1234567", "e\u03011234567"), + ("1234567", "1234567\u200b8"), + ("1234567", "1234567\u0301"), + ], +) +def test_undefined_characters_invalidate_the_entire_ocr_group(target, text): + assert not reads_number(observations(text), target) + # No longer serial may be assembled by dropping the unsupported character either. + assert candidates({"lines": observations(text), "barcodes": []}, target) == [] + assert number_core(text) is None + + +@pytest.mark.parametrize("character", ["\u0301", "\u200b", "\u200d", "\ufeff", "😀", "♥", "\0"]) +def test_unsupported_marks_symbols_and_format_characters_are_not_separators(character): + for text in (f"123{character}4567", f"{character} 1234567", f"1234567 {character}"): + assert not reads_number(observations(text), "1234567") + assert number_core(f"No. {character}1234567") is None + + +@pytest.mark.parametrize("separator", [" ", "\t", "\n", "\u00a0", "\u2003", "-", "/", ".", ":"]) +def test_only_defined_separators_split_serial_groups(separator): + text = f"123{separator}4567" + assert reads_number(observations(text), "1234567") + assert number_core(text) == "1234567" + + +@pytest.mark.parametrize("text", ["240V\u200b", "\u0301240V", "240V😀"]) +def test_invalid_rating_shaped_group_is_not_extracted_as_a_boundary(text): + assert not reads_number(observations(f"1234567 {text}"), "1234567") + + +@pytest.mark.parametrize( + "label", + [ + "#", + "№", + "N°", + "No", + "No.", + "Nr", + "Nr.", + "S/N", + "SN", + "Serial", + "Meter", + "Serial No", + "Meter No", + "Meter #", + ], +) +def test_attached_label_prefix_is_a_boundary_before_validating_the_suffix(label): + text = f"{label}1234567" + lines = observations(text) + assert reads_number(lines, "1234567") + assert number_core(text) == "1234567" + found = candidates({"lines": lines, "barcodes": []}, "1234567") + assert ranked(found)[0] == "1234567" + assert next(candidate for candidate in found if candidate["core"] == "1234567")["boxes"] == [ + lines[0]["box"] + ] + + +@pytest.mark.parametrize( + "text", ["#é1234567", "№e\u03011234567", "N°123\u200b4567", "No.123\u200d4567", "SN1234567😀"] +) +def test_attached_label_does_not_erase_an_invalid_suffix(text): + assert number_core(text) is None + assert not reads_number(observations(text), "1234567") + assert not candidates({"lines": observations(text), "barcodes": []}, "1234567") + + +@pytest.mark.parametrize( + "payload,target", + [ + ("X0YZ01234567;12345678", "12345678"), + ("ABC1234567;XYZ7654321", "ABC1234567"), + ("ABC1234567;XYZ7654321", "XYZ7654321"), + ("*1234567*", "1234567"), + ("{1234567}", "1234567"), + ("unrelated,1234567", "1234567"), + ("é1234567;1234567", "1234567"), + ("e\u03011234567;1234567", "1234567"), + ], +) +def test_barcode_fields_are_validated_independently(payload, target): + from meter_eval.locate import barcode_stats, payload_tokens + + barcodes = [{"payload": payload, "box": [0.1, 0.5, 0.4, 0.1]}] + assert target in payload_tokens(barcodes) + result = {"lines": observations(target), "barcodes": barcodes} + found = candidates(result, target) + assert next(candidate for candidate in found if candidate["core"] == target)[ + "barcode_confirmed" + ] + row = { + "number_hmac": digest(target), + "number_core_hmac": digest(target), + "number_core_len": len(target), + } + assert barcode_stats(row, result) == { + "barcode_found": 1, + "barcode_decoded": 1, + "barcode_has_number": 1, + } + + +def test_barcode_fields_cannot_join_across_a_delimiter(): + from meter_eval.locate import barcode_stats, payload_tokens + + barcodes = [{"payload": "1234;567"}] + assert "1234567" not in payload_tokens(barcodes) + row = { + "number_hmac": digest("1234567"), + "number_core_hmac": digest("1234567"), + "number_core_len": 7, + } + assert barcode_stats(row, {"barcodes": barcodes})["barcode_has_number"] == 0 + + +def blind_ranking_photo(): + return { + "lines": [ + {"text": "7654321", "box": [0.1, 0.1, 0.4, 0.2]}, + {"text": "1234567", "box": [0.1, 0.6, 0.4, 0.1]}, + ], + "barcodes": [], + } + + +def test_evaluator_scores_a_readable_serial_at_its_blind_rank(): + from meter_eval.locate import rule_table, rules + + result = blind_ranking_photo() + row = { + "id": "m02", + "class_kind": "", + "number_agreed_strict": "yes", + "number_agreed": "yes", + "number_hmac": digest("1234567"), + "number_core_hmac": digest("1234567"), + "number_core_len": 7, + } + assert ranked(candidates(result)) == ["7654321", "1234567"] + scored = evaluate(row, result, "test") + assert scored["read"] == 1 + assert scored["rank"] == 2 + row |= {"number_hmac": digest("9999999"), "number_core_hmac": digest("9999999")} + missed = evaluate(row, result, "test") + assert missed["read"] == 0 + assert missed["rank"] is None + table = rule_table([scored, missed], list(rules([], []))) + assert "ranking, top 1, over read photos only | 1 | – | 0/1 = 0%" in table + assert "ranking, top 3, over read photos only | 1 | – | 1/1 = 100%" in table + + +def test_best_and_all_rules_are_blind_to_the_target_digest(): + from meter_eval.locate import best, rules + + result = blind_ranking_photo() + result["barcodes"] = [ + {"payload": number, "box": [0.1, 0.85, 0.4, 0.05]} for number in ("7654321", "1234567") + ] + blind = candidates(result) + labelled = candidates(result, target_digest=digest("1234567")) + assert best(blind)["core"] == "7654321" + assert best(labelled)["core"] == best(blind)["core"] + assert ranked(labelled) == ranked(blind) + blind_picks = rules(blind, result["barcodes"]) + labelled_picks = rules(labelled, result["barcodes"]) + assert {name: pick and pick["core"] for name, pick in labelled_picks.items()} == { + name: pick and pick["core"] for name, pick in blind_picks.items() + } + + +def test_target_annotations_do_not_change_source_order_or_blind_ties(): + result = blind_ranking_photo() + result["lines"][1]["box"][3] = result["lines"][0]["box"][3] + blind = candidates(result) + for labelled in ( + candidates(result, "1234567"), + candidates(result, target_digest=digest("1234567")), + ): + assert [candidate["core"] for candidate in labelled] == [ + candidate["core"] for candidate in blind + ] + assert ranked(labelled) == ranked(blind) + + +def test_field_cli_reports_the_blind_second_rank_and_uses_the_blind_box( + tmp_path, monkeypatch, capsys +): + import sys + + from PIL import Image + + from meter_eval import fieldtest + + result = blind_ranking_photo() + + class Reader: + def __enter__(self): + return self + + def __exit__(self, *exc): + pass + + def read(self, path, barcodes): + return result + + selected_boxes = [] + + def reasons(gray, box): + selected_boxes.append(box) + return [] + + Image.new("RGB", (128, 128), "white").save(tmp_path / "photo.jpg") + monkeypatch.setattr(fieldtest, "Reader", Reader) + monkeypatch.setattr(fieldtest.retake, "reasons", reasons) + monkeypatch.setattr(sys, "argv", ["fieldtest", str(tmp_path), "--number", "1234567"]) + fieldtest.main() + assert "| photo.jpg | yes | 2 | no |" in capsys.readouterr().out + assert len(selected_boxes) == 1 + assert selected_boxes[0] == pytest.approx(result["lines"][0]["box"]) + + +@pytest.mark.parametrize( + "target,text,expected", + [ + ("Serial No: SN1234567", "Serial No: SN1234567", True), + ("SN1234567", "SN 1234567", True), + ("No. 1234567", "1234567", True), + ("NoSN1234567", "NoSN1234567", True), + ("Serial No: NoSN1234567", "#1234567", True), + ("123 SN4567", "123 SN4567", False), + ], +) +def test_targets_and_ocr_use_the_same_repeated_label_stripping(target, text, expected): + assert number_core(target) == "1234567" + # The last case retains a label boundary between numeric groups in the OCR. + assert reads_number(observations(text), target) is expected + + +@pytest.mark.parametrize("text", ["Serial No: SNé1234567", "NoSN123\u200b4567"]) +def test_repeated_labels_never_remove_an_unsupported_suffix_character(text): + assert number_core(text) is None + assert not reads_number(observations(text), "1234567") + + +def test_field_help_describes_blind_ranking(tmp_path, monkeypatch, capsys): + import sys + + from meter_eval import fieldtest + + monkeypatch.setattr(sys, "argv", ["fieldtest", "--help"]) + with pytest.raises(SystemExit) as error: + fieldtest.main() + assert error.value.code == 0 + help_text = capsys.readouterr().out + assert "blind heuristic ranking" in help_text + assert "--number only scores" in help_text + assert "exact --number match first" not in help_text + + +@pytest.mark.parametrize("target", ["1234567-2S", "ABC 240V", "Serial No: 1234567-2S"]) +def test_cli_rejects_a_serial_with_a_rating_shaped_part(tmp_path, monkeypatch, capsys, target): + import sys + + from meter_eval import fieldtest + + monkeypatch.setattr(sys, "argv", ["fieldtest", str(tmp_path), "--number", target]) + with pytest.raises(SystemExit) as error: + fieldtest.main() + assert error.value.code == 2 + assert "rating-shaped part" in capsys.readouterr().err + + +@pytest.mark.parametrize( + "serial,normalised", + [ + ("1234567-2S", "12345672S"), + ("ABC 240V", "ABC240V"), + ("Serial No: SN1234567-2S", "12345672S"), + ], +) +def test_supplied_serial_normalisation_never_strips_rating_shaped_parts(serial, normalised): + assert number_core(serial) == normalised + assert not reads_number(observations("1234567"), serial) + + +@pytest.mark.parametrize( + "payload,target,confirmed", + [ + ("1234567-2S", "1234567", False), + ("12345678", "1234567", False), + ("12345678", "12345678", True), + ("1234567-2S", "12345672S", True), + ], +) +def test_barcode_confirmation_requires_the_whole_normalised_field(payload, target, confirmed): + from meter_eval.locate import barcode_stats, payload_tokens + + barcodes = [{"payload": payload, "box": [0.1, 0.5, 0.4, 0.1]}] + assert (target in payload_tokens(barcodes)) is confirmed + found = candidates({"lines": observations(target), "barcodes": barcodes}) + candidate = next(candidate for candidate in found if candidate["core"] == target) + assert candidate["barcode_confirmed"] is confirmed + row = { + "number_hmac": digest(target), + "number_core_hmac": digest(target), + "number_core_len": len(target), + } + assert barcode_stats(row, {"barcodes": barcodes})["barcode_has_number"] == int(confirmed) diff --git a/experiments/plane-consistency/run.py b/experiments/plane-consistency/run.py index 9df288f0d..27cd26b91 100644 --- a/experiments/plane-consistency/run.py +++ b/experiments/plane-consistency/run.py @@ -620,6 +620,12 @@ def measured_noise(setup) -> float: return float(np.median([noise_sigma(load_gray(v)) for v in setup.views])) +def peak_rss_mb() -> float: + # getrusage reports bytes on macOS and KiB on Linux. + divisor = 2**20 if sys.platform == "darwin" else 1024 + return resource.getrusage(resource.RUSAGE_SELF).ru_maxrss / divisor + + def main() -> None: started = time.time() check_evals() @@ -682,8 +688,7 @@ def images_of(i): "wall_ft": (setup.wall.right - setup.wall.left) / cov.FEET, "runs": runs, "seconds": round(time.time() - started), - # ru_maxrss is bytes on macOS. - "peak_rss_mb": round(resource.getrusage(resource.RUSAGE_SELF).ru_maxrss / 2**20), + "peak_rss_mb": round(peak_rss_mb()), } RESULTS.mkdir(exist_ok=True) (RESULTS / "plane_consistency.json").write_text(json.dumps(res, indent=1, default=_json)) diff --git a/experiments/plane-consistency/tests/test_run_metadata.py b/experiments/plane-consistency/tests/test_run_metadata.py new file mode 100644 index 000000000..513be6190 --- /dev/null +++ b/experiments/plane-consistency/tests/test_run_metadata.py @@ -0,0 +1,21 @@ +import ast +from pathlib import Path +from types import CodeType, FunctionType, SimpleNamespace + +import pytest + + +def test_peak_rss_units(): + path = Path(__file__).resolve().parents[1] / "run.py" + tree = ast.parse(path.read_text()) + node = next(n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name == "peak_rss_mb") + for platform, rss in [("darwin", 900 * 2**20), ("linux", 900 * 1024)]: + namespace = { + "sys": SimpleNamespace(platform=platform), + "resource": SimpleNamespace( + RUSAGE_SELF=0, getrusage=lambda _, rss=rss: SimpleNamespace(ru_maxrss=rss) + ), + } + module = compile(ast.Module(body=[node], type_ignores=[]), str(path), "exec") + code = next(value for value in module.co_consts if isinstance(value, CodeType)) + assert FunctionType(code, namespace)() == pytest.approx(900) diff --git a/experiments/scoring/FORMATS.md b/experiments/scoring/FORMATS.md index 77096e0b5..a8cd54403 100644 --- a/experiments/scoring/FORMATS.md +++ b/experiments/scoring/FORMATS.md @@ -132,7 +132,7 @@ Map a route as `"unsupported"`. Measure Lab records no routed cable path, and an Every value needs a stated uncertainty: its entry's `plus_minus_ft`, or its key's value in `plus_minus_ft_by_key`. There is no default. The ±0.3 ft in the example is the plan's untested error bar for an AR tap, listed in `docs/00-overview.md` under "Conventions the code relies on". It is not a measured error bar. `session` must match the zip's session id. -The importer converts meters exactly (1 ft = 0.3048 m) and rounds half up to a millionth of a foot. The capture id is the zip's sha256, which must already be in the survey's `captures`. The importer checks that before it unpacks anything, and it refuses a `session.json` larger than 16 MiB. That limit is a chosen safety bound, not calibrated against real captures. `scale_source` is `ar_poses`. `capture_s` runs from the session's start to its last measurement. `processing_s` is null, because the app shows each value as it is tapped. A map or session problem stops the import with a specific message. The importer loads its output through the scorer before it writes the file. +The importer converts meters exactly (1 ft = 0.3048 m) and rounds half up to a millionth of a foot. The capture id is the zip's sha256, which must already be in the survey's `captures`. The importer checks that before it unpacks anything, and it refuses a `session.json` larger than 16 MiB. That limit is a chosen safety bound, not calibrated against real captures. `scale_source` is `ar_poses`. `capture_s` runs from the session's start to its last capture attempt (measurement or refusal). `processing_s` is null, because the app shows each value as it is tapped. A map or session problem stops the import with a specific message. The importer loads its output through the scorer before it writes the file. `--decide` also writes a row named `+rule`. Its outcomes apply the survey's strict rule, described in [METRICS.md](METRICS.md), to the run's own values and uncertainties. With no value the row says unsure, except that an absent feature passes an `at_least` check. The row emulates the check rule in `docs/00-overview.md` ("Every check answers PASS, FAIL or UNSURE") until a real solver exists. diff --git a/experiments/scoring/METRICS.md b/experiments/scoring/METRICS.md index 0a7de44ac..416f406dc 100644 --- a/experiments/scoring/METRICS.md +++ b/experiments/scoring/METRICS.md @@ -52,7 +52,7 @@ The categories do not cover every wrong answer on purpose. A fail where the surv A pass or fail is **decided without its measurement** when its deciding value is missing as `failed` or `unsupported`. A claimed absence counts the same way when it supports any decision on an `at_most` limit, or a fail on an `at_least` check. The scorer counts the decision as usual and also flags and lists it. -An unsure is justified when the survey is borderline or review, or the run's measurement is missing as `failed` or `unsupported`. Any other unsure is avoidable, including one on a measurement the run marked `absent`. +An unsure is justified when the survey is borderline or review, or the run's measurement is missing as `failed` or `unsupported`, even when the survey truth is unknown. With a known survey outcome, any other unsure is avoidable, including one on a measurement the run marked `absent`. When the survey truth is unknown and the run has numeric or claimed-absence evidence, the abstention stays unclassified. Timing is copied from the results files. Rows that share a recording share its capture time. diff --git a/experiments/scoring/src/scoring/inputs.py b/experiments/scoring/src/scoring/inputs.py index 5bea15d66..5781f40b8 100644 --- a/experiments/scoring/src/scoring/inputs.py +++ b/experiments/scoring/src/scoring/inputs.py @@ -248,6 +248,14 @@ def refuse_output_over_inputs(outputs: Iterable[Path], inputs: Iterable[tuple[st """Stop before any write when an output file is one of the inputs, whether by the same path, another spelling of it, a symlink or a hard link.""" inputs = list(inputs) + outputs = list(outputs) + for index, out_path in enumerate(outputs): + for other in outputs[:index]: + same = out_path.resolve() == other.resolve() + if not same and out_path.exists() and other.exists(): + same = os.path.samefile(out_path, other) + if same: + raise InputError(f"{out_path} aliases output {other}; refusing to overwrite it") for out_path in outputs: out_resolved = out_path.resolve() for role, path in inputs: diff --git a/experiments/scoring/src/scoring/measure_lab.py b/experiments/scoring/src/scoring/measure_lab.py index 1dbb73611..b4f57a5e6 100644 --- a/experiments/scoring/src/scoring/measure_lab.py +++ b/experiments/scoring/src/scoring/measure_lab.py @@ -10,9 +10,11 @@ import argparse import hashlib import json +import lzma import sys import tempfile import zipfile +import zlib from dataclasses import dataclass from decimal import ROUND_HALF_UP, Decimal, localcontext from pathlib import Path @@ -100,7 +102,7 @@ class Session: id: str started_at_uptime: Decimal measurements: dict[str, SessionMeasurement] - refusals: set[str] + refusals: dict[str, Decimal] def _number(value: Any, where: str) -> Decimal: @@ -228,6 +230,15 @@ def load_session(path: Path, truth: Truth) -> Session: raw = handle.read(MAX_SESSION_JSON_BYTES + 1) except zipfile.BadZipFile: raise InputError(f"{path}: not a zip file") from None + except ( + NotImplementedError, + RuntimeError, + OSError, + EOFError, + zlib.error, + lzma.LZMAError, + ) as error: + raise InputError(f"{path}: cannot read session.json: {error}") from None where = f"{path}!{member}" if len(raw) > MAX_SESSION_JSON_BYTES: raise InputError( @@ -289,11 +300,15 @@ def load_session(path: Path, truth: Truth) -> Session: _check_endpoints(entry, measurement, points, walls, f"{at} ({measurement_id})") measurements[measurement_id] = measurement - refusals: set[str] = set() + refusals: dict[str, Decimal] = {} for index, entry in enumerate(_list(data, "refusals", where)): + at = f"{where}: refusals[{index}]" if not isinstance(entry, dict): - raise InputError(f"{where}: refusals[{index}]: expected an object") - refusals.add(_text(entry.get("id"), f"{where}: refusals[{index}].id")) + raise InputError(f"{at}: expected an object") + refusal_id = _text(entry.get("id"), f"{at}.id") + if refusal_id in refusals: + raise InputError(f"{at}.id: refusal {refusal_id!r} is listed twice") + refusals[refusal_id] = _number(entry.get("time"), f"{at}.time") return Session( zip_path=path, @@ -408,6 +423,7 @@ def _check_map(mapping: Map, session: Session, truth: Truth) -> None: 'each to a session measurement, a refusal, "absent" or "unsupported"' ) routes = _route_measurements(truth) + used_measurements: dict[str, str] = {} for survey_id, entry in mapping.entries.items(): at = f"{where}: measurements.{survey_id}" if survey_id in routes and entry != "unsupported": @@ -420,6 +436,13 @@ def _check_map(mapping: Map, session: Session, truth: Truth) -> None: raise InputError(f"{at}.refusal: {session.member} has no refusal {entry.refusal!r}") if not isinstance(entry, FromSession): continue + if entry.measurement in used_measurements: + raise InputError( + f"{at}.session_measurement: {entry.measurement!r} is already mapped to " + f"{used_measurements[entry.measurement]!r}; distinct survey distances need " + "distinct session measurements" + ) + used_measurements[entry.measurement] = survey_id measurement = session.measurements.get(entry.measurement) if measurement is None: raise InputError( @@ -511,13 +534,14 @@ def build_results( if survey_id in mapping.entries } times = [measurement.time for measurement in session.measurements.values()] + times.extend(session.refusals.values()) capture_s = None if times: capture_s = (max(times) - session.started_at_uptime).quantize( SECONDS_PLACES, rounding=ROUND_HALF_UP ) - if capture_s < 0: - raise InputError(f"{session.zip_path}: a measurement predates startedAtUptime") + if min(times) < session.started_at_uptime: + raise InputError(f"{session.zip_path}: a capture attempt predates startedAtUptime") return { "format": FORMAT, "unit": UNIT, diff --git a/experiments/scoring/src/scoring/metrics.py b/experiments/scoring/src/scoring/metrics.py index d48b8c2c2..ef37c3584 100644 --- a/experiments/scoring/src/scoring/metrics.py +++ b/experiments/scoring/src/scoring/metrics.py @@ -255,10 +255,13 @@ def score_check( missed = reported == "pass" and truth in ("borderline", "review") over_caution = reported in ("unsure", "fail") and truth == "pass" rejection = reported == "fail" and truth == "pass" - if reported == "unsure": - evidence_missing = measurement.status in ("missing_unsupported", "missing_failed") - justified = truth in ("borderline", "review") or evidence_missing - abstention = "justified" if justified else "avoidable" + if reported == "unsure": + missing = measurement.reported.missing if measurement.reported else None + evidence_missing = missing in ("unsupported", "failed") + if evidence_missing or truth in ("borderline", "review"): + abstention = "justified" + elif truth != "unknown": + abstention = "avoidable" return CheckScore( check=check, diff --git a/experiments/scoring/src/scoring/report.py b/experiments/scoring/src/scoring/report.py index fc426bb32..7c66338e5 100644 --- a/experiments/scoring/src/scoring/report.py +++ b/experiments/scoring/src/scoring/report.py @@ -284,7 +284,15 @@ def _text(value: str) -> str: def _cell(value: str) -> str: # GitHub splits a table row at every unescaped pipe, even inside a code span. - return value.replace("|", "\\|") + return value.replace("\\", "\\\\").replace("|", "\\|") + + +def _table_code(value: str) -> str: + # Code spans preserve backslashes. Entities prevent Markdown from parsing the identifier + # inside the HTML code tag, including paired backticks or emphasis punctuation. + if "\\" in value: + return "" + "".join(f"&#{ord(char)};" for char in _one_line(value)) + "" + return _code(value) def _table(header: list[str], rows: list[list[str]]) -> list[str]: @@ -343,7 +351,7 @@ def _distances(runs: list[RunScore]) -> list[str]: within, with_uncertainty = run.within_reported rows.append( [ - _code(run.results.pipeline), + _table_code(run.results.pipeline), run.results.scale_source, f"{run.count('scored')}/{denominator}", inches(run.median_abs_error_in), @@ -355,6 +363,7 @@ def _distances(runs: list[RunScore]) -> list[str]: str(run.count("phantom")), str(run.count("absent_agreed")), str(run.count("not_surveyed")), + _table_code(run.results.capture), ] ) header = [ @@ -370,6 +379,7 @@ def _distances(runs: list[RunScore]) -> list[str]: "Phantom", "Absent, agreed", "Not surveyed", + "Capture", ] return [ "", @@ -402,7 +412,14 @@ def _checks(runs: list[RunScore]) -> list[str]: ] else: decided = ["no decisions", "", "", "", "", "", "", ""] - rows.append([_code(run.results.pipeline), *decided, str(run.could_flip)]) + rows.append( + [ + _table_code(run.results.pipeline), + *decided, + str(run.could_flip), + _table_code(run.results.capture), + ] + ) header = [ "Pipeline", "Agree", @@ -414,6 +431,7 @@ def _checks(runs: list[RunScore]) -> list[str]: "Unsure, justified", "Unsure, avoidable", "Error could flip", + "Capture", ] return [ "", @@ -440,13 +458,19 @@ def _checks(runs: list[RunScore]) -> list[str]: def _timing(runs: list[RunScore]) -> list[str]: rows = [ [ - _code(run.results.pipeline), + _table_code(run.results.pipeline), seconds(run.results.capture_seconds) or "not recorded", seconds(run.results.processing_seconds) or "not recorded", + _table_code(run.results.capture), ] for run in runs ] - return ["", "### Timing", "", *_table(["Pipeline", "Capture (s)", "Processing (s)"], rows)] + return [ + "", + "### Timing", + "", + *_table(["Pipeline", "Capture (s)", "Processing (s)", "Capture"], rows), + ] def _survey_text(score: CheckScore) -> str: @@ -469,7 +493,8 @@ def _wrong_pass(run: RunScore, score: CheckScore) -> str: run_value = f"{feet(reported.value_ft)} ft" if has_value else "no value" return ( f"- {_code(run.results.pipeline)} passed {_code(score.check.check)} at " - f"{_code(score.check.candidate)}; {_survey_text(score)} (run measured {run_value})." + f"{_code(score.check.candidate)}; {_survey_text(score)} (run measured {run_value}). " + f"Capture {_code(run.results.capture)}." ) @@ -480,7 +505,7 @@ def _decision_without_measurement(run: RunScore, score: CheckScore) -> str: f"- {_code(run.results.pipeline)} reported {score.reported} for " f"{_code(score.check.check)} at {_code(score.check.candidate)} with measurement " f"{_code(score.check.measurement)} missing " - f"({reported.missing}); {_survey_text(score)}." + f"({reported.missing}); {_survey_text(score)}. Capture {_code(run.results.capture)}." ) diff --git a/experiments/scoring/tests/test_fixtures.py b/experiments/scoring/tests/test_fixtures.py index e203e99d8..aa3ad61e5 100644 --- a/experiments/scoring/tests/test_fixtures.py +++ b/experiments/scoring/tests/test_fixtures.py @@ -103,7 +103,7 @@ def by_key(rows: list[dict[str, str]], *keys: str) -> dict[tuple[str, ...], dict "over_cautious": "2", # c1 facing_gap fail, c1 route unsure "false_rejections": "1", # c1 facing_gap "decided_without_measurement": "0", # its unsupported distances got unsure - "abstentions_justified": "1", + "abstentions_justified": "2", "abstentions_avoidable": "1", "could_flip": "2", "capture_s": "420.0", @@ -254,14 +254,15 @@ def test_markdown_summary(scored): assert blind.strip().splitlines() == [ "- `ar-taps` reported pass for `facing_gap` at `c2` with measurement `c2-facing` missing " "(failed); the survey is fail at 2.500 ± 0.020 ft against `facing_gap_ft` at_least " - "3.000 ft.", + "3.000 ft. Capture `synthetic-01-walk-1`.", "- `ar-taps` reported fail for `pool` at `c2` with measurement `c2-pool` missing " "(failed); the survey is unknown (not measured) against `pool_clearance_ft` at_least " - "5.000 ft.", + "5.000 ft. Capture `synthetic-01-walk-1`.", ] assert unsafe.strip() == ( "- `ar-taps` passed `facing_gap` at `c2`; the survey is fail at 2.500 ± 0.020 ft " - "against `facing_gap_ft` at_least 3.000 ft (run measured no value)." + "against `facing_gap_ft` at_least 3.000 ft (run measured no value). " + "Capture `synthetic-01-walk-1`." ) assert ( "- `ar-taps` passed `gas` at `c2`; the survey is borderline at 3.020 ± 0.030 ft against " diff --git a/experiments/scoring/tests/test_measure_lab.py b/experiments/scoring/tests/test_measure_lab.py index 8221db9c6..e0714b9f3 100644 --- a/experiments/scoring/tests/test_measure_lab.py +++ b/experiments/scoring/tests/test_measure_lab.py @@ -514,7 +514,7 @@ def test_end_to_end_scores_and_decide_row_lands_in_checks(tmp_path: Path, capsys assert ( "| `measure-lab` | ar_poses | 3/9 | 1.20 | 1.20 | 3/3 | 2 | 2 | 0 | 0 | 1 | 1 |" in stdout ) - assert "| `measure-lab+rule` | 5/7 | 0 | 0 | 1 | 0 | 0 | 4 | 0 | 0 |" in stdout + assert "| `measure-lab+rule` | 5/7 | 0 | 0 | 1 | 0 | 0 | 5 | 0 | 0 |" in stdout assert "| `measure-lab` | no decisions |" in stdout with (csv_dir / "checks.csv").open(newline="") as handle: diff --git a/experiments/scoring/tests/test_review_debt.py b/experiments/scoring/tests/test_review_debt.py new file mode 100644 index 000000000..ccfede853 --- /dev/null +++ b/experiments/scoring/tests/test_review_debt.py @@ -0,0 +1,187 @@ +"""Regression cases for unresolved scoring comments on merged PR #4.""" + +import json +import lzma +import os +import struct +import zipfile +import zlib +from decimal import Decimal + +import pytest +from helpers import FIXTURES, SESSION_ZIP, reported, survey, threshold +from test_measure_lab import Setup, listed_truth + +from scoring.cli import main +from scoring.inputs import Check, InputError, load_rules, load_truth +from scoring.measure_lab import load_session +from scoring.metrics import score_check, score_measurement +from scoring.report import CSV_NAMES, _table, _table_code + +RULES = FIXTURES / "rules.json" +TRUTH = FIXTURES / "truth" / "synthetic-01.json" +RESULT = FIXTURES / "results" / "ar-taps.json" +MAP = FIXTURES / "measure-lab" / "map.json" + + +@pytest.mark.parametrize("session_id", ["m3", "m6"]) +def test_duplicate_session_measurements_are_rejected(tmp_path, session_id): + setup = Setup(tmp_path) + setup.map["measurements"]["c1-gas"]["session_measurement"] = session_id + setup.map["measurements"]["c2-gas"]["session_measurement"] = session_id + assert "distinct survey distances need distinct session measurements" in setup.error() + + +def test_capture_duration_includes_last_refusal(tmp_path): + setup = Setup(tmp_path) + setup.session["refusals"][0]["time"] = 1500.75 + assert setup.run()["timing"]["capture_s"] == Decimal("500.500") + + +def test_refusal_only_session_has_capture_duration(tmp_path): + setup = Setup(tmp_path) + setup.session["measurements"] = [] + setup.map["measurements"] = { + survey_id: "unsupported" for survey_id in setup.map["measurements"] + } + assert setup.run()["timing"]["capture_s"] == Decimal("299.750") + + +def test_refusal_before_start_is_rejected(tmp_path): + setup = Setup(tmp_path) + setup.session["refusals"][0]["time"] = 999 + assert "capture attempt predates startedAtUptime" in setup.error() + + +@pytest.mark.parametrize("failure", [NotImplementedError, RuntimeError, OSError, EOFError]) +def test_unreadable_zip_member_is_input_error(tmp_path, monkeypatch, failure): + truth = load_truth(TRUTH, load_rules(RULES)) + + def unreadable(*args, **kwargs): + raise failure("unreadable compression") + + monkeypatch.setattr(zipfile.ZipFile, "open", unreadable) + with pytest.raises(InputError, match=r"cannot read session\.json"): + load_session(SESSION_ZIP, truth) + args = [str(SESSION_ZIP), "--map", str(MAP), "--rules", str(RULES), "--truth", str(TRUTH)] + out = tmp_path / "out.json" + assert main(["import-measure-lab", *args, "--out", str(out)]) == 2 + assert not out.exists() + + +@pytest.mark.parametrize( + ("compression", "decompression_error"), + [(zipfile.ZIP_DEFLATED, zlib.error), (zipfile.ZIP_LZMA, lzma.LZMAError)], +) +def test_corrupt_compressed_member_is_input_error( + tmp_path, capsys, compression, decompression_error +): + path = tmp_path / "corrupt.zip" + with zipfile.ZipFile(path, "w", compression=compression) as archive: + archive.writestr("session.json", b"{}") + raw = bytearray(path.read_bytes()) + name_length, extra_length = struct.unpack_from("p" + "\" * backslashes + "|x |" + assert rendered.count("|") == 2 + + +def test_backslash_code_cell_cannot_inject_inline_markdown(): + rendered = _table(["Pipeline"], [[_table_code("\\`x`*y*_z_")]])[-1] + assert rendered == ( + "| \`x`*y*_z_<b> |" + ) + + +@pytest.mark.parametrize("link_kind", ["symlink", "hardlink"]) +@pytest.mark.parametrize("first,second", [(0, 1), (0, 2), (1, 2)]) +def test_csv_destination_aliases_are_rejected(tmp_path, capsys, link_kind, first, second): + out = tmp_path / "out" + out.mkdir() + target = out / CSV_NAMES[first] + target.write_bytes(b"original output") + alias = out / CSV_NAMES[second] + if link_kind == "symlink": + alias.symlink_to(target) + else: + os.link(target, alias) + args = ["--rules", str(RULES), "--truth", str(TRUTH), "--results", str(RESULT)] + assert main([*args, "--out", str(out)]) == 2 + captured = capsys.readouterr() + assert "aliases output" in captured.err + assert captured.out == "" + assert target.read_bytes() == alias.read_bytes() == b"original output" + assert not (out / CSV_NAMES[3 - first - second]).exists() + + +def test_all_report_run_labels_include_capture(tmp_path, capsys): + truth = json.loads(TRUTH.read_text()) + truth["captures"].append("second-walk") + truth_path = tmp_path / "truth.json" + truth_path.write_text(json.dumps(truth)) + run = json.loads(RESULT.read_text()) + run["capture"] = "second-walk" + result_path = tmp_path / "second.json" + result_path.write_text(json.dumps(run)) + args = [ + "--rules", + str(RULES), + "--truth", + str(truth_path), + "--results", + str(RESULT), + str(result_path), + "--out", + str(tmp_path / "out"), + ] + assert main(args) == 0 + lines = capsys.readouterr().out.splitlines() + for capture in ("synthetic-01-walk-1", "second-walk"): + rows = [line for line in lines if line.startswith("| `ar-taps`") and f"`{capture}`" in line] + assert len(rows) == 3 + findings = [ + line + for line in lines + if line.startswith("- `ar-taps`") and f"Capture `{capture}`." in line + ] + assert len(findings) == 5 diff --git a/experiments/sensor-budget/README.md b/experiments/sensor-budget/README.md index eacd73acc..68316ca58 100644 --- a/experiments/sensor-budget/README.md +++ b/experiments/sensor-budget/README.md @@ -1,5 +1,7 @@ # Sensor budget +The tracked budget tables predate the conversion of empirical yaw quantiles to Gaussian sigmas. Their anchor-plane estimates and verdict need a new budget run before use. + ## Question What error does physics allow each signal we can't test on data, at the range it would be used? The signals are dual-camera disparity, focus distance, the barometer, UWB, known-size objects, one photo per span, and the phone as a probe. Each gets a formula and sourced optimistic and conservative parameters; [METHODS.md](METHODS.md) and [results/budget.md](results/budget.md) list them. diff --git a/experiments/sensor-budget/budget.py b/experiments/sensor-budget/budget.py index fb2b7d733..c9f862816 100644 --- a/experiments/sensor-budget/budget.py +++ b/experiments/sensor-budget/budget.py @@ -15,6 +15,7 @@ from collections.abc import Callable from dataclasses import asdict, dataclass from pathlib import Path +from statistics import NormalDist import numpy as np @@ -104,10 +105,10 @@ class P: ), # The app's wall plane: one raycast normal at the meter "anchor_yaw_deg": P( - 2.15, - 6.72, + 2.15 / NormalDist().inv_cdf(0.75), + 6.72 / NormalDist().inv_cdf(0.95), "deg", - "added after the run: median and p90 yaw of a 0.3 m depth-patch plane on ETH3D electro (experiments/edge-geometry), a proxy for one ARKit raycast normal; the first run assumed 1 to 5", + "Gaussian sigmas inferred separately from median absolute yaw and p90 absolute yaw of a 0.3 m depth-patch plane on ETH3D electro (experiments/edge-geometry), a proxy for one ARKit raycast normal; the first run assumed 1 to 5; Gaussian shape is assumed, not measured", ), "tap_view_deg": P( 35.0, diff --git a/experiments/sensor-budget/tests/test_budget.py b/experiments/sensor-budget/tests/test_budget.py index bee86e020..eea126350 100644 --- a/experiments/sensor-budget/tests/test_budget.py +++ b/experiments/sensor-budget/tests/test_budget.py @@ -1,5 +1,7 @@ """Strict checks of the closed-form models: each has one correct answer.""" +from statistics import NormalDist + import numpy as np import pytest @@ -74,3 +76,9 @@ def test_one_photo_yaw_term_matches_first_order(): ) def test_verdict_boundaries(kind, opt, cons, want): assert b.verdict(kind, opt, cons) == want + + +def test_empirical_yaw_endpoints_are_converted_to_gaussian_sigmas(): + parameter = b.PARAMS["anchor_yaw_deg"] + assert parameter.opt * NormalDist().inv_cdf(.75) == pytest.approx(2.15) + assert parameter.cons * NormalDist().inv_cdf(.95) == pytest.approx(6.72)