Skip to content

refactor: extract plane_validation module (Candidate 1) #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ofeq-dev merged 1 commit into from
Nov 7, 2026
Merged

refactor: extract plane_validation module (Candidate 1) #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 10 additions & 225 deletions Ex0.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@

from __future__ import annotations

import csv
import math
import sys
from collections.abc import Iterable, Iterator, Sequence
Expand All @@ -41,9 +40,9 @@
import numpy as np
import nrrd


class UsageError(Exception):
"""A user-fixable input/usage problem reported as a one-line CLI error (FR-2)."""
# Import plane validation from the new module
from plane_validation import Plane, PlaneError, parse_planes, count_planes
from errors import UsageError


def _require_file(label: str, path: Path) -> None:
Expand Down Expand Up @@ -131,217 +130,12 @@ def load_volume(path: str | Path) -> np.ndarray:


# ---------------------------------------------------------------------------
# Plane-list parsing + per-row validation (FR-7..FR-11)
# Frame rendering (FR-12, FR-13; geometry pinned in docs/algorithm.md)
# ---------------------------------------------------------------------------

#: Plane validity tolerance (brief Q&A; SRS FR-10).
EPS = 0.001

#: Relative tolerance for the coplanarity predicate (rule 3, 4-point): the
#: scalar triple product of the leg directions and the apex separation is
#: compared against the product of the involved lengths, so the test is
#: scale-free. Genuinely non-coplanar point sets sit at O(0.01..1); the
#: corpus's rectangular planes sit at float-dust level (~1e-18).
COPLANAR_RTOL = 1e-9

Point = tuple[float, float, float]


@dataclass(frozen=True)
class Plane:
"""One validated CSV plane row (FR-8).

``points`` keeps the brief's column order: 3-point rows are
``(a, b, c)`` (legs measured from ``a``); 4-point rows are
``(a1, a2, b, c)`` (legs ``a1→b``/``a2→c``, bases ``a1→a2``/``b→c``).
Canonical voxel-index space per ADR 0002 - no world-space conversion.
"""

row: int
points: tuple[Point, ...]

@property
def is_trapezoid(self) -> bool:
return len(self.points) == 4


@dataclass(frozen=True)
class PlaneError:
"""One CSV row that failed validation (FR-10/FR-11).

``reason`` names the exact failing predicate with its numbers - the same
text the future error-frame banner (FR-15) and CLI log line (FR-16)
carry. ``banner()`` composes the full line, addressable by row index.
"""

row: int
reason: str

def banner(self) -> str:
return f"row {self.row}: {self.reason}"


def _fmt(value: float) -> str:
"""Compact %g formatting for banner numbers (100 not 100.00000)."""
return f"{value:g}"


def _dist(p: Point, q: Point) -> float:
return math.dist(p, q)


def _cross(u: Point, v: Point) -> Point:
return (
u[1] * v[2] - u[2] * v[1],
u[2] * v[0] - u[0] * v[2],
u[0] * v[1] - u[1] * v[0],
)


def _sub(p: Point, q: Point) -> Point:
return (p[0] - q[0], p[1] - q[1], p[2] - q[2])


def _dot(u: Point, v: Point) -> float:
return u[0] * v[0] + u[1] * v[1] + u[2] * v[2]


def _validate_triangle(a: Point, b: Point, c: Point) -> str | None:
"""3-point validity + constructibility: the failing rule, or None.

Rule 1 (FR-10): ``|dist(a,b) - dist(a,c)| < EPS`` - the isosceles legs
are measured from the first point. Constructibility (FR-11): no
zero-length leg; ``b == c`` makes every row's width 0 (the two legs
collapse onto one direction), a degenerate frame.
"""
dab, dac = _dist(a, b), _dist(a, c)
if dab == 0:
return "zero-length leg a→b (a == b)"
if dac == 0:
return "zero-length leg a→c (a == c)"
if abs(dab - dac) >= EPS:
return (
f"|dist(a,b) − dist(a,c)| = {_fmt(abs(dab - dac))} ≥ EPS = {EPS}"
f" (dist(a,b) = {_fmt(dab)}, dist(a,c) = {_fmt(dac)})"
)
if b == c:
return "degenerate: b == c (parallel legs give a zero-width frame)"
return None


def _validate_trapezoid(a1: Point, a2: Point, b: Point, c: Point) -> str | None:
"""4-point validity + constructibility: the failing rule, or None.

Rules in the brief's order (FR-10): legs-equal ``|dist(a1,b) -
dist(a2,c)| < EPS``; base ``dist(a1,a2) <= dist(b,c)`` (with the EPS
tolerance - the corpus's rectangles exceed it by ~4e-7 float dust);
coplanarity of the two legs. Constructibility (FR-11): no zero-length
leg; ``a1 == a2`` with same-direction legs maximizes the zero-width
collapse, while ``a1 == a2`` with diverging legs (including opposite-way
legs) is a triangle and renders fine.
Parallel legs with a nonzero top base are legal rectangles (the whole
head.csv corpus).
"""
d1, d2 = _dist(a1, b), _dist(a2, c)
if d1 == 0:
return "zero-length leg a1→b (a1 == b)"
if d2 == 0:
return "zero-length leg a2→c (a2 == c)"
if abs(d1 - d2) >= EPS:
return (
f"|dist(a1,b) − dist(a2,c)| = {_fmt(abs(d1 - d2))} ≥ EPS = {EPS}"
f" (dist(a1,b) = {_fmt(d1)}, dist(a2,c) = {_fmt(d2)})"
)
d12, dbc = _dist(a1, a2), _dist(b, c)
if d12 > dbc + EPS:
return (
f"dist(a1,a2) = {_fmt(d12)} > dist(b,c) = {_fmt(dbc)}"
f" (rule: dist(a1,a2) ≤ dist(b,c))"
)
leg1, leg2, sep = _sub(b, a1), _sub(c, a2), _sub(a2, a1)
triple = abs(_dot(sep, _cross(leg1, leg2)))
if triple > COPLANAR_RTOL * d1 * d2 * (d12 or 1.0):
return "legs a1→b and a2→c do not lie in the same plane (rule 3)"
if d12 == 0:
# a1 == a2: the frame is zero-width on every row exactly when the
# legs step the same way (a2->c == a1->b). Opposite-way legs
# (a2->c == -(a1->b)) are the 3-point triangle geometry — apex a1,
# base through b and c — and render.
if leg1 == leg2:
return "degenerate: a1 == a2 with same-direction legs (zero-width frame)"
return None


def _parse_row(row_idx: int, raw: list[str]) -> Plane | PlaneError:
"""Parse and validate one CSV data row into its final record (FR-8..FR-11)."""

def fail(reason: str) -> PlaneError:
return PlaneError(row=row_idx, reason=reason)

count_cell = raw[0].strip() if raw else ""
try:
n = float(count_cell)
except ValueError:
return fail(f"point count is not a number: '{count_cell}'")
if n not in (3.0, 4.0):
return fail(f"point count must be 3 or 4, got '{count_cell}'")
n = int(n)

if len(raw) < 1 + 3 * n:
return fail(f"expected {1 + 3 * n} columns for a {n}-point row, got {len(raw)}")
points: list[Point] = []
for i in range(n):
cells = raw[1 + 3 * i : 4 + 3 * i]
point: list[float] = []
for j, cell in enumerate(cells):
try:
point.append(float(cell))
except ValueError:
return fail(f"coordinate is not a number: '{cell.strip()}' (point {i + 1})")
points.append((point[0], point[1], point[2]))

if n == 3:
reason = _validate_triangle(*points)
else:
reason = _validate_trapezoid(*points)
if reason is not None:
return fail(reason)
return Plane(row=row_idx, points=tuple(points))


def _iter_plane_rows(path: Path) -> Iterator[list[str]]:
"""Yield the CSV's non-empty data rows (header excluded), FR-9-tolerant."""
_require_file("plane list", path)
with _os_errors_as_usage("plane list", path):
with open(path, newline="") as file:
reader = csv.reader(file)
header = next(reader, None)
if header is None:
raise UsageError(f"cannot read plane list '{path}': empty file (no header)")
for row in reader:
if any(cell.strip() for cell in row):
yield row


def parse_planes(path: str | Path) -> list[Plane | PlaneError]:
"""Parse the CSV plane list into one record per data row, in row order.

Each non-empty row becomes either a validated :class:`Plane` or a
:class:`PlaneError` naming the exact failing rule (FR-10/FR-11) - no
row ever stops the run (NFR-4 groundwork; the error-frame ticket turns
these into banners). File-level problems (missing, directory, empty)
stay whole-run :class:`UsageError`s (FR-2).

Tolerant of what the corpus provides (FR-9): a missing trailing
newline, integer- or float-coordinate cells, and blank separator rows.
"""
return [_parse_row(i, row) for i, row in enumerate(_iter_plane_rows(Path(path)), 1)]


# ---------------------------------------------------------------------------
# Frame rendering (FR-12, FR-13; geometry pinned in docs/algorithm.md)
# ---------------------------------------------------------------------------

@dataclass(frozen=True)
class Frame:
"""One rendered plane projection, display-scaled to grayscale uint8.
Expand Down Expand Up @@ -384,6 +178,10 @@ def _plane_corners(plane: Plane) -> tuple[Point, Point, Point, Point]:
return (a, a, b, c)


def _dist(p: Point, q: Point) -> float:
return math.dist(p, q)


def _render_plane(volume: np.ndarray, plane: Plane) -> Frame:
"""Render one validated plane as a display-scaled 2D uint8 frame (FR-12).

Expand Down Expand Up @@ -576,7 +374,7 @@ def text_width(value: str) -> int:


# ---------------------------------------------------------------------------
# Plane-list counting (FR-17 groundwork; superseded body: one record per row)
# Canvas assembly (FR-18/FR-14)
# ---------------------------------------------------------------------------

