test(3d ngldm): demote the family's one vetted row, and pin its wrong values honestly - #436
Open
darkclad wants to merge 1 commit into
Open
test(3d ngldm): demote the family's one vetted row, and pin its wrong values honestly#436darkclad wants to merge 1 commit into
darkclad wants to merge 1 commit into
Conversation
… values honestly
The last family in the per-family series, and the only one that closes its gap row by demoting it.
3NGLDM_DCP read status=vetted, oracle=mirp from an offline harness run with target_test naming
test_3d_ngldm_mirp.h, a file that does not exist. Running that oracle shows the claim should never
have been made - and shows why the other 18 rows are right to be regression.
The oracle run
--------------
gen_ngldm3d_mirp.py runs MIRP 2.6.0 on the segmented phantom at a config-matched recipe:
fixed_bin_number n=64 against Nyxus' GREYDEPTH=64, distance 1, alpha 0, by_slice=False.
D3_NGLDM_feature bins on STNGS_NGREYS, the generic grey depth, so there is no binning mismatch to
explain the results. MIRP disagrees on 16 of the 17 features it can compute:
GLNU 115443 vs 4350.27 26.5x
DCENE 0.143484 vs 0.00287482 49.9x
GLNUN 0.225757 vs 0.0158519 14.2x
HDE 261.018 vs 28.0738 9.3x
DCV 86.1706 vs 11.9476 7.21x
HDLGLE 0.0252015 vs 8.71408 0.00289x
LGLCE 0.000359684 vs 0.0321849 0.0112x
... full table in the audit report
3NGLDM_GLM and 3NGLDM_DCM have no MIRP counterpart at all (no gl_mean / dc_mean column).
Two causes, both in the source
------------------------------
calc_ngld_matrix iterates the ROI *bounding box* and explicitly declines to skip background
("// Do not skip off-ROI pixels", with the guard commented out). On this phantom that is 551,040
voxels against an ROI of 274,432 - a factor of 2.008. IBSI NGLDM is defined over ROI voxels only.
This predicts the error pattern rather than merely correlating with it: background voxels bin alike
and each sees ~24 identical neighbours, so they pile into one grey row at maximum dependence, which
inflates a sum-of-squares over grey rows superlinearly (GLNU), raises energy (DCENE), lowers entropy
(DCENT) and pushes the dependence distribution to its top end (HDE up, LDE down). DCNU, which scales
closer to linearly with voxel count, sits at 2.09x - the bounding-box ratio.
Second, the shifts table holds 24 entries where a 3D Chebyshev-1 neighbourhood has 26: the two
pure-axial neighbours (0,0,+1) and (0,0,-1), directly above and below the centre, are absent. nsh is
therefore 24, and maxNr = nsh + 1 carries a comment reading "max dependence 8 (due to 8 neighbors)",
which is the 2D count.
Neither is fixed here - both change 19 public feature values and belong on their own branch.
Why DCP is demoted rather than promoted
---------------------------------------
It is the family's only agreement and it agrees at 1.0 on both sides. Dependence-count percentage is
the fraction of voxels with at least one dependency; it is exactly 1 on this phantom under both the
correct and the incorrect neighbourhood, with or without background voxels. It is an assertion that
cannot fail for the reasons we would want it to fail. Promoting it would leave the registry saying
3D NGLDM had been checked, on the strength of a degenerate constant standing beside sixteen features
that miss the same tool by up to 50x. The row now reads status=regression, candidate_oracle=mirp,
flag=implementation-defect.
That leaves the family with zero vetted rows, the only one in the series. It was zero before as well;
the difference is that the registry now says so.
The pins are wrong on purpose
-----------------------------
All 19 goldens are regenerated at full precision and the band tightened from 10% to rel=1e-9. The old
pins were two- and three-significant-figure numbers (0.1, 261, 740, 0.00036) inside a +-10% band,
which cannot detect the very fix these values are waiting for. A drift guard's job is to notice
change, and these will change; test_3d_ngldm_dump_regression() regenerates them when the
implementation is corrected, at which point the MIRP comparison should be re-run to promote the
family properly.
Test file cleanup
-----------------
test_3d_ngldm_regression.h carried a dead #if 0 block (a superseded copy of the assert body) and
relied transitively on <iomanip>, <iostream>, <string>, <tuple>, <vector> and helpers/fsystem.h for
fs::exists; all now direct. Its golden lookup used operator[], which default-inserts a missing key as
0 and then compares against that fabricated reference - now guarded with find(). The 2026-07 MIRP
comparison that lived inline in the header moves to the audit report, at full precision and with
causes attached.
For the record, since an earlier note in this series said otherwise: the 19 tests in this file DO
assert. Verified by negative control - perturbing 3NGLDM_DCENE from 0.14 to 0.99 makes
TEST_3D_NGLDM_DCENE_REGRESSION fail at the assertion.
Registry
--------
All 19 rows get a config_recipe id (two new: ngldm3d.regression_ut_phantom and ngldm3d.mirp_fbn64 -
the second is referenced but not asserted against, so the divergence stays reproducible and the
promotion can be re-run), a real tolerance, target_test cleared, source moved off tracker, and
current_test synced to the files that cover each feature.
Verified: runAllTests 774/774 on Windows and under ASan+UBSan on Linux; pytest tests/python/ full
directory; check_coverage.py --check, check_test_names.py --check and
audit/scan_ngldm3d_coverage.py --check all clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The last family in the per-family series, and the only one that closes its gap row by demoting it. 3NGLDM_DCP read status=vetted, oracle=mirp from an offline harness run with target_test naming test_3d_ngldm_mirp.h, a file that does not exist. Running that oracle shows the claim should never have been made - and shows why the other 18 rows are right to be regression.
The oracle run
gen_ngldm3d_mirp.py runs MIRP 2.6.0 on the segmented phantom at a config-matched recipe: fixed_bin_number n=64 against Nyxus' GREYDEPTH=64, distance 1, alpha 0, by_slice=False. D3_NGLDM_feature bins on STNGS_NGREYS, the generic grey depth, so there is no binning mismatch to explain the results. MIRP disagrees on 16 of the 17 features it can compute:
GLNU 115443 vs 4350.27 26.5x
DCENE 0.143484 vs 0.00287482 49.9x
GLNUN 0.225757 vs 0.0158519 14.2x
HDE 261.018 vs 28.0738 9.3x
DCV 86.1706 vs 11.9476 7.21x
HDLGLE 0.0252015 vs 8.71408 0.00289x
LGLCE 0.000359684 vs 0.0321849 0.0112x
... full table in the audit report
3NGLDM_GLM and 3NGLDM_DCM have no MIRP counterpart at all (no gl_mean / dc_mean column).
Two causes, both in the source
calc_ngld_matrix iterates the ROI bounding box and explicitly declines to skip background ("// Do not skip off-ROI pixels", with the guard commented out). On this phantom that is 551,040 voxels against an ROI of 274,432 - a factor of 2.008. IBSI NGLDM is defined over ROI voxels only. This predicts the error pattern rather than merely correlating with it: background voxels bin alike and each sees ~24 identical neighbours, so they pile into one grey row at maximum dependence, which inflates a sum-of-squares over grey rows superlinearly (GLNU), raises energy (DCENE), lowers entropy (DCENT) and pushes the dependence distribution to its top end (HDE up, LDE down). DCNU, which scales closer to linearly with voxel count, sits at 2.09x - the bounding-box ratio.
Second, the shifts table holds 24 entries where a 3D Chebyshev-1 neighbourhood has 26: the two pure-axial neighbours (0,0,+1) and (0,0,-1), directly above and below the centre, are absent. nsh is therefore 24, and maxNr = nsh + 1 carries a comment reading "max dependence 8 (due to 8 neighbors)", which is the 2D count.
Neither is fixed here - both change 19 public feature values and belong on their own branch.
Why DCP is demoted rather than promoted
It is the family's only agreement and it agrees at 1.0 on both sides. Dependence-count percentage is the fraction of voxels with at least one dependency; it is exactly 1 on this phantom under both the correct and the incorrect neighbourhood, with or without background voxels. It is an assertion that cannot fail for the reasons we would want it to fail. Promoting it would leave the registry saying 3D NGLDM had been checked, on the strength of a degenerate constant standing beside sixteen features that miss the same tool by up to 50x. The row now reads status=regression, candidate_oracle=mirp, flag=implementation-defect.
That leaves the family with zero vetted rows, the only one in the series. It was zero before as well; the difference is that the registry now says so.
The pins are wrong on purpose
All 19 goldens are regenerated at full precision and the band tightened from 10% to rel=1e-9. The old pins were two- and three-significant-figure numbers (0.1, 261, 740, 0.00036) inside a +-10% band, which cannot detect the very fix these values are waiting for. A drift guard's job is to notice change, and these will change; test_3d_ngldm_dump_regression() regenerates them when the implementation is corrected, at which point the MIRP comparison should be re-run to promote the family properly.
Test file cleanup
test_3d_ngldm_regression.h carried a dead #if 0 block (a superseded copy of the assert body) and relied transitively on , , , , and helpers/fsystem.h for fs::exists; all now direct. Its golden lookup used operator[], which default-inserts a missing key as 0 and then compares against that fabricated reference - now guarded with find(). The 2026-07 MIRP comparison that lived inline in the header moves to the audit report, at full precision and with causes attached.
For the record, since an earlier note in this series said otherwise: the 19 tests in this file DO assert. Verified by negative control - perturbing 3NGLDM_DCENE from 0.14 to 0.99 makes TEST_3D_NGLDM_DCENE_REGRESSION fail at the assertion.
Registry
All 19 rows get a config_recipe id (two new: ngldm3d.regression_ut_phantom and ngldm3d.mirp_fbn64 - the second is referenced but not asserted against, so the divergence stays reproducible and the promotion can be re-run), a real tolerance, target_test cleared, source moved off tracker, and current_test synced to the files that cover each feature.
Verified: runAllTests 774/774 on Windows and under ASan+UBSan on Linux; pytest tests/python/ full directory; check_coverage.py --check, check_test_names.py --check and audit/scan_ngldm3d_coverage.py --check all clean.