From fb7b6cee5b1c48064e0977f371f47019915a5277 Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Wed, 29 Jul 2026 12:15:10 +0000 Subject: [PATCH] docs: document the R layer in ARCHITECTURE.md; correct the stale src table ARCHITECTURE.md named NONE of the five files in R/ -- it documented only the C++ kernels, so a reader had no map of the layer they actually call. The meta-repo's held-back audit check 8 (MR !76) reports all five. Adds an "R contents" table in the same shape as the existing kernel table, one row per file, covering what the R layer is for: the validate / coerce / pad / dispatch / un-coerce sandwich in thin.R (including why NA is rejected -- NA_integer_ is INT_MIN in the kernels, where it corrupts neighbour sums far from the NA cell -- and why the one-pixel pad exists), how medial_axis() differs from thin() (width information), and distance_transform()'s all-foreground Inf convention. R/RcppExports.R is documented as GENERATED by Rcpp::compileAttributes(), matching how the existing table already documents src/RcppExports.cpp, and lists the nine internal .Call shims plus the regenerate step. No exemption for generated files is needed or proposed: the package's own document already documents its generated .cpp counterpart, and the entry carries information a reader needs. Also corrects the adjacent kernel table, which was contradicted by the new R rows. It listed Lee and K3M as "Stub" and omitted five files. All seven methods are implemented and exercised by test-thin.R with exact expected outputs; hilditch / opta / holt / medial_axis / distance_transform / thinr_common.h were absent entirely. Design constraint 3 ("Lee and K3M throw") is rewritten to the rule it was really encoding -- unsupported input errors rather than silently falling back -- which still holds for NA and >2-D arrays. Docs-only. Verified with the real check-8 code extracted from the figureextract meta-repo's origin/audit/architecture-contents-check: thinr goes from 5 violations to 0. Co-Authored-By: Claude Opus 4.8 --- ARCHITECTURE.md | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 311cada..4d00fe2 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -21,17 +21,36 @@ as_binary_matrix() ──> IntegerMatrix with {0, 1} values restore_storage(skeleton, original) ──> matrix in original storage mode ``` -The dispatcher (`thin()`) selects the C++ implementation by name and handles the coercion sandwich. The C++ implementations are isolated by file, one per algorithm. +The dispatcher (`thin()`) selects the C++ implementation by name and handles the coercion sandwich (plus the one-pixel padding described under `R/thin.R` below). The C++ implementations are isolated by file, one per algorithm. + +## R contents + +The R layer is thin by design: validate, coerce, pad, dispatch, un-coerce. No +pixel loop lives here — every kernel is in `src/`. + +| File | Role | +|---|---| +| `R/thinr-package.R` | Package-level docs (`"_PACKAGE"`). Carries the citation list for all seven methods, the medial-axis / distance-transform summary, the `@useDynLib` registration, and the "relationship to EBImage" statement (EBImage supplies binary morphology but no thinning operator; `thinr` fills that gap under the same LGPL-3 licence). | +| `R/thin.R` | The public dispatcher `thin(image, method, max_iter)` plus the two coercion helpers the whole package shares. `thin()` selects a kernel by name, **pads the image with a one-pixel background margin and crops it back** — the kernels inspect an 8-neighbourhood and so can never delete a pixel in the outermost row/column, which left edge-touching shapes two to three pixels thick — then restores the caller's storage mode. `as_binary_matrix()` maps logical / integer / numeric input to a `{0, 1}` `IntegerMatrix` and **rejects `NA` loudly**: `NA_integer_` is `INT_MIN` in the C++ kernels, where it corrupts neighbour sums far from the `NA` cell. `restore_storage()` returns a logical input as logical and a double input as double, so the return type is never a surprise. Arrays with more than two dimensions are rejected. | +| `R/medial_axis.R` | `medial_axis(image, return_distance)` — the medial axis transform (Blum 1967), distinct from `thin()`: it returns ridge points of the Euclidean distance transform (local maxima along at least one of the four principal directions) and can hand back the per-pixel distance, so each skeleton pixel carries local **width** information. Classical thinning gives a connected 1-pixel skeleton with no width. Same coercion sandwich as `thin()`; delegates to `.medial_axis_cpp()`. | +| `R/distance_transform.R` | `distance_transform(image, metric)` — distance from each foreground pixel to the nearest background pixel, under `"euclidean"` (exact L2, Felzenszwalb & Huttenlocher linear-time separable), `"manhattan"` (L1) or `"chessboard"` (L∞), the latter two by a two-pass forward/backward sweep. Documents its own degenerate case explicitly: an all-foreground image has no nearest background pixel, so every pixel is `Inf` consistently across all three metrics (matching `EBImage::distmap()`), and the region outside the matrix is **not** treated as an implicit background border. | +| `R/RcppExports.R` | **Generated by `Rcpp::compileAttributes()` — do not edit by hand.** The `.Call()` shims for the nine C++ entry points (`.zhang_suen_cpp`, `.guo_hall_cpp`, `.lee_cpp`, `.k3m_cpp`, `.hilditch_cpp`, `.opta_cpp`, `.holt_cpp`, `.medial_axis_cpp`, `.distance_transform_cpp`). All are internal (dot-prefixed, unexported): the only supported entry points are `thin()`, `medial_axis()` and `distance_transform()`. Regenerate together with `src/RcppExports.cpp` whenever an `// [[Rcpp::export]]` signature changes. | ## Algorithm contents | File | Algorithm | Status | Notes | |---|---|---|---| -| `src/zhang_suen.cpp` | Zhang & Suen (1984) | Full | Two sub-iterations per pass; standard parallel thinning. | +| `src/zhang_suen.cpp` | Zhang & Suen (1984) | Full | Two sub-iterations per pass; standard parallel thinning. The default. | | `src/guo_hall.cpp` | Guo & Hall (1989) | Full | Two sub-iterations; uses the OpenCV-canonical C(p) and N(p) formulations. | -| `src/lee.cpp` | Lee (1994) | Stub | 3-D capable; Euler-invariance check. v0.2. | -| `src/k3m.cpp` | Saeed et al. (2010) | Stub | Six-phase look-up-table thinning with strong corner preservation. v0.2. | -| `src/RcppExports.cpp` | (generated) | n/a | Regenerate via `Rcpp::compileAttributes()`. | +| `src/lee.cpp` | Lee, Kashyap & Chu (1994) | Full | 2-D adaptation; four directional sub-iterations. | +| `src/k3m.cpp` | Saeed et al. (2010) | Full | Six-phase look-up-table thinning with strong corner preservation. | +| `src/hilditch.cpp` | Hilditch (1969), parallel form | Full | Single-pass thinning with a look-ahead crossing-number check. | +| `src/opta.cpp` | Naccache & Shinghal (1984) | Full | One-pass safe-point thinning (SPTA). | +| `src/holt.cpp` | Holt et al. (1987) | Full | One-subcycle parallel thinning using edge information about neighbours. | +| `src/medial_axis.cpp` | Blum (1967) MAT | Full | Ridge points of the squared Euclidean distance transform; backs `medial_axis()`. | +| `src/distance_transform.cpp` | Felzenszwalb & Huttenlocher (2012); Rosenfeld & Pfaltz (1968) | Full | Backs `distance_transform()`; Euclidean via the linear-time separable algorithm, Manhattan / chessboard via two-pass sweeps. | +| `src/thinr_common.h` | (shared) | n/a | The single definition of the Zhang-Suen crossing number `A(p)` and neighbour count `B(p)`. All four kernels that need them call these rather than keeping inline copies, so a correctness fix reaches every kernel (thinr 0.3.0.9000, F018). | +| `src/RcppExports.cpp` | (generated) | n/a | Regenerate via `Rcpp::compileAttributes()`, together with `R/RcppExports.R`. | ## Design constraints @@ -39,9 +58,9 @@ The dispatcher (`thin()`) selects the C++ implementation by name and handles the 2. **API stability.** `thin()` is the public thinning surface. Any signature change is a major version bump. The default algorithm is locked to Zhang-Suen. -3. **Stubs error, don't silently fall back.** Lee and K3M throw with a message that names the planned version. Silent fall-backs to Zhang-Suen would hide the limitation. +3. **Unsupported input errors, it does not silently fall back.** All seven methods are now implemented (Lee and K3M shipped after this constraint was written; the contents table above is the current state). The rule it encodes still holds for everything else the package cannot do: `NA` pixels and >2-D arrays abort with a message naming the limitation, because a silent coercion or a silent fall-back to Zhang-Suen would hide it. -4. **2-D for now.** Higher-dimensional arrays are explicitly rejected. The Lee implementation in v0.2 introduces 3-D support; the dispatcher will route based on `length(dim(image))`. +4. **2-D for now.** Higher-dimensional arrays are explicitly rejected. `src/lee.cpp` implements the 2-D adaptation of Lee, Kashyap & Chu; the 3-D form is not wired up, and adding it would route in the dispatcher on `length(dim(image))`. ## Versioning