Fix 2D out-of-core feature paths to match the in-RAM path - #395
Open
darkclad wants to merge 13 commits into
Open
Fix 2D out-of-core feature paths to match the in-RAM path#395darkclad wants to merge 13 commits into
darkclad wants to merge 13 commits into
Conversation
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. |
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.
…indeterminate scalar
…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
force-pushed
the
main-io-2d-ooc-fix
branch
from
July 30, 2026 22:38
34e191e to
4978b1c
Compare
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.
Summary
Nyxus computes every feature two ways — an in-RAM
calculate()for ROIs that fitin 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 provencalculate(), fixesthree 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)
invalid vector subscriptto_grayscalevsbin_intensities0xC0000005originalScorebaseline never assigned → divide by tiny-floatintenfunctionIn 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 pixelcloud (which these paths already did anyway) and delegates to
calculate(), viatwo 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:
INTEGRATED_INTENSITY,ENERGY,MODE,ROOT_MEAN_SQUAREDand the percentiles with the soft-NANsentinel even though all are well defined; and OOC excess kurtosis used
KURTOSIS − 3while in-RAM usedMoments4(differ only on constant data).EROSIONS_2_VANISHran its erosion chain overdisk-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 whole2d_geomoments_basic_nt.cpptranslation unit (+ its CMake entry).
Tests
Adds trivial-vs-out-of-core equality mechanics tests in
test_ooc_mechanics.pycovering every 2D feature group, on both an ordinary ROI and a degenerate
constant-intensity ROI.
Verification
ALLEXTRAS=OFF)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.