Skip to content

fix(neighbors): implement closest-neighbor dist/ang and cap PERCENT_T…#359

Open
darkclad wants to merge 3 commits into
PolusAI:mainfrom
darkclad:main-neighbors-closest-and-percent-touching
Open

fix(neighbors): implement closest-neighbor dist/ang and cap PERCENT_T…#359
darkclad wants to merge 3 commits into
PolusAI:mainfrom
darkclad:main-neighbors-closest-and-percent-touching

Conversation

@darkclad

@darkclad darkclad commented Jul 1, 2026

Copy link
Copy Markdown

…OUCHING

Two production defects:

  • CLOSEST_NEIGHBOR{1,2}DIST/ANG and ANG_BW_NEIGHBORS* need CENTROID_X/Y from Basic Morphology. When only neighbor features were requested, Basic Morphology was skipped in the trivial-ROI reduce, so the centroids stayed 0 and the closest-neighbor distances/angles came out 0. Run Basic Morphology whenever neighbor features are requested (reduce_trivial_rois.cpp).
  • PERCENT_TOUCHING summed a per-neighbor touch count, so a contour pixel adjacent to several neighbors was counted repeatedly and the total could exceed 100% (e.g. 127%). Build a deduped per-ROI mask of contour pixels touching ANY neighbor (8-connected adjacency) and divide by contour length -> <= 100% (neighbors.cpp).

Refresh the one affected vetted golden in test_neighbors_2d.h (label 2 PERCENT_TOUCHING 33.33 -> 66.67; all closest-neighbor goldens unchanged) and add tests/python/test_feature_bugs.py::test_neighbor_distance_and_percent_touching.

@darkclad

darkclad commented Jul 1, 2026

Copy link
Copy Markdown
Author

Justification — golden value change for main-neighbors-closest-and-percent-touching

Branch: main-neighbors-closest-and-percent-touching (split from main-feature-validation, based on main)
Source fix: src/nyx/features/neighbors.cpp, src/nyx/reduce_trivial_rois.cpp
Test file touched: tests/test_neighbors_2d.h (1 golden: label 2 PERCENT_TOUCHING)

The bugs

  • fix variable type to address windows build issue #13 PERCENT_TOUCHING could exceed 100% (neighbors.cpp). The old code added a per-neighbor
    touch count, so a contour pixel adjacent to several neighbours was counted once per neighbour and
    the total could exceed the contour length (e.g. 127%). Fix: build a deduped per-ROI mask of
    contour pixels touching ANY neighbour (8-connected adjacency, sqdist ≤ 2) and divide the distinct
    touching-pixel count by the contour length → guaranteed ≤ 100%.
  • Better cache friendliness #12 CLOSEST_NEIGHBOR{1,2}_DIST/ANG were 0 in production (reduce_trivial_rois.cpp). Those
    features need CENTROID_X/Y, which are produced by Basic Morphology. When a user requested only
    neighbour features, Basic Morphology was not run, so the centroids stayed 0 and the closest-neighbour
    distances/angles came out 0. Fix: run Basic Morphology whenever neighbour features are requested.

Golden change (1 key)

Label Golden Old New Why
2 PERCENT_TOUCHING 33.333… 66.666… The deduped, adjacency-based touching fraction for this fixture ROI.

Only this one value changed. Every other neighbour golden (NUM_NEIGHBORS, all
CLOSEST_NEIGHBOR*_DIST/ANG, ANG_BW_NEIGHBORS_*, and the other labels' PERCENT_TOUCHING) is
unchanged — the C++ unit test calls Basic Morphology explicitly before the neighbour reduce, so
its centroids are always populated and bug #12 (a production-only reduce-ordering defect) does not
manifest in the C++ fixture. That the whole suite passes with only label 2's PERCENT_TOUCHING
updated confirms the closest-neighbour reimplementation is value-preserving on this fixture.

Why this is not fudging

  • Bounded invariant: the fix's purpose is PERCENT_TOUCHING <= 100%; label 2 stays well within
    bounds. The old summing scheme is the one that produced impossible >100% values (exercised by the
    Python test on touching squares).
  • Minimal blast radius: exactly one golden moved; all others are byte-identical to main.
  • Value is byte-identical to the oracle-validated main-feature-validation (copied, not invented);
    only the shared two-table→one-table refactor was dropped.

Regression guards (Python, production path)

tests/python/test_feature_bugs.py::test_neighbor_distance_and_percent_touching on two 4×4 squares
2 px apart asserts, through the production featurize() path:

CI

Full suite green on this branch: C++ runAllTests 683/683; pytest tests/python/ 49 passed, 1 skipped.

Demian Vladi added 2 commits July 2, 2026 08:37
…OUCHING

Two production defects:

- CLOSEST_NEIGHBOR{1,2}_DIST/ANG and ANG_BW_NEIGHBORS_* need CENTROID_X/Y from
  Basic Morphology. When only neighbor features were requested, Basic Morphology
  was skipped in the trivial-ROI reduce, so the centroids stayed 0 and the
  closest-neighbor distances/angles came out 0. Run Basic Morphology whenever
  neighbor features are requested (reduce_trivial_rois.cpp).
- PERCENT_TOUCHING summed a per-neighbor touch count, so a contour pixel adjacent
  to several neighbors was counted repeatedly and the total could exceed 100%
  (e.g. 127%). Build a deduped per-ROI mask of contour pixels touching ANY
  neighbor (8-connected adjacency) and divide by contour length -> <= 100%
  (neighbors.cpp).

Refresh the one affected vetted golden in test_neighbors_2d.h (label 2
PERCENT_TOUCHING 33.33 -> 66.67; all closest-neighbor goldens unchanged) and add
tests/python/test_feature_bugs.py::test_neighbor_distance_and_percent_touching.
@darkclad darkclad force-pushed the main-neighbors-closest-and-percent-touching branch from d2e3504 to 4b03dd2 Compare July 2, 2026 15:46
sameeul added a commit to sameeul/nyxus that referenced this pull request Jul 12, 2026
… not analytic-only

Correct an over-conservative claim: the neighbor family DOES have a tool oracle.
CellProfiler MeasureObjectNeighbors vets CLOSEST_NEIGHBOR*_DIST (1:1, centroid-
Euclidean) and NUM_NEIGHBORS / PERCENT_TOUCHING (with-caveat: disk-dilation vs
Euclidean contour distance, perimeter-set differences). A scipy.cKDTree
boundary-pixel recipe reproduces Nyxus's definitions exactly and is the primary
oracle for the angle features + a bit-exact backstop for the rest.

Only the neighbor ANGLE stats stay analytic-only: CellProfiler's
AngleBetweenNeighbors is a different quantity (interior subtended angle of the two
closest, 0-180) than Nyxus ANG_BW_NEIGHBORS_* (population stats of all-neighbor
bearings, 0-360).

CellProfiler config to match Nyxus PERCENT_TOUCHING: "Within a specified distance"
= radius-based touch (pre-PolusAI#359); "Adjacent" = 8-connected touch (post-PolusAI#359).

Updates the coverage-by-family table + adds a neighbor reconciliation subsection to
TOOLS.md, and adds MeasureObjectNeighbors to the SPEC.md tool catalog.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant