Skip to content

test(3d glrlm): assert the last two _AVE features, and fix the tolerances hiding an exact match - #435

Open
darkclad wants to merge 1 commit into
PolusAI:mainfrom
darkclad:main-vet-3d-glrlm
Open

test(3d glrlm): assert the last two _AVE features, and fix the tolerances hiding an exact match#435
darkclad wants to merge 1 commit into
PolusAI:mainfrom
darkclad:main-vet-3d-glrlm

Conversation

@darkclad

Copy link
Copy Markdown

Closes the two 3GLRLM_*_AVE rows that read status=vetted with no in-tree oracle assertion, corrects the fourteen whose registry entry never recorded the assertion already covering them, and wires in 16 assertions that existed in the tree but were unreachable.

The gap was two rows, not sixteen

test_3d_glrlm_ave_pyradiomics() already asserted 14 of the 16 _AVE features against the PyRadiomics goldens and passed. The registry never recorded it: all sixteen _AVE rows read current_test = test_3d_glrlm_coverage.h;test_nyxus.py, so the report counted the whole family as vetted-with-no-oracle-test.

3GLRLM_RP_AVE and 3GLRLM_RE_AVE were genuinely unasserted in C++, omitted from that function's table under a comment claiming they were "already vetted elsewhere". They are now in it. Both also read oracle=mirp while their base rows read oracle=pyradiomics for the same measured quantity, with no notes on either - the same incoherence 3D GLCM carried. No mirp run has ever existed for 3D GLRLM in this tree: no generator, no goldens, no test file. Both now name the tool the assertion compares against.

test_3d_glrlm_ave_pyradiomics() now fails if any 3GLRLM_* feature the build exposes is missing from the golden table or from its own _AVE list, so a feature added later cannot go unasserted while the test still passes over the entries it has.

The goldens reproduce bit-for-bit

The family had no generator - gen_glrlm_{mirp,pyradiomics}.py are 2D-only - so SPEC 6.4's "generator script path" was unmet and the sixteen numbers rested on a CLI recipe written into a comment. That comment also contradicted itself, naming the ut_ phantom at 100 grey levels with an "offset 1, asymmetric cooc matrix" (GLCM language) while every assertion uses the compat phantom at binCount 20.

gen_glrlm3d_pyradiomics.py now runs PyRadiomics 3.0.1 against the compat phantom and re-verifies every pin: all 16 reproduce at rel=0. It also checks the oracle's own output against the five bounds the family carries by construction, so a misconfigured run is caught before anything is pinned.

The 10% band was eight orders of magnitude too loose ---------------------------------------------------- Once the _AVE slots were being read, the residuals are measurable: 15 of 16 features land within 2.9e-16 of PyRadiomics (six of them exactly 0), and 3GLRLM_RE within 3.9e-4. Every assertion in the file was banded at 10%.

Unlike 3D GLCM, whose asymmetric offset-1 cooc matrix genuinely differs from PyRadiomics' symmetric default, GLRLM has no convention gap to absorb - it lands on the tool exactly. A 10% band there would pass a value wrong by a factor of 1.1. Both the per-angle and the _AVE assertions now hold rel=1e-9, with 3GLRLM_RE at rel=5e-3: it is the family's only sum over logarithms, evaluated through fast_log10 with an EPSILON guard, and the 2D family carries the same exception for the same reason.

The pytest assertions had a worse form of it: np.isclose(rtol=1e-1, atol=1e-2) on values as small as 0.0115 makes the effective band +-96%. Tightened to match; they pass at rel=1e-9 too, so the Python path reproduces the tool to double precision as well.

16 assertions that existed but never executed

test_3d_glrlm_regression.h was not #included and had zero TEST() registrations, for the same mechanical reason as its GLCM sibling: it carried its own definition of get_3d_segmented_phantom(), which redefines the one in test_3d_glcm_pyradiomics.h inside the single test_all.cc translation unit. It now forward-declares it, and its live body replaces the dead #if 0 one it also carried.

Wired in, 11 of the 16 failed, on two separate defects:

The settings were incomplete. The file set the generic GREYDEPTH only, but D3_GLRLM_feature bins on GLRLM_GREYDEPTH. Left unset that defaults to 0 - no binning at all - so the features ran on raw intensities and HGLRE came out at 4.3e6 against the ~4e3 ceiling 64 grey levels imply.

The pins were unrecoverable. No GLRLM_GREYDEPTH reproduces them; the best case is 5 of 16 within 10%, with residuals over 240%, and the file's own "Calculated at 100 grey levels" does not match either. Regenerated from the current implementation - which the sibling PyRadiomics file, which does run, vets independently - at GLRLM_GREYDEPTH=-20, tolerance tightened 10% -> rel=1e-9, and test_3d_glrlm_dump_regression() regenerates the table so the next refresh is a gtest filter rather than a hand transcription.

The helper also aggregated wrongly: it read r.fvals[fcode][0] under the comment "we have just 1 value, no need to aggregate subfeatures", but save_value writes the full per-angle vector to that slot, so the guard pinned one direction and let the other twelve drift unwatched. It now uses calc_ave over the 13 angles, matching the GLCM regression file.

3GLRLM_RP leaves its mathematical bound

Run Percentage is runs / voxels and cannot exceed 1. On the ut_ phantom, averaged over the 13 angles, it reaches 1.007 at GLRLM_GREYDEPTH=128 and 1.045 at 256, growing monotonically with the bin count (and crossing 1 at 64 for a single angle). Every other bounded feature in the family stays in range across the whole sweep, so it is specific to RP and points at a denominator that does not track the binning.

Not fixed here - it is a behaviour change in a public feature and belongs on its own branch, together with a cheap invariant test (RP <= 1, SRE <= 1, LRE >= 1), which is the check that found it. Both benchmarks in this PR use binCount binning, where RP is in range, and the 3GLRLM_RP_AVE registry row says so rather than claiming the feature holds at every config.

Include hygiene

test_3d_glrlm_pyradiomics.h included src/nyx/raw_nifti.h and referenced no symbol from it; removed. Both headers relied transitively on // (and the regression one on , and helpers/fsystem.h for fs::exists); those are now direct. test_3d_glrlm_coverage.h keeps its single include of test_3d_coverage_common.h - all eight 3D _coverage.h files share that form and SPEC 6.3.1 sanctions a _common.h for fixture scaffolding.

Registry

All 32 rows get a real config_recipe id (two new: glrlm3d.pyradiomics_bincount20 and glrlm3d.regression_ut_phantom, same binning on different fixtures), a real tolerance, target_test cleared, source moved off tracker, and current_test synced to the files that actually cover each feature. The 16 base rows previously carried a note calling test_3d_glrlm_regression.h orphaned, which is no longer true.

Noted for the remaining 3D families, measured on this commit rather than assumed: test_3d_{gldm,glszm,ngtdm}_regression.h (14, 16 and 5 test functions) and test_3d_firstorder_matlab.h (35) are still never executed - not #included, no TEST() registrations. But unlike the GLCM file they do NOT define get_3d_segmented_phantom() (all forward-declare it), and their assert bodies are NOT dead (a disabled old body in #if 0 followed by a live one, the same shape this file had). Wiring them in is an #include plus registrations, then fixing what the first run reports.

test_3d_ngldm_regression.h is not the "19 tests that assert nothing" case an earlier note in this series described: it is included, all 19 are registered, and its live agrees_gt does fire - verified by negative control (perturbing 3NGLDM_DCENE 0.14 -> 0.99 makes that test fail at the assertion). Its real open item is that its pins disagree with MIRP by up to an order of magnitude, which is that family's business.

Verified: runAllTests 790/790 on Windows and under ASan+UBSan on Linux; pytest tests/python/ full directory (92 passed, 1 skipped); check_coverage.py --check, check_test_names.py --check and audit/scan_glrlm3d_coverage.py --check all clean; gen_glrlm3d_pyradiomics.py re-verifies all 16 goldens against a fresh PyRadiomics 3.0.1 run.

…nces hiding an exact match

Closes the two 3GLRLM_*_AVE rows that read status=vetted with no in-tree oracle assertion, corrects
the fourteen whose registry entry never recorded the assertion already covering them, and wires in 16
assertions that existed in the tree but were unreachable.

The gap was two rows, not sixteen
---------------------------------
test_3d_glrlm_ave_pyradiomics() already asserted 14 of the 16 _AVE features against the PyRadiomics
goldens and passed. The registry never recorded it: all sixteen _AVE rows read
current_test = test_3d_glrlm_coverage.h;test_nyxus.py, so the report counted the whole family as
vetted-with-no-oracle-test.

3GLRLM_RP_AVE and 3GLRLM_RE_AVE were genuinely unasserted in C++, omitted from that function's table
under a comment claiming they were "already vetted elsewhere". They are now in it. Both also read
oracle=mirp while their base rows read oracle=pyradiomics for the same measured quantity, with no
notes on either - the same incoherence 3D GLCM carried. No mirp run has ever existed for 3D GLRLM in
this tree: no generator, no goldens, no test file. Both now name the tool the assertion compares
against.

test_3d_glrlm_ave_pyradiomics() now fails if any 3GLRLM_* feature the build exposes is missing from
the golden table or from its own _AVE list, so a feature added later cannot go unasserted while the
test still passes over the entries it has.

The goldens reproduce bit-for-bit
---------------------------------
The family had no generator - gen_glrlm_{mirp,pyradiomics}.py are 2D-only - so SPEC 6.4's "generator
script path" was unmet and the sixteen numbers rested on a CLI recipe written into a comment. That
comment also contradicted itself, naming the ut_ phantom at 100 grey levels with an "offset 1,
asymmetric cooc matrix" (GLCM language) while every assertion uses the compat phantom at binCount 20.

gen_glrlm3d_pyradiomics.py now runs PyRadiomics 3.0.1 against the compat phantom and re-verifies every
pin: all 16 reproduce at rel=0. It also checks the oracle's own output against the five bounds the
family carries by construction, so a misconfigured run is caught before anything is pinned.

The 10% band was eight orders of magnitude too loose
----------------------------------------------------
Once the _AVE slots were being read, the residuals are measurable: 15 of 16 features land within
2.9e-16 of PyRadiomics (six of them exactly 0), and 3GLRLM_RE within 3.9e-4. Every assertion in the
file was banded at 10%.

Unlike 3D GLCM, whose asymmetric offset-1 cooc matrix genuinely differs from PyRadiomics' symmetric
default, GLRLM has no convention gap to absorb - it lands on the tool exactly. A 10% band there would
pass a value wrong by a factor of 1.1. Both the per-angle and the _AVE assertions now hold rel=1e-9,
with 3GLRLM_RE at rel=5e-3: it is the family's only sum over logarithms, evaluated through fast_log10
with an EPSILON guard, and the 2D family carries the same exception for the same reason.

The pytest assertions had a worse form of it: np.isclose(rtol=1e-1, atol=1e-2) on values as small as
0.0115 makes the effective band +-96%. Tightened to match; they pass at rel=1e-9 too, so the Python
path reproduces the tool to double precision as well.

16 assertions that existed but never executed
---------------------------------------------
test_3d_glrlm_regression.h was not #included and had zero TEST() registrations, for the same
mechanical reason as its GLCM sibling: it carried its own definition of get_3d_segmented_phantom(),
which redefines the one in test_3d_glcm_pyradiomics.h inside the single test_all.cc translation unit.
It now forward-declares it, and its live body replaces the dead #if 0 one it also carried.

Wired in, 11 of the 16 failed, on two separate defects:

The settings were incomplete. The file set the generic GREYDEPTH only, but D3_GLRLM_feature bins on
GLRLM_GREYDEPTH. Left unset that defaults to 0 - no binning at all - so the features ran on raw
intensities and HGLRE came out at 4.3e6 against the ~4e3 ceiling 64 grey levels imply.

The pins were unrecoverable. No GLRLM_GREYDEPTH reproduces them; the best case is 5 of 16 within 10%,
with residuals over 240%, and the file's own "Calculated at 100 grey levels" does not match either.
Regenerated from the current implementation - which the sibling PyRadiomics file, which does run,
vets independently - at GLRLM_GREYDEPTH=-20, tolerance tightened 10% -> rel=1e-9, and
test_3d_glrlm_dump_regression() regenerates the table so the next refresh is a gtest filter rather
than a hand transcription.

The helper also aggregated wrongly: it read r.fvals[fcode][0] under the comment "we have just 1
value, no need to aggregate subfeatures", but save_value writes the full per-angle vector to that
slot, so the guard pinned one direction and let the other twelve drift unwatched. It now uses
calc_ave over the 13 angles, matching the GLCM regression file.

3GLRLM_RP leaves its mathematical bound
---------------------------------------
Run Percentage is runs / voxels and cannot exceed 1. On the ut_ phantom, averaged over the 13 angles,
it reaches 1.007 at GLRLM_GREYDEPTH=128 and 1.045 at 256, growing monotonically with the bin count
(and crossing 1 at 64 for a single angle). Every other bounded feature in the family stays in range
across the whole sweep, so it is specific to RP and points at a denominator that does not track the
binning.

Not fixed here - it is a behaviour change in a public feature and belongs on its own branch, together
with a cheap invariant test (RP <= 1, SRE <= 1, LRE >= 1), which is the check that found it. Both
benchmarks in this PR use binCount binning, where RP is in range, and the 3GLRLM_RP_AVE registry row
says so rather than claiming the feature holds at every config.

Include hygiene
---------------
test_3d_glrlm_pyradiomics.h included src/nyx/raw_nifti.h and referenced no symbol from it; removed.
Both headers relied transitively on <string>/<tuple>/<vector> (and the regression one on <iomanip>,
<iostream> and helpers/fsystem.h for fs::exists); those are now direct. test_3d_glrlm_coverage.h
keeps its single include of test_3d_coverage_common.h - all eight 3D _coverage.h files share that
form and SPEC 6.3.1 sanctions a _common.h for fixture scaffolding.

Registry
--------
All 32 rows get a real config_recipe id (two new: glrlm3d.pyradiomics_bincount20 and
glrlm3d.regression_ut_phantom, same binning on different fixtures), a real tolerance, target_test
cleared, source moved off tracker, and current_test synced to the files that actually cover each
feature. The 16 base rows previously carried a note calling test_3d_glrlm_regression.h orphaned,
which is no longer true.

Noted for the remaining 3D families, measured on this commit rather than assumed:
test_3d_{gldm,glszm,ngtdm}_regression.h (14, 16 and 5 test functions) and test_3d_firstorder_matlab.h
(35) are still never executed - not #included, no TEST() registrations. But unlike the GLCM file they
do NOT define get_3d_segmented_phantom() (all forward-declare it), and their assert bodies are NOT
dead (a disabled old body in #if 0 followed by a live one, the same shape this file had). Wiring them
in is an #include plus registrations, then fixing what the first run reports.

test_3d_ngldm_regression.h is not the "19 tests that assert nothing" case an earlier note in this
series described: it is included, all 19 are registered, and its live agrees_gt does fire - verified
by negative control (perturbing 3NGLDM_DCENE 0.14 -> 0.99 makes that test fail at the assertion). Its
real open item is that its pins disagree with MIRP by up to an order of magnitude, which is that
family's business.

Verified: runAllTests 790/790 on Windows and under ASan+UBSan on Linux; pytest tests/python/ full
directory (92 passed, 1 skipped); check_coverage.py --check, check_test_names.py --check and
audit/scan_glrlm3d_coverage.py --check all clean; gen_glrlm3d_pyradiomics.py re-verifies all 16
goldens against a fresh PyRadiomics 3.0.1 run.
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