Skip to content

fix(morphology): geodetic length/thickness from real perimeter (was size_t-truncated) #392

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

Open
darkclad wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(morphology): geodetic length/thickness from real perimeter (was size_t-truncated) #392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4d650d
test(morphology): vet DIAMETER_EQUAL_PERIMETER + ORIENTATION + EROSIO…
Jul 23, 2026
31de05c
fix(morphology): geodetic length/thickness from real perimeter (was s…
Jul 23, 2026
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
13 changes: 8 additions & 5 deletions src/nyx/features/geo_len_thickness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,23 @@ GeodeticLengthThicknessFeature::GeodeticLengthThicknessFeature() : FeatureMethod

void GeodeticLengthThicknessFeature::calculate (LR& r, const Fsettings& s)
{
size_t roiArea = r.aux_area,
// Keep perimeter as a real number: assigning it into size_t truncated the fractional part and
// forced integer division below, so geodetic_length/thickness deviated from the rectangle-model
// formula (e.g. ~11% on the 8x8 shape2d fixture: geodetic 10.0 instead of ~11.13).
double roiArea = r.aux_area,
roiPerimeter = r.fvals[(int)Feature2D::PERIMETER][0];

double SqRootTmp = roiPerimeter * roiPerimeter / 16 - (double)roiArea;
double SqRootTmp = roiPerimeter * roiPerimeter / 16.0 - roiArea;

// Make sure value under SqRootTmp is always positive
if (SqRootTmp < 0)
if (SqRootTmp < 0)
SqRootTmp = 0;

// Calculate geodetic_length with pq-formula (see above):
geodetic_length = roiPerimeter / 4 + sqrt(SqRootTmp);
geodetic_length = roiPerimeter / 4.0 + sqrt(SqRootTmp);

// Calculate thickness by rewriting Equation (2):
thickness = roiPerimeter / 2 - geodetic_length;
thickness = roiPerimeter / 2.0 - geodetic_length;
}

void GeodeticLengthThicknessFeature::osized_calculate (LR& r, const Fsettings& s, ImageLoader&)
Expand Down
13 changes: 4 additions & 9 deletions tests/test_all.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,14 +1012,14 @@ TEST(TEST_NYXUS, TEST_SHAPE2D_CONTOUR_FEATURES)
ASSERT_NO_THROW(test_shape2d_contour_features());
}

TEST(TEST_NYXUS, TEST_SHAPE2D_VERIFIABLE_WITH_3P_BUILTIN_ORACLE_CONTOUR_DIAMETER_EQUAL_PERIMETER)
TEST(TEST_NYXUS, TEST_SHAPE2D_CONVEX_HULL_FEATURES)

@vjaganat90 vjaganat90 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker. If all the oracle supported tests are in tests/vetting then we should releax the naming of test groups and fixtures to original names, We should discuss this offline.

{
ASSERT_NO_THROW(test_shape2d_verifiable_with_3p_builtin_oracle_contour_diameter_equal_perimeter());
ASSERT_NO_THROW(test_shape2d_convex_hull_features());
}

TEST(TEST_NYXUS, TEST_SHAPE2D_CONVEX_HULL_FEATURES)
TEST(TEST_NYXUS, TEST_SHAPE2D_SKIMAGE_ORIENTATION_AND_EROSIONS)
{
ASSERT_NO_THROW(test_shape2d_convex_hull_features());
ASSERT_NO_THROW(test_shape2d_skimage_orientation_and_erosions());
}

TEST(TEST_NYXUS, TEST_SHAPE2D_VERIFIABLE_WITH_3P_BUILTIN_ORACLE_EXTREMA_FEATURES)
Expand Down Expand Up @@ -1072,11 +1072,6 @@ TEST(TEST_NYXUS, TEST_2D_INTENSITY_GEOMETRIC_MOMENTS_UNVETTED_NO_DIRECT_ORACLE)
ASSERT_NO_THROW(test_2d_intensity_geometric_moments_unvetted_no_direct_oracle());
}

TEST(TEST_NYXUS, TEST_SHAPE2D_VERIFIABLE_WITH_3P_BUILTIN_ORACLE_GEODETIC_THICKNESS_EROSION)
{
ASSERT_NO_THROW(test_shape2d_verifiable_with_3p_builtin_oracle_geodetic_thickness_erosion_features());
}

TEST(TEST_NYXUS, TEST_REMAINING2D_VERIFIABLE_WITH_3P_BUILTIN_ORACLE_EROSION_COMPLEMENT)
{
ASSERT_NO_THROW(test_remaining2d_verifiable_with_3p_builtin_oracle_erosion_complement_feature());
Expand Down
7 changes: 5 additions & 2 deletions tests/test_morphology_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,11 @@ static std::unordered_map<std::string, double> oracle_3p_shape2d_feature_golden_
{"DIAMETER_EQUAL_PERIMETER", 8.57365809435587},
{"DIAMETER_CIRCUMSCRIBING_CIRCLE", 12.3317073399088},
{"DIAMETER_INSCRIBING_CIRCLE", 0.828486893405308},
{"GEODETIC_LENGTH", 10.0},
{"THICKNESS", 3.0},
// Real-valued rectangle-model roots after the geo_len_thickness.cpp perimeter-truncation fix
// (were the integer-truncated 10.0 / 3.0). Now vetted by analytic conformance in

@vjaganat90 vjaganat90 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GREAT! I have always suspected these

// test_shape2d_documented_formula_conformance_no_external_oracle.
{"GEODETIC_LENGTH", 11.13182483477333},
{"THICKNESS", 2.3356458070362205},
{"EROSIONS_2_VANISH", 1.0},
// EXTREMA P1..P8 (X,Y) match MATLAB/Octave regionprops('Extrema') EXACTLY under the documented
// coordinate convention: MATLAB returns 1-based sub-pixel *corner* coords, Nyxus returns 0-based
Expand Down
42 changes: 26 additions & 16 deletions tests/test_morphology_regression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ void test_shape2d_ellipse_features()
assert_unvetted_no_direct_oracle_shape2d_feature(fvals, Nyxus::Feature2D::MINOR_AXIS_LENGTH, "MINOR_AXIS_LENGTH");
assert_unvetted_no_direct_oracle_shape2d_feature(fvals, Nyxus::Feature2D::ELONGATION, "ELONGATION");
assert_unvetted_no_direct_oracle_shape2d_feature(fvals, Nyxus::Feature2D::ECCENTRICITY, "ECCENTRICITY");
assert_unvetted_no_direct_oracle_shape2d_feature(fvals, Nyxus::Feature2D::ORIENTATION, "ORIENTATION");
// ORIENTATION is vetted vs scikit-image in test_morphology_skimage.h; ROUNDNESS is vetted by
// documented-formula conformance in test_shape2d_documented_formula_conformance_no_external_oracle.
assert_unvetted_no_direct_oracle_shape2d_feature(fvals, Nyxus::Feature2D::ROUNDNESS, "ROUNDNESS");
}

Expand Down Expand Up @@ -85,6 +86,25 @@ void test_shape2d_documented_formula_conformance_no_external_oracle()
const double round_formula = 4.0 * A / (PI * major * major);
ASSERT_NEAR(fvals[static_cast<int>(Nyxus::Feature2D::ROUNDNESS)][0], round_formula, 1e-9)
<< "ROUNDNESS does not match 4A/(pi*major^2)";

// DIAMETER_EQUAL_PERIMETER = P / pi (contour.cpp) -- diameter of the circle with the same
// perimeter. Pure double arithmetic on the vetted PERIMETER, so it conforms exactly.
const double dep_formula = P / PI;
ASSERT_NEAR(fvals[static_cast<int>(Nyxus::Feature2D::DIAMETER_EQUAL_PERIMETER)][0], dep_formula, 1e-9)
<< "DIAMETER_EQUAL_PERIMETER does not match PERIMETER/pi";

// GEODETIC_LENGTH / THICKNESS: the two side lengths of the rectangle with the same area and
// perimeter (geo_len_thickness.cpp), i.e. the roots of x^2 - (P/2)x + A = 0. After the perimeter
// size_t-truncation fix these are real-valued, so they conform to the documented formula exactly.
double disc = P * P / 16.0 - A;
if (disc < 0.0)
disc = 0.0;
const double geodetic_formula = P / 4.0 + std::sqrt(disc);
const double thickness_formula = P / 2.0 - geodetic_formula;
ASSERT_NEAR(fvals[static_cast<int>(Nyxus::Feature2D::GEODETIC_LENGTH)][0], geodetic_formula, 1e-9)
<< "GEODETIC_LENGTH does not match P/4 + sqrt(P^2/16 - A)";
ASSERT_NEAR(fvals[static_cast<int>(Nyxus::Feature2D::THICKNESS)][0], thickness_formula, 1e-9)
<< "THICKNESS does not match P/2 - GEODETIC_LENGTH";
}

void test_shape2d_unvetted_no_direct_oracle_radius_features()
Expand All @@ -97,13 +117,8 @@ void test_shape2d_unvetted_no_direct_oracle_radius_features()
assert_unvetted_no_direct_oracle_shape2d_feature(fvals, Nyxus::Feature2D::ROI_RADIUS_MEDIAN, "ROI_RADIUS_MEDIAN");
}

void test_shape2d_verifiable_with_3p_builtin_oracle_contour_diameter_equal_perimeter()
{
std::vector<std::vector<double>> fvals;
calculate_shape2d_feature_values(fvals);

assert_verifiable_with_3p_builtin_oracle_shape2d_feature(fvals, Nyxus::Feature2D::DIAMETER_EQUAL_PERIMETER, "DIAMETER_EQUAL_PERIMETER");
}
// DIAMETER_EQUAL_PERIMETER is now vetted by documented-formula conformance (P/pi) in
// test_shape2d_documented_formula_conformance_no_external_oracle.

void test_shape2d_verifiable_with_3p_builtin_oracle_fractal_circle_features()
{
Expand All @@ -116,14 +131,9 @@ void test_shape2d_verifiable_with_3p_builtin_oracle_fractal_circle_features()
assert_verifiable_with_3p_builtin_oracle_shape2d_feature(fvals, Nyxus::Feature2D::DIAMETER_INSCRIBING_CIRCLE, "DIAMETER_INSCRIBING_CIRCLE");
}

void test_shape2d_verifiable_with_3p_builtin_oracle_geodetic_thickness_erosion_features()
{
std::vector<std::vector<double>> fvals;
calculate_shape2d_feature_values(fvals);
assert_verifiable_with_3p_builtin_oracle_shape2d_feature(fvals, Nyxus::Feature2D::GEODETIC_LENGTH, "GEODETIC_LENGTH");
assert_verifiable_with_3p_builtin_oracle_shape2d_feature(fvals, Nyxus::Feature2D::THICKNESS, "THICKNESS");
assert_verifiable_with_3p_builtin_oracle_shape2d_feature(fvals, Nyxus::Feature2D::EROSIONS_2_VANISH, "EROSIONS_2_VANISH");
}
// GEODETIC_LENGTH + THICKNESS are now vetted by documented-formula conformance (rectangle roots) in
// test_shape2d_documented_formula_conformance_no_external_oracle, after the geo_len_thickness.cpp
// perimeter-truncation fix. EROSIONS_2_VANISH is vetted vs scikit-image in test_morphology_skimage.h.

// ---------------------------------------------------------------------------------------------------
// Migrated from test_2d_remaining_features.h (Wave 6): erosion-complement, caliper (feret/martin/
Expand Down
19 changes: 19 additions & 0 deletions tests/test_morphology_skimage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,22 @@ void test_shape2d_convex_hull_features()
assert_verifiable_with_3p_builtin_oracle_shape2d_feature(fvals, Nyxus::Feature2D::CONVEX_HULL_AREA, "CONVEX_HULL_AREA", 100.0);
assert_verifiable_with_3p_builtin_oracle_shape2d_feature(fvals, Nyxus::Feature2D::SOLIDITY, "SOLIDITY", 100.0);
}

// ORIENTATION and EROSIONS_2_VANISH vetted vs scikit-image (tests/vetting/oracles/gen_morphology_skimage.py).
// ORIENTATION: skimage regionprops orientation is measured from the row axis; Nyxus measures the same
// ellipse's major axis from the x axis, so NYXUS == 90 - degrees(skimage.orientation) = 70.4173944984
// (matches to 10 decimals -- the angle is invariant to the pixel-size second-moment correction that makes
// the AXIS LENGTHS differ ~1.4%, which is why MAJOR/MINOR/ECCENTRICITY stay regression).
// EROSIONS_2_VANISH: Nyxus' 3x3 (8-connected) structuring element == skimage square(3); the count (1)
// matches, and disk(1)/4-connected gives 2, so the test also pins the connectivity convention.
void test_shape2d_skimage_orientation_and_erosions()
{
std::vector<std::vector<double>> fvals;
calculate_shape2d_feature_values(fvals);

// skimage-derived goldens (90 - degrees(regionprops.orientation); square(3) erosion count)
ASSERT_NEAR(fvals[static_cast<int>(Nyxus::Feature2D::ORIENTATION)][0], 70.4173944984207, 1e-3)
<< "ORIENTATION does not match 90 - skimage.orientation(deg)";
ASSERT_NEAR(fvals[static_cast<int>(Nyxus::Feature2D::EROSIONS_2_VANISH)][0], 1.0, 1e-9)
<< "EROSIONS_2_VANISH does not match skimage square(3) erosion count";
}
6 changes: 3 additions & 3 deletions tests/vetting/coverage_report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

_Generated by check_coverage.py from oracle_coverage.csv. Do not edit by hand._

Features vetted by >=1 oracle: 604/758 (80%)
regression: 154 untested: 0
Features vetted by >=1 oracle: 608/758 (80%)
regression: 150 untested: 0

| family | total | vetted | regression | untested |
|---|---|---|---|---|
Expand All @@ -17,7 +17,7 @@ regression: 154 untested: 0
| imq | 6 | 0 | 6 | 0 |
| intensity_histogram | 47 | 47 | 0 | 0 |
| moments | 180 | 118 | 62 | 0 |
| morphology | 113 | 77 | 36 | 0 |
| morphology | 113 | 81 | 32 | 0 |
| neighbor | 9 | 2 | 7 | 0 |
| ngldm | 38 | 20 | 18 | 0 |
| ngtdm | 10 | 10 | 0 | 0 |
Expand Down
Loading
Loading