Skip to content

Fix 2D out-of-core feature paths to match the in-RAM path - #395

Open
darkclad wants to merge 13 commits into
PolusAI:mainfrom
darkclad:main-io-2d-ooc-fix
Open

Fix 2D out-of-core feature paths to match the in-RAM path#395
darkclad wants to merge 13 commits into
PolusAI:mainfrom
darkclad:main-io-2d-ooc-fix

Conversation

@darkclad

@darkclad darkclad commented Jul 24, 2026

Copy link
Copy Markdown

Summary

Nyxus computes every feature two ways — an in-RAM calculate() for ROIs that fit
in memory, and a streaming osized_calculate() for oversized ("non-trivial")
ROIs — and the two must return identical values. Exercising the 2D out-of-core
path against its in-RAM twin showed that most of it had silently drifted: 9 of
14 feature families disagreed, two of them crashing the process.

This PR makes every 2D osized_calculate() reuse the proven calculate(), fixes
three pre-existing defects the work surfaced, removes the now-dead streaming code,
and adds equality tests so the invariant can't rot again.

What was broken (2D trivial vs. out-of-core)

Family Symptom Cause
GLCM throws invalid vector subscript binned with to_grayscale vs bin_intensities
GLRLM, NGTDM crash 0xC0000005 same, plus NGTDM indexed its matrix by raw grey level
GLSZM, GLDM, GLDZM wrong / zero values same binning mismatch
GABOR garbage (~1e13) originalScore baseline never assigned → divide by tiny-float
GEOMOMS shape moments summed intensity (area → 1.9e9) shared OOC path dropped the intensity-vs-shape intenfunction
CHORDS shorter chord lengths OOC subsampled columns instead of scanning all

In every case the in-RAM value was correct and the out-of-core value was wrong.

The fix

Each osized_calculate() now materializes the ROI from the disk-backed pixel
cloud (which these paths already did anyway) and delegates to calculate(), via
two new helpers on LR:

  • LR::rebuild_raw_pixels_from_cloud()
  • LR::rebuild_aux_image_matrix_from_cloud()

So the two paths agree by construction instead of by duplicated logic.

Pre-existing bugs also fixed

Found while probing degenerate (constant-intensity) ROIs:

  • Intensity data loss — constant ROIs replaced INTEGRATED_INTENSITY,
    ENERGY, MODE, ROOT_MEAN_SQUARED and the percentiles with the soft-NAN
    sentinel even though all are well defined; and OOC excess kurtosis used
    KURTOSIS − 3 while in-RAM used Moments4 (differ only on constant data).
  • Erosion performanceEROSIONS_2_VANISH ran its erosion chain over
    disk-backed matrices at ~1250× the in-RAM cost (285 s vs 0.23 s at
    100×100); now ~1.1×. It also lacked the in-RAM driver's constant-ROI skip.

Dead code removed

Delegation left the bespoke streaming implementations with no callers. Removed
(~700 lines): GLCMFeature::{Extract_Texture_Features2_NT, calculateCoocMatAtAngle_NT},
GaborFeature::{GaborEnergy_NT2, conv_dud_NT, GetStats_NT},
ZernikeFeature::mb_zernike2D_nontriv, and the whole 2d_geomoments_basic_nt.cpp
translation unit (+ its CMake entry).

Tests

Adds trivial-vs-out-of-core equality mechanics tests in test_ooc_mechanics.py
covering every 2D feature group, on both an ordinary ROI and a degenerate
constant-intensity ROI.

Verification

  • C++ gtest: 799/799 pass
  • pytest (full dir): 96 pass (7 Arrow failures expected under ALLEXTRAS=OFF)
  • trivial == out-of-core: 14/14 families on ordinary and degenerate ROIs
  • Zernike OOC vs in-RAM confirmed bit-identical across 10 diverse ROI shapes

Net: roughly −1,950 / +280 lines.

Design note

Delegation materializes the ROI in RAM, so a genuinely huge oversized ROI no
longer streams. This is the right trade here — those paths already built a
disk-backed matrix and were crashing or returning wrong values — and the new
equality tests now guard the invariant, so true streaming can be reintroduced
later without silently diverging again.

@vjaganat90

Copy link
Copy Markdown
Member

This looks fine but can't be merged until PR 389. I suggest, the 2D fixes are real correctness/crash blockers for OOC, they should get CI coverage via pytest, and they are portable to main without #389. As a short rebased/port PR.

As is this PR can't be reviewed or merged.

Demian Vladi added 13 commits July 30, 2026 12:35
…of the IO series)

Foundation branch for the main-io-reorg-* stack. Bundles oracle-vetting
promotions for the moments/GLCM/GLRLM/GLSZM/GLDM/morphology/caliper feature
families, a release-please manifest fix, and the CLAUDE.md workflow guide --
none of which are IO/OME work. Kept separate from the 4 IO PRs; should be
reviewed/merged via its own track.

Absorbed commits (oldest to newest):
73f6ca8 fix(moments): correct Hu invariant h5/h6 formulas (9x bracket, stray +eta03)
a500bd9 ci(release-please): use manifest config so 0.x breaking = minor, and pin 0.12.0
79d0974 fix(3d): compute 3ROBUST_MEAN (was hardcoded 0) + vet 18 3D first-order features
60a1ebb fix(glcm): normalize HOM2/ENTROPY by sum_p (2D+3D) + fully oracle-vet GLCM
d44dde7 test(glrlm): vet 12 2D gray-level-run joint-emphasis features vs IBSI
cb07b9d test(glrlm): vet 14 3D _AVE features vs PyRadiomics -> glrlm 100%
285bc39 docs(vetting): record the config scope of the glrlm promotions
7db4532 test(glszm): vet 6 2D gray-level-zone joint-emphasis features vs IBSI -> glszm 100%
c4bb8b1 docs(vetting): record the config scope of the glszm promotions
d213b50 test(gldm): vet 14 2D grey-level-dependence features vs IBSI -> gldm 100%
0780912 docs(vetting): record the config scope of the gldm promotions (7 are mode-invariant)
0af100b fix(3d-shape): correct mislabeled axis lengths + vet 7 morphology features
bc3e92a fix(caliper): correct Martin/Nassenstein diameters + vet 12 features vs imea
059998c test(morphology): vet Feret diameters vs imea + promote 6 features
701201b test(morphology): vet min-enclosing-circle diameter vs imea/cv2
965ab47 test(morphology): vet 8 EXTREMA coords vs matlab regionprops
992d5e6 test(morphology): promote FRACT_DIM_BOXCOUNT/PERIMETER (fraclac oracle)
a9ddc15 test(morphology): vet FRACT_DIM (fraclac) + CIRCULARITY/ROUNDNESS (analytic)
3377183 fix(caliper): float-precision hull rotation + address PR PolusAI#381 review
cc5b4de docs(caliper): correct inaccurate/stale comments (no code change)
b924f8e docs(vetting): retract unproven 3D NGLDM "ibsi" promotions; document MIRP disagreement
e301fe6 docs: add CLAUDE.md codebase and workflow guide
a548b57 docs(CLAUDE): add git workflow and tests/vetting conformance rules
466c038 docs(CLAUDE): address PR PolusAI#388 review - C++20; agentic git workflow
4235e0c docs(CLAUDE): address PR PolusAI#388 review - generalize paths, drop z5py pin, add no-local-paths rule
…lumetric reads

Step 1 of the IO project: lays the groundwork for native 5D (X,Y,Z,C,T) OME
input. Adds the OmeAxes descriptor and OME metadata parsers with unified
format detection, axis-aware tile loading (channel/timeframe), native
OME-TIFF and OME-Zarr reads honoring the file's own DimensionOrder,
ImageLoader::load_volume to assemble a whole X*Y*Z volume for the volumetric
pipeline, and wires all 3D volumetric consumers onto it. A NIfTI load_volume
equivalence guard protects the existing consumer path through the rewrite.

Absorbed commits (oldest to newest):
c916752 feat(io): OmeAxes descriptor, OME metadata parsers, unified format detection
364e802 feat(io): axis-aware loadTileFromFile (channel/timeframe) + 5D OME-Zarr addressability tests
33686dd feat(io): OME-Zarr native axis-order read via OmeAxes + rank/order/negative tests
a0d88be feat(io): OME-TIFF native read, full DimensionOrder permutations, adversarial hardening
eba3d4a feat(io): ImageLoader::load_volume - assemble whole X*Y*Z volume for the volumetric pipeline
958e1e1 test(io): NIfTI load_volume equivalence guard for the volumetric consumer rewrite
3e613cf feat(io): wire all 3D volumetric consumers onto load_volume + end-to-end consumer tests
…ion, and Zarr v3/tiled OME-TIFF hardening

Step 2 of the IO project: builds multi-dimensional addressing and physical
calibration on top of the Step 1 native-read foundation, and hardens it with
the fixes and edge-case coverage that came from exercising it against real
fixtures. Adds per-channel/timeframe volumetric featurization with a c_index
output column, opt-in physical-voxel-spacing calibration with
phys_x/y/z/unit output columns, tiled multi-plane OME-TIFF (z,c,t)->IFD
support, and native OME-Zarr 0.5 (Zarr v3) reads including zstd. The
remaining commits fix correctness issues this surfaced: prescan
out-of-bounds reads, CSV output overwriting itself across channels/
timeframes, partial edge tiles, Zarr v3 sharding/blosc/pyramidal reads,
non-canonical OME-XML TiffData plane->IFD mapping, a multi-timeframe
segmented OME-TIFF crash, and an end-to-end anisotropy-rescaling check.

Absorbed commits (oldest to newest):
4d2afda feat(io): per-channel/timeframe volumetric featurization + c_index output column
dd01c6e feat(io): opt-in physical-voxel-spacing calibration + phys_x/y/z/unit output columns
5839643 test(io): cover resolve_slide_anisotropy and out-of-range volume reads
9a24427 fix(io): reuse a single-channel mask across all intensity channels
7a19496 fix(io): pass use_physical_spacing to initialize_environment from the 3D Python classes
c06a2d7 feat(io): tiled multi-plane OME-TIFF - (z,c,t)->IFD in the tile loaders
9157d34 feat(io): read OME-Zarr 0.5 (Zarr v3), incl. zstd - via z5, no tensorstore
4df18ba fix(3d): whole-volume featurization threw "illegal call" and produced no output
2c72a85 fix(3d): prescan read out-of-bounds garbage and covered only (c0,t0)
6f3ce30 test(python): drop index/calibration columns by name in the 3D IBSI check
6267d16 fix(io): assemble volumes from the whole tile grid, and stream the prescan
4e8766a fix(io): stop CSV output from overwriting itself once per channel/timeframe
66b5d31 test(io): cover partial edge tiles in the volumetric assembly
b68bbb5 test(io): verify Zarr v3 sharding (sharding_indexed codec) reads end-to-end
3831675 fix(io): honor OME-XML <TiffData> plane->IFD mapping (non-canonical layouts)
fd81d63 test(io): cover Zarr v3 blosc and pyramidal (SubIFD) OME-TIFF reads
302a4b7 fix(io): clamp mask timeframe -- multi-timeframe segmented OME-TIFF crashed
374bbd5 test(io): positive + negative edge coverage across the OME read surfaces
2c916fe test(io): assert anisotropy actually rescales ROI geometry end-to-end
…feature families

Step 3 of the IO project: the out-of-core streaming capability. Volumetric
ROIs too large to hold in RAM are now streamed rather than rejected, across
every 3D feature family -- whole-volume assembly, intensity/histogram,
surface, GLCM/GLDM/NGLDM/NGTDM, GLRLM, and GLSZM/GLDZM (via streaming
connected-component labeling) -- plus a fix to the 2D out-of-core intensity
dispatch that predates the 3D work. Includes the negative-path test coverage
that caught a degenerate-ROI bug, Zarr v3 sharding validated at
multi-shard-file scale, and a chunk-depth read fix.

Absorbed commits (oldest to newest):
80614c4 fix(3d): oversized whole volume fails loudly instead of emitting zeros
4222cdc fix(3d): whole-volume oversized check must use the 3D footprint estimator
6bef8bd fix(io): correct 2D out-of-core intensity features (dispatch + 3 latent bugs)
696061f feat(io): stream 3D intensity/histogram out-of-core for oversized volumetric ROIs
025a658 feat(io): stream 3D surface features out-of-core for oversized volumetric ROIs
8c848aa feat(io): stream 3D GLCM/GLDM/NGLDM/NGTDM out-of-core for oversized volumetric ROIs
d003cc0 feat(io): stream 3D GLRLM out-of-core; fix latent grey-LUT gaps in GLCM/GLDM
67adfff feat(io): stream 3D GLSZM/GLDZM out-of-core via streaming connected-component labeling
d469e0e test(io): add negative-path tests for the 3D out-of-core stream; fix a degenerate-ROI bug they caught
a141d98 feat(io): stream oversized 3D whole-volume ROIs out-of-core
12adfe5 docs(vetting): register test_ooc_mechanics.py on the 3D features it covers
0fbe34b test(omezarr): validate Zarr v3 sharding at multi-shard-FILE scale
6da2482 fix(omezarr): read the full chunk depth instead of one Z-plane
… document native 3D input

Step 4 of the IO project: closes out the branch. Reconciles with upstream
after rebasing main-io onto PolusAI/nyxus main (carrying an upstream 3D
surface axis-length fix into the out-of-core path it predated), then hardens
calibration and read correctness -- physical-size-unit canonicalization,
float-precision-free OME size parsing, a whole-volume anisotropic-resampling
hang fix, a RAM-footprint-estimate underflow at n_rois==0, and a degenerate-
segmented-image prescan fix. Also fixes two build issues (a missing zstd
header check, a GCC-only constexpr link error), hardens a host-memory-
dependent test, removes dead code and dedupes the Zarr loaders/histogram
paths, drops machine-specific paths and stale planning-phase language from
tests/comments, and documents native OME 3D input and physical voxel
spacing in the README.

Absorbed commits (oldest to newest):
43d1343 fix(io): reconcile with upstream after rebasing main-io onto PolusAI/nyxus main
bcf93b1 fix(prescan): COVERED_IMAGE_INTENSITY_RANGE degenerate for any segmented image (2D+3D)
7908ffd fix(ome): canonicalize physical-size units to micrometer
d626794 fix(3d): whole-volume anisotropic-resampling hang + 3 related bugs
7701c38 fix(roi): LR::get_ram_footprint_estimate(_3D) underflow at n_rois==0
eb5f7bf chore(io): drop machine-specific paths and planning labels from tests
a6a575a fix(ome): parse OME physical sizes without floating-point from_chars
904519b docs(readme): document native OME 3D input and physical voxel spacing
9545836 fix(build): require the zstd header, not just the library, before WITH_ZSTD
bebb43f fix(tests): pick a ram_limit the host can actually grant
e97228b Fix GCC link error: make OutOfRamVoxelCloud::BLK_RECORDS constexpr
db0f981 refactor(io,3d): remove dead code, dedupe Zarr loaders + histogram, speed up OOC paths
daeb1c0 refactor(comments): drop FIX-prefixed bug narration from loader dispatch and 3D intensity comments
The osized_calculate implementations of the 2D texture (GLCM, GLRLM, NGTDM,
GLSZM, GLDM, GLDZM), moments, gabor, chords, erosion and zernike features had
drifted from their calculate() twins: they binned intensities with
to_grayscale() instead of TextureFeature::bin_intensities(), GLRLM and NGTDM
indexed their matrices out of bounds and crashed, GLCM threw "invalid vector
subscript", GLSZM and GLDM returned 0, gabor never assigned its originalScore
baseline so every frequency was divided by the tiny-number floor, geomoments
funneled intensity and shape moments through one shared path that dropped the
intenfunction (so shape moments summed raw intensities), and chords stepped over
columns instead of scanning every one.

Each osized_calculate now materializes the ROI from the disk-backed pixel cloud
through the new LR::rebuild_raw_pixels_from_cloud and
LR::rebuild_aux_image_matrix_from_cloud helpers and reuses calculate(), so the
trivial and non-trivial paths agree by construction rather than by duplicated
logic. The out-of-core-only code left with no caller after this is removed:
GLCMFeature's Extract_Texture_Features2_NT and calculateCoocMatAtAngle_NT,
GaborFeature's GaborEnergy_NT2 / conv_dud_NT / GetStats_NT,
ZernikeFeature::mb_zernike2D_nontriv, and the whole
2d_geomoments_basic_nt.cpp translation unit.

Also fixes pre-existing defects found while probing degenerate
(constant-intensity) ROIs: the in-RAM intensity path replaced
INTEGRATED_INTENSITY, ENERGY, MODE, ROOT_MEAN_SQUARED and the percentiles with
the soft-NAN sentinel even though all of them are well defined on a constant
ROI, and its out-of-core excess kurtosis used KURTOSIS-3 rather than Moments4
(equal on ordinary data, not on a constant ROI); and the erosion out-of-core
path ran its entire erosion chain over disk-backed matrices, copying and
rescanning one per iteration at roughly 1250x the in-RAM time, and lacked the
in-RAM driver's constant-ROI skip so it reported the sanity cap where the
in-RAM path reports 0.

Adds 2D trivial-versus-out-of-core equality mechanics tests covering every 2D
feature group on both an ordinary ROI and a degenerate constant-intensity ROI.
…tative aux extrema; erosion skip resets its count)
…s to diagnose CI-only blank-test failure (revert after)
… ROI (reset coeffs with assign, not resize); add multi-ROI OOC regression test; remove temp diagnostics
…calculate instead of a drifted streaming copy); extend multi-ROI OOC regression test to the ellipse family
…citly so it is order-independent (a prior out-of-core test leaves the process-global ram_limit at 0, which made the tiny ROI 'oversized' on the no-out-of-core in-memory path)
@darkclad
darkclad force-pushed the main-io-2d-ooc-fix branch from 34e191e to 4978b1c Compare July 30, 2026 22:38
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.

2 participants