def iter_canvas_frames(
Expand All @@ -597,19 +395,6 @@ def iter_canvas_frames(
)


def count_planes(path: str | Path) -> int:
"""Count the CSV plane-list's data rows (the header row is excluded).

Full parsing/validation of each row lives in :func:`parse_planes`; the
count is simply one record per non-empty row, so the video keeps one
frame per row, in row order (FR-17). Tolerant already of
what the corpus provides: integer- or float-coordinate cells and a
missing trailing newline (FR-9); rows that are entirely empty are not
planes.
"""
return len(parse_planes(path))


# ---------------------------------------------------------------------------
# Video writer (FR-17..FR-21 container contract; encoder choice: docs/adr/0004)
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -736,4 +521,4 @@ def main(argv: list[str] | None = None) -> int:


if __name__ == "__main__":
raise SystemExit(main())
raise SystemExit(main())
14 changes: 9 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ Measured end-to-end (load + render, min-of-repeats) on the reference machine:

| Input | Frames | FPS |
|---|---:|---:|
| `croissant.nrrd` × `croissant.csv` — 500³ uint8, the canonical 500³ case | 100 | **43.6** |
| `onion.nrrd` × `head.csv` | 280 | 382.0 |
| `MRHead_1.nrrd` × `head.csv` | 280 | 381.3 |
| `croissant.nrrd` × `croissant.csv` — 500³ uint8, the canonical 500³ case | 100 | **50.1** |
| `onion.nrrd` × `head.csv` | 280 | 433.5 |
| `MRHead_1.nrrd` × `head.csv` | 280 | 380.2 |

The ≥ 30 FPS hard bar is met; the 100 FPS aspirational target is not met on
this (2013 dual-core) machine. Every row of the full 30-case cross-product —
with the exact one-line command that reproduced it — is in
[`benchmarks/results-2026-11-06.md`](benchmarks/results-2026-11-06.md).
[`benchmarks/results-2026-11-07.md`](benchmarks/results-2026-11-07.md).
Reproduce with:

```sh
Expand Down Expand Up @@ -188,7 +188,11 @@ volume has a matching CSV in [`assets/csv/`](assets/csv/), and
## Repository layout

```
Ex0.py CLI + loader + validation + renderer + writer (single-file deliverable)
Ex0.py CLI orchestrator (imports from modules below)
errors.py Shared UsageError exception
geometry.py Scalar geometry helpers (dist, cross, sub, dot)
csv_reader.py FR-9 tolerant CSV row iterator
plane_validation.py Plane parsing + validation (parse_planes, count_planes)
bench.py Benchmark harness (corpus cross-product, min-of-repeats)
tests/ pytest suite (unit, golden, CLI/MP4, benchmark)
assets/nrrd|csv|videos/ Sample volumes, plane lists and rendered outputs (Git LFS for large files)
Expand Down
44 changes: 44 additions & 0 deletions benchmarks/results-2026-11-07.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Benchmark results

Recorded: 2026-11-07 22:09 +0200
Machine: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz (4 cores)
Software: Python 3.15.2, numpy 2.5.4, encode: cv2.VideoWriter mp4v @30 fps (ADR 0004), excluded from throughput

Methodology (NFR-2): wall time via `time.perf_counter()` (monotonic, highest resolution); best = min-of-repeats (timeit methodology); the **load** scope is the NRRD read; the **render** scope is parse + render + canvas assembly; the mp4 write is *not* exercised: throughput runs do not write the mp4 — encode seconds appear only in the dedicated encode-probe row; throughput = rendered frames ÷ (load + render) wall time.

| input | command | frames | load s | render s | encode s | FPS (load+render) |
|---|---|---|---|---|---|---|
| swiss_roll.nrrd x biscuit_cake.csv | `python bench.py assets/nrrd/swiss_roll.nrrd assets/csv/biscuit_cake.csv --repeats 3` | 120 | 0.180 | 0.343 | 0.000 | 229.3 |
| swiss_roll.nrrd x croissant.csv | `python bench.py assets/nrrd/swiss_roll.nrrd assets/csv/croissant.csv --repeats 3` | 100 | 0.184 | 0.705 | 0.000 | 112.5 |
| swiss_roll.nrrd x head.csv | `python bench.py assets/nrrd/swiss_roll.nrrd assets/csv/head.csv --repeats 3` | 280 | 0.179 | 0.727 | 0.000 | 309.1 |
| swiss_roll.nrrd x lecturer_example.csv | `python bench.py assets/nrrd/swiss_roll.nrrd assets/csv/lecturer_example.csv --repeats 3` | 2 | 0.187 | 0.015 | 0.000 | 9.9 |
| swiss_roll.nrrd x onion.csv | `python bench.py assets/nrrd/swiss_roll.nrrd assets/csv/onion.csv --repeats 3` | 120 | 0.207 | 0.344 | 0.000 | 217.7 |
| swiss_roll.nrrd x swiss_roll.csv | `python bench.py assets/nrrd/swiss_roll.nrrd assets/csv/swiss_roll.csv --repeats 3` | 120 | 0.189 | 0.349 | 0.000 | 223.2 |
| onion.nrrd x biscuit_cake.csv | `python bench.py assets/nrrd/onion.nrrd assets/csv/biscuit_cake.csv --repeats 3` | 120 | 0.052 | 0.310 | 0.000 | 331.6 |
| onion.nrrd x croissant.csv | `python bench.py assets/nrrd/onion.nrrd assets/csv/croissant.csv --repeats 3` | 100 | 0.051 | 0.693 | 0.000 | 134.5 |
| onion.nrrd x head.csv | `python bench.py assets/nrrd/onion.nrrd assets/csv/head.csv --repeats 3` | 280 | 0.050 | 0.596 | 0.000 | 433.5 |
| onion.nrrd x lecturer_example.csv | `python bench.py assets/nrrd/onion.nrrd assets/csv/lecturer_example.csv --repeats 3` | 2 | 0.050 | 0.010 | 0.000 | 33.3 |
| onion.nrrd x onion.csv | `python bench.py assets/nrrd/onion.nrrd assets/csv/onion.csv --repeats 3` | 120 | 0.049 | 0.301 | 0.000 | 342.7 |
| onion.nrrd x swiss_roll.csv | `python bench.py assets/nrrd/onion.nrrd assets/csv/swiss_roll.csv --repeats 3` | 120 | 0.049 | 0.295 | 0.000 | 348.8 |
| croissant.nrrd x biscuit_cake.csv | `python bench.py assets/nrrd/croissant.nrrd assets/csv/biscuit_cake.csv --repeats 3` | 120 | 0.848 | 0.337 | 0.000 | 101.2 |
| croissant.nrrd x croissant.csv | `python bench.py assets/nrrd/croissant.nrrd assets/csv/croissant.csv --repeats 3` | 100 | 0.878 | 1.116 | 0.000 | 50.1 |
| croissant.nrrd x head.csv | `python bench.py assets/nrrd/croissant.nrrd assets/csv/head.csv --repeats 3` | 280 | 0.894 | 0.625 | 0.000 | 184.3 |
| croissant.nrrd x lecturer_example.csv | `python bench.py assets/nrrd/croissant.nrrd assets/csv/lecturer_example.csv --repeats 3` | 2 | 0.905 | 0.018 | 0.000 | 2.2 |
| croissant.nrrd x onion.csv | `python bench.py assets/nrrd/croissant.nrrd assets/csv/onion.csv --repeats 3` | 120 | 0.969 | 0.338 | 0.000 | 91.8 |
| croissant.nrrd x swiss_roll.csv | `python bench.py assets/nrrd/croissant.nrrd assets/csv/swiss_roll.csv --repeats 3` | 120 | 0.871 | 0.344 | 0.000 | 98.8 |
| biscuit_cake.nrrd x biscuit_cake.csv | `python bench.py assets/nrrd/biscuit_cake.nrrd assets/csv/biscuit_cake.csv --repeats 3` | 120 | 0.091 | 0.339 | 0.000 | 278.6 |
| biscuit_cake.nrrd x croissant.csv | `python bench.py assets/nrrd/biscuit_cake.nrrd assets/csv/croissant.csv --repeats 3` | 100 | 0.093 | 0.675 | 0.000 | 130.3 |
| biscuit_cake.nrrd x head.csv | `python bench.py assets/nrrd/biscuit_cake.nrrd assets/csv/head.csv --repeats 3` | 280 | 0.117 | 0.662 | 0.000 | 359.5 |
| biscuit_cake.nrrd x lecturer_example.csv | `python bench.py assets/nrrd/biscuit_cake.nrrd assets/csv/lecturer_example.csv --repeats 3` | 2 | 0.107 | 0.011 | 0.000 | 17.0 |
| biscuit_cake.nrrd x onion.csv | `python bench.py assets/nrrd/biscuit_cake.nrrd assets/csv/onion.csv --repeats 3` | 120 | 0.108 | 0.330 | 0.000 | 274.3 |
| biscuit_cake.nrrd x swiss_roll.csv | `python bench.py assets/nrrd/biscuit_cake.nrrd assets/csv/swiss_roll.csv --repeats 3` | 120 | 0.099 | 0.332 | 0.000 | 278.7 |
| MRHead_1.nrrd x biscuit_cake.csv | `python bench.py assets/nrrd/MRHead_1.nrrd assets/csv/biscuit_cake.csv --repeats 3` | 120 | 0.081 | 0.327 | 0.000 | 294.2 |
| MRHead_1.nrrd x croissant.csv | `python bench.py assets/nrrd/MRHead_1.nrrd assets/csv/croissant.csv --repeats 3` | 100 | 0.080 | 0.682 | 0.000 | 131.2 |
| MRHead_1.nrrd x head.csv | `python bench.py assets/nrrd/MRHead_1.nrrd assets/csv/head.csv --repeats 3` | 280 | 0.087 | 0.650 | 0.000 | 380.2 |
| MRHead_1.nrrd x lecturer_example.csv | `python bench.py assets/nrrd/MRHead_1.nrrd assets/csv/lecturer_example.csv --repeats 3` | 2 | 0.081 | 0.010 | 0.000 | 22.1 |
| MRHead_1.nrrd x onion.csv | `python bench.py assets/nrrd/MRHead_1.nrrd assets/csv/onion.csv --repeats 3` | 120 | 0.080 | 0.327 | 0.000 | 294.5 |
| MRHead_1.nrrd x swiss_roll.csv | `python bench.py assets/nrrd/MRHead_1.nrrd assets/csv/swiss_roll.csv --repeats 3` | 120 | 0.081 | 0.325 | 0.000 | 295.9 |
| croissant.nrrd x croissant.csv (encode probe: throwaway mp4 written) | `python bench.py assets/nrrd/croissant.nrrd assets/csv/croissant.csv --repeats 1 --encode-probe` | 100 | 0.882 | 1.144 | 0.399 | 49.4 |

NFR-1 hard bar (croissant.nrrd x croissant.csv, 500³ uint8): 50.1 FPS — met the 30 FPS hard bar (target of 100 FPS NOT met).
Repeatability (drift criterion < ~10%): 1.2% between two independent min-of-3 runs of the canonical pair.
Loading
Loading