fix(intensity): report first-order features in the slide's own intensity domain - #464
Open
darkclad wants to merge 1 commit into
Open
fix(intensity): report first-order features in the slide's own intensity domain#464darkclad wants to merge 1 commit into
darkclad wants to merge 1 commit into
Conversation
…ity domain A CT NIfTI read with no flag set came back in an arbitrary offset domain rather than in Hounsfield units. NiftiLoader::unhounsfield read a minimum off the buffer it was loading and shifted the whole volume by -min whenever that minimum was negative -- which every CT's is. Nothing recorded the shift and nothing undid it, so MEAN, MIN, MAX, MEDIAN, the percentiles and ENERGY were displaced by an image-dependent amount. --preserve-hu shifted by a different base (the within-mask minimum), so the two settings disagreed with each other as well as with the truth, and only the intensity-histogram family inverted anything. Reported against 0.11.0.post1.dev76 on clinical CT NIfTI data, reproduced locally, and re-verified on that same image and mask: with no flag, 3MEAN -607.098375, 3MIN -1013, 3MAX 831, 3MEDIAN -659, 3ENERGY 2.335867e+12 -- all matching PyRadiomics/SimpleITK to the digit, where the defaults previously gave 2371.901625, 1966, 3810, 2320 and 3.314212e+13. preserve_hu=True now produces the same numbers instead of differing by 1966. The load-time map is recorded per slide and undone on the way out. SlideProps carries inten_scale / inten_offset / inten_map, filled by Nyxus::record_intensity_domain_map at the end of scan_slide_props, and both intensity families apply the inverse in report_in_source_domain: affine for the location statistics, the scale alone for the dispersions, nothing for the standardized moments and the histogram shape, and a recompute from the mapped parts for COV, QCOD and UNIFORMITY_PIU. ENERGY is reconstructed from the two sums already on hand, so no second pass over the pixel cloud is needed. The identity map returns early, which leaves every ordinary non-negative integer image byte-identical. No loader reads a minimum of its own any more -- ImageLoader::open hands each one the recorded offset, and the offset is driven by the ALL-PIXEL minimum, not the within-mask one: offsetting by a within-mask minimum would clamp every off-mask pixel below it and move the ROI's own minimum onto grey level 0, which the grey binning reads as background. Also in scope, because they are the same defect seen from other loaders: - Signed-integer TIFF no longer clamps negatives to 0 in either the tile or the strip loader; both take the offset map. The strip loader had no real-valued handling at all and now shares the same map as the tile loader. - NIfTI scl_slope/scl_inter and DICOM RescaleSlope/RescaleIntercept are applied unconditionally rather than only under --preserve-hu, so a scanner writing a non-identity rescale is no longer wrong by default. Mask loaders are built with the rescale explicitly off, since their pixels are labels. - A real-valued slide's min-max quantization is inverted too, so its features come back in its own float range instead of in quantization steps -- what the IH family already did. - OME-Zarr is recorded as native. Its tile loader copies voxels untouched, so the previous derivation was applying a quantization inverse to a float zarr that never happened. - COVERED_IMAGE_INTENSITY_RANGE scales its ROI range, closing the quantized-vs-source half of the domain mismatch. The truncation half stays and is documented. --preserve-hu survives with a narrower meaning: it selects the offset map for a real-valued slide. Integer, DICOM and NIfTI slides take it regardless, so the flag is no longer what stands between a CT and its Hounsfield units. README, the two rst pages and the three nyxus.py docstrings say so. Goldens: the 3D first-order MATLAB family was pinned in the loader's shifted domain, so all 29 values moved. Re-derived on GNU Octave 11.3.0 + statistics over the same built-ins the MATLAB generator names, with the previous MATLAB values reproduced to 13 significant digits on the old domain first as a control. Every location statistic moved by exactly -1024, every dispersion statistic is unchanged, and COV/QCOD/UNIFORMITY_PIU/ROOT_MEAN_SQUARED were recomputed from their moved parts. The comparison changed with them: a relative tolerance is meaningless once a percentile can sit near zero -- 3P01 is 13 on a 2000-wide range, where 2.4 grey levels of binning is 18% of the value and 0.12% of the range -- so the binned percentiles are measured against the voxel range at 2e-3 (worst residual 1.19e-3), a zero reference against the range at 1e-3, and everything else keeps its relative tier. Every 3D texture golden is unchanged: the offset base is still the all-pixel minimum and the map still truncates, so the stored grey levels are byte-identical wherever a slide holds a negative value. Gates: gtest 889 passed / 1 skipped (GPU) of 890; pytest tests/python/ 98 passed / 1 skipped; check_coverage.py --check and check_test_names.py --check clean; ASan+UBSan gate PASSED on ubtest1 (886 cases, 885 passed / 1 GPU skip, 0 diagnostics, checksums verified before the build and again after the run).
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.
Report first-order intensity features in the slide's own intensity domain
Closes the CT/Hounsfield defect reported against
0.11.0.post1.dev76: a CT NIfTI read with noflag set came back in an arbitrary offset domain rather than in Hounsfield units.
The defect
NiftiLoader::unhounsfieldread a minimum off the buffer it was loading and shifted the wholevolume by
-minwhenever that minimum was negative — which every CT's is. Nothing recorded theshift and nothing undid it, so
MEAN,MIN,MAX,MEDIAN, the percentiles andENERGYweredisplaced by an image-dependent amount.
--preserve-hushifted by a different base (thewithin-mask minimum), so the two settings disagreed with each other as well as with the truth, and
only the intensity-histogram family inverted anything.
Shift-invariant features (
RANGE,VARIANCE,STDDEV) were correct in both modes, which is whatmade it easy to miss: the whole feature set was self-consistent, just displaced.
Measured on the reporter's own image and mask, with no flag:
3MEAN3MIN3MAX3MEDIAN3RANGE3VARIANCE_BIASED3ENERGYpreserve_hu=Truenow produces the same numbers instead of differing by 1966.The fix
The load-time map is recorded per slide and undone on the way out.
SlidePropscarriesinten_scale/inten_offset/inten_map, filled byNyxus::record_intensity_domain_mapat the end ofscan_slide_props. Three maps: the offset map(shift by the floored all-pixel minimum, only when the slide holds a negative value), the
min-max quantization of a real-valued slide into
[0, --fpimgdr], and native.report_in_source_domain: affine for the locationstatistics, the scale alone for the dispersions, nothing for the standardized moments and the
histogram shape, and a recompute from the mapped parts for
COV,QCODandUNIFORMITY_PIU.ENERGYis reconstructed from the two sums already on hand, so there is no second pass over thepixel cloud. The identity map returns early, which leaves every ordinary non-negative integer
image byte-identical.
ImageLoader::openhands each one the recordedoffset. The offset is driven by the all-pixel minimum rather than the within-mask one:
offsetting by a within-mask minimum would clamp every off-mask pixel below it and move the ROI's
own minimum onto grey level 0, which the grey binning reads as background.
Also in scope, because they are the same defect from other loaders
take the offset map. The strip loader had no real-valued handling at all and now shares the same
map as the tile loader.
scl_slope/scl_interand DICOMRescaleSlope/RescaleInterceptare appliedunconditionally rather than only under
--preserve-hu, so a scanner writing a non-identityrescale is no longer wrong by default. Mask loaders are built with the rescale explicitly off,
since their pixels are labels.
float range instead of in quantization steps — what the IH family already did.
native. Its tile loader copies voxels untouched, so the previousderivation was applying a quantization inverse to a float zarr that never happened.
COVERED_IMAGE_INTENSITY_RANGEscales its ROI range, closing the quantized-vs-source half of thedomain mismatch. The truncation half stays and is documented in
not_covered.md§F.--preserve-huchanges meaningIt now selects the offset map for a real-valued slide. Integer, DICOM and NIfTI slides take it
regardless, so the flag is no longer what stands between a CT and its Hounsfield units.
README.md,docs/source/supported_formats.rst,docs/source/cmdline_and_examples.rstand the threenyxus.pydocstrings say so.
The name was kept deliberately, and it is now a poor one — a CT user reading
--preserve-huwould reasonably conclude they must pass it, and they must not. Renaming was considered and
declined for this PR; every doc surface therefore leads with the explicit negative ("NOT needed for
CT — Hounsfield units are preserved and reported either way, despite the name") rather than merely
describing the correct behaviour and leaving the inference to the reader. The flag has never
appeared in a tagged release (last tag
0.11.0, Feb 2026; the flag landed onmainin July 2026),so a rename remains available later at no cost to any released API.
Values that move
Every first-order intensity feature on a slide that holds a negative pixel or is real-valued. This
is not a silent bugfix from a user's point of view — the numbers change, and they change to the
correct ones.
Every 3D texture golden is unchanged. The offset base is still the all-pixel minimum and the map
still truncates, so the stored grey levels are byte-identical wherever a slide holds a negative
value. Whether the truncation should become rounding is a separate call, deliberately kept out.
Goldens re-derived
The 3D first-order MATLAB family was pinned in the loader's shifted domain, so all 29 values moved.
Re-derived on GNU Octave 11.3.0 + statistics over the same built-ins the MATLAB generator names,
with the previous MATLAB values reproduced to 13 significant digits on the old domain first as a
control. Every location statistic moved by exactly -1024, every dispersion statistic is unchanged,
and
3COV/3QCOD/3UNIFORMITY_PIU/3ROOT_MEAN_SQUAREDwere recomputed from their movedparts. The generator and the audit report carry the new domain.
The comparison changed with them. A relative tolerance is meaningless once a percentile can sit near
zero —
3P01is 13 on a 2000-wide range, where 2.4 grey levels of binning is 18% of the value and0.12% of the range — so the binned percentiles are now measured against the voxel range at
2e-3(worst residual
1.19e-3), a zero reference against the range at1e-3, and everything else keepsits relative tier.
Tests
test_hu_analytic.hrewritten: five cases pinning the recorder's three branches and the forwardand inverse maps, including the deliberate choice that OME-Zarr is carried natively.
test_2d_hu_ct_small_pydicom.pynow asserts absolute Hounsfield units directly against pydicomrather than reconstructing them in the test, plus a new case for the invariant this defect would
have failed: for a CT read in Hounsfield units,
MINis the ROI's true Hounsfield minimum.test_2d_signed_int16_loader_mechanics.pyasserts the negatives survive rather than that theyclamp to 0.
test_2d_hu_regression.py/test_3d_hu_nifti_regression.pyre-pinned to absolute values, andeach gained a case asserting the flag is no longer what makes the difference.
test_3d_nifti_mechanics.hsupplies the offset the wayImageLoader::opendoes, and asserts whathappens without one.
Gates
pytest tests/python/check_coverage.py --checkcheck_test_names.py --checkNot in scope
Confirmed on the same data but untouched here:
3VOXEL_VOLUMEstill reports a voxel countrather than a volume (it ignores voxel spacing), and
3MESH_VOLUMEis still byte-identical to3VOLUME_CONVEXHULL. Both are tracked separately.