fix: repository-wide bug sweep (77 bugs), stale-code removal and simplification - #53
Open
alxndrkalinin wants to merge 35 commits into
Open
fix: repository-wide bug sweep (77 bugs), stale-code removal and simplification#53alxndrkalinin wants to merge 35 commits into
alxndrkalinin wants to merge 35 commits into
Conversation
ruff selected only D and I, so unused imports and variables were never caught and had accumulated. Example notebooks are authored content, so they keep only the import-order and docstring rules. trimesh's curvature path imports rtree, so extract_mesh_features raised ModuleNotFoundError for anyone following the documented cubic[mesh] install.
to_device(array, "CPU") used np.asarray, which CuPy rejects, so Image.to_cpu() and cross-device to_same_device had never worked; there was no test_image.py at all. The SciPy and scikit-image proxies duplicated their wrapper body, and only the SciPy one fell back to the host when the GPU backend lacked a function -- GPU input to any skimage function cuCIM does not implement raised AttributeError instead of running on host. Both now delegate to dispatch_device_call, so the fallback and the misleading "cupyx ... falling back to CPU" warning are fixed once. Array results, including every array in a returned tuple, come back to the GPU. CUDAManager published cls._instance before init_gpu() populated it, so a racing thread saw an object without cp/cucim/num_gpus. plot_utils handed CuPy arrays straight to matplotlib.
Each of these produced a crash or a silently wrong result: - _tukey_window_1d divided by n-1, so a length-1 axis yielded NaNs and a length-2 window; tukey_window raised on any single-slice volume. - random_crop and get_xy_block_coords unpacked shape[1:], failing on 2D and leaving random_crop's ndim > 2 branch unreachable. - pad_image indexed a sequence pad_size by axis number and discarded any (before, after) intent, so per-axis padding was wrong and axes=[1,2] raised IndexError. It now aligns positionally with axes. - crop_to_divisor dropped axes for all four corner crops, cropping the default [1,2] while crop_size was computed for the caller's axes. - crop_corner gated the far-edge test on len(axes) >= 2, so a single-axis crop_bl was indistinguishable from crop_tl. - distance_transform_edt silently discarded block_params and float64_distances on GPU, the two arguments the host branch rejects as GPU-only, and dispatched on isinstance(np.ndarray) so a list took the cuCIM branch. - clahe floor-divided the image shape by its tile count with no clamp. - image_stats returned a misspelled percentile key. Input validation moves from assert (stripped under -O) to ValueError.
…g input The Wiener-Butterworth back projector was wrong for even-shaped PSFs in two independent ways. The centered grids used (S-1)/2 while ifftshift puts index S//2 at DC, so the Butterworth mask was not Hermitian and np.real discarded a large imaginary part; and f[::-1] maps centered index j to -j-1 on even axes, so the flipped PSF was not the conjugate OTF. Measured on a 16^3 PSF, imaginary leakage in the returned projector's OTF: traditional 5.32e-1, gaussian 2.92e-1, wiener 1.00e0, wiener-butterworth 8.81e-1, all now ~1e-17. Odd-shaped PSFs are bit-identical except gaussian at res_flag 0/2, which previously ignored res entirely and now tracks it as the reference does. This diverges from BackProjector.m for even shapes, deliberately: the reference assumes odd-sized PSFs. richardson_lucy_xp did image *= mask, and img_as_float returns the same object for float input, so it destroyed the caller's array. beta > 1 returned an all-NaN projector and alpha = 0 a non-finite one; both now raise, as do a negative Butterworth ee and a zero forward gain at the cutoff. The unmatched path's 1e-3 floor was absolute, clamping 32.4% of a uint16 image scaled into [0,1]; it now scales with the data and is exposed as small_value.
…olution radial_bin_id treated all-1.0 spacing as index units while radial_edges stayed in physical units, so at spacing=1.0 np.digitize clipped every non-DC voxel into the last bin: 1 of 32 bins occupied on a 64x64. frc_resolution returned nan, dcr_resolution inf, and spectral_pcc silently degraded to an unweighted correlation. five_crop_resolution and grid_crop_resolution had never executed -- both passed 2D arrays to the 3D-only fsc_resolution and raised IndexError. They are in __all__ and had no tests. Their XZ slices were also reflect-padded to square, replicating real data 2-16x and holding FRC above threshold so every xz value was NaN; unpadded, per-slice finite counts go 0/8 to 8/8, 1/32 to 31/32 and 3/64 to 64/64. The default aggregate is now nanmedian, since one unmeasurable tile should not propagate to the whole aggregate. Sectioned FSC normalized its axis by the minimum Nyquist but inverted with the XY spacing, reporting 0.1225 um at 0.13 um sampling -- below the physical floor. Now 0.395 um. _frc_dataset drops zero-count bins: the empty innermost bin's log-domain quotient landed at exactly 1.0, and that fabricated point made the analyzer read curves as starting above threshold, so every binomial FRC returned NaN. _kc_to_resolution returns NaN, never inf, matching the FRC/FSC no-crossing convention. Removes ~250 lines of never-instantiated iterators and their private helpers, and the unused get_frc_options and nitems.
The shims warned that the old path would be removed in 0.8.0, and v0.8.0 is tagged and released with them still present. Nothing in the package, tests, notebooks or docs imports them.
…handling Masked ssim eroded the validity footprint with width 7, but skimage's own default with gaussian_weights=True is 11, so out-of-mask pixels leaked into the masked mean: a fully-corrupted-outside fixture scored 0.99721 where the correct footprint gives exactly 1.0. spectral_pcc derived its weights from radial_power_spectrum of the raw target -- no mean subtraction, no apodization -- while correlating the windowed spectrum, so the DC pedestal and edge discontinuities inflated the low-frequency weights. It now reuses F_targ, which also removes a redundant FFT and grid build. estimate_cutoff silently ignored an unrecognized method, returning the Nyquist-only bound; band_limited_ssim returned nan for a flat filtered target while band_limited_pcc returned 0.0. nan is now the single degenerate convention. _min_max_to_unit called type(rng)(eps), which raises for a 0-d CuPy scalar. _label_overlap cast to uint32 only on the GPU path, so a float label image worked on GPU and raised on CPU. A fully-labelled mask was rejected as non-sequential when the real complaint was the missing background. mask is keyword-only on the four decorated metrics, so scale_invariant can no longer miss a positional mask. morphology.square/cube are removed in skimage 0.27; footprint_rectangle produces byte-identical footprints.
…args ms_ssim never cast its inputs, so np.pad preserved uint8 and i1p * i1p wrapped mod 256, while skimage's gaussian additionally rescaled by 1/255 against a data_range of 255. ms_ssim(u8, u8+5, data_range=255) returned 0.7446 where the float64 reference gives 0.9998; ms_ssim(u8, u8) gave 0.9999999985 rather than 1.0. Both now match float64 exactly. compute_ssim_elements hardcoded gaussian_weights and crop, so the documented pass-through raised TypeError for either. win_size=1 passed validation and then divided by zero; with population covariance it built slice(0, -0) and returned silently empty element arrays. The kernel_size docstring claimed it matched torchmetrics, which sizes its Gaussian from sigma alone and ignores kernel_size. Corrected the docstring rather than the numerics, with the measured divergence bounds. Drops _flatten_elements, whose five array copies per call were a no-op because .mean() already reduces over all axes, and folds the duplicated quadratic block and the two fit-and-score clones. Verified bit-identical against HEAD on all 60 fixture slices; the float64 cast is the only numeric change and it moves MicroMS3IM closer to the upstream fixture (4.172e-07 to 3.732e-07).
remove_touching_objects built morphology.cube(3), which the proxy routes to host skimage, so a NumPy footprint met a CuPy image and cuCIM raised -- it failed on every GPU label image. cube(3) is also always 3D, so it raised on every 2D one, and cellpose_segment calls it unconditionally. Rewritten to derive adjacency from shifted comparisons, which needs no footprint at all: 500 labels in 32x128x128 goes 4.997s to 0.0117s (426x), byte-identical. cleanup_segmentation cast to uint16, silently wrapping past 65535 -- with 70000 objects, distinct labels collapsed and 2 pixels became background. _remove_small_holes filled area <= N on CPU but < N on GPU, so the same call returned different label images per device. clear_xy_borders preserved label ids in 2D but relabelled in 3D, making the "transforms preserve labels" comment false for the main use case. remove_large_objects, remove_touching_objects, remove_thin_objects and cleanup_segmentation's hole fill all mutated the caller's array. fill_label_holes raised TypeError for any kwarg; fill_holes_slicer raised on CuPy input and aliased NumPy input. np.unique(x)[1:] skipped a real label whenever no background pixel existed. find_objects was 48 lines reimplementing scipy.ndimage.find_objects: identical output, 1000 labels in 64x256x256 goes 1.337s to 0.0081s. _fill_holes_and_size_filter now compacts label ids as upstream does; gapped ids fed _stitch3D NaN IoU rows. compute_masks is guarded by the availability flag the module docstring promises, and cellpose models are cached per (pretrained_model, gpu) instead of reloading weights.
…eshes
morphology_correlations sliced np.corrcoef by len(features), but
extract_features expands vector properties, so with
features=["area", "centroid"] on a 3D image the 4-column matrix was
sliced as if it had 2. The diagonal then read true-vs-true entries and
never touched the prediction: identical inputs returned
{'area': 0.921, 'centroid': 0.212} instead of 1.0. extract_features now
also returns the expanded column names, and results are keyed by them.
_direction_matrix returned an all-zero GLCM for a direction with no
valid pairs and the mean divided by len(offsets) regardless, so empty
directions were averaged in as zeros. A flat single-slice object in a
3D volume scored entropy 1.520 against 4.941 for the same voxels as 2D,
and correlation flipped sign (-0.017 to +0.687). Since the dilution
tracked object shape, it would fabricate a morphology-texture
correlation. Only non-empty directions are averaged now.
mask2mesh pads the mask, so border-touching labels no longer produce
open meshes with negative volume that passed the isfinite guard. The
same change lets each label be cropped to its bounding box: 20 labels in
128^3 goes 0.496s to 0.045s.
Voxel spacing could not reach marching cubes, a 4x volume error on
z-anisotropic stacks. An int-typed feature range floor-divided the
zero-range guard to 0, yielding inf. A non-bool mask silently did
bitwise-and and integer fancy-indexing. regionprops_table(properties=None)
returned an empty dict.
GLCM accumulation is one bincount per direction summed on device with a
single host transfer, rather than concatenating every direction's
indices: peak pool at 256^3 goes 3.95 GB to 0.53 GB.
_calculate_fsc_sectioned_hist now returns (data, max_freq) so the resolution conversion can use the Nyquist the axis was normalized by.
Contributor
There was a problem hiding this comment.
Sorry @alxndrkalinin, your pull request is larger than the review limit of 150000 diff characters
There was a problem hiding this comment.
Pull request overview
This PR performs a broad reliability and correctness sweep across cubic, with a strong emphasis on device-agnostic (CPU/GPU) parity and regression testing for previously silent wrong-results and crash cases in metrics, segmentation, and preprocessing utilities.
Changes:
- Adds extensive regression tests across proxies, CUDA utilities, segmentation, preprocessing, and metrics to prevent CPU/GPU divergence and silent numerical errors.
- Refactors the SciPy/scikit-image proxy routing into a shared
dispatch_device_callhelper with consistent CPU fallback + result re-homing to GPU. - Fixes/updates multiple computational kernels and APIs (e.g., feature extraction naming/shape, IoU/label overlap, MS-SSIM/SSIM edge cases, Cellpose integration and GPU post-processing), plus removes deprecated/stale shims and code.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_skimage_proxy.py | Adds proxy fallback/tuple-return tests for cuCIM-missing functions/modules. |
| tests/test_scipy_proxy.py | Extends SciPy proxy tests for correct fallback behavior and warning suppression on unknown CPU symbols. |
| tests/test_plot_utils.py | Verifies plotting helpers accept GPU arrays by moving inputs to host for Matplotlib. |
| tests/test_image.py | Introduces coverage for Image device handling and CPU/GPU round-trips. |
| tests/test_cuda.py | Adds coverage for to_device(..., "CPU") with GPU inputs and CUDAManager concurrency behavior. |
| tests/segmentation/test_clear_border.py | Adds behavioral tests for non-mutation, out=, and removed in_place. |
| tests/segmentation/test_cellpose.py | Adds caching/availability behavior tests and signature tightening for Cellpose wrapper. |
| tests/segmentation/test_cellpose_dynamics.py | Adds regression tests for Cellpose GPU mask post-processing correctness. |
| tests/preprocessing/test_thresholding.py | Adds GPU-input regression test for select_nonempty_patches. |
| tests/metrics/test_skimage_metrics.py | Adds extensive regression suite for masked SSIM windowing, deprecations, keyword-only mask, and scale-invariant validation. |
| tests/metrics/test_feature.py | Adds regression tests for expanded feature columns and correct correlation slicing. |
| tests/metrics/test_average_precision.py | Adds regression tests for dtype/label validation and IoU/background handling. |
| tests/metrics/spectral/test_radial.py | Adds regression tests around spacing==1.0 behavior, overflow exclusion, and reducers. |
| tests/metrics/microssim/test_ssim_elements.py | Adds edge-case tests for win_size=1 + covariance handling. |
| tests/metrics/microssim/test_ri_factor.py | Adds solver/layout invariance tests and more robust bracketing/bisection regression checks. |
| tests/metrics/microssim/test_ms_ssim.py | Adds integer dtype promotion regression tests and new helper coverage. |
| tests/metrics/microssim/test_micro_ssim.py | Adds regression tests for kwargs pass-through and explicit unsupported flags. |
| tests/feature/test_voxel.py | Updates tests for extract_features returning expanded column names and improved validation. |
| tests/feature/test_texture.py | Adds direction-matrix validation tests and new error-path coverage for masking/empty directions. |
| tests/feature/test_mesh.py | Adds extensive mesh feature pipeline regression tests, including spacing and border-touching labels. |
| pyproject.toml | Adds rtree to mesh extra; expands Ruff lint selection and notebook ignores. |
| examples/scripts/resolution_estimation_3d.py | Updates to new _calculate_fsc_sectioned_hist return signature. |
| examples/notebooks/resolution_estimation_3d.ipynb | Updates to new _calculate_fsc_sectioned_hist return signature. |
| cubic/utils.py | Fixes docstring typo and makes print_mean_std output more stable/readable. |
| cubic/skimage.py | Refactors skimage proxy to use shared dispatch_device_call and improves CPU fallback behavior. |
| cubic/scipy.py | Refactors SciPy proxy to use shared dispatch_device_call. |
| cubic/cuda.py | Makes CUDAManager construction atomic; adds dispatch_device_call; fixes CPU transfers for GPU arrays. |
| cubic/segmentation/init.py | Fixes module docstring to “segmentation”, not “preprocessing”. |
| cubic/segmentation/_clear_border.py | Updates API to modern out= model and makes implementation device-agnostic. |
| cubic/segmentation/cellpose.py | Removes import-time warning; adds model caching + model= injection; removes inert parameter. |
| cubic/segmentation/cellpose_sam_gpu.py | Fixes GPU resize handling and various internal behavior/comments in the GPU pipeline. |
| cubic/segmentation/cellpose_dynamics.py | Adds explicit cellpose requirement checks and multiple correctness fixes in mask generation/stitching. |
| cubic/preprocessing/init.py | Exposes richardson_lucy_iter publicly. |
| cubic/preprocessing/thresholding.py | Switches to asnumpy for CuPy compatibility and clarifies return type/contract. |
| cubic/preprocessing/richardson_lucy_xp.py | Adds input validation, prevents input mutation, and improves unmatched-path stability controls. |
| cubic/plot_utils.py | Moves GPU inputs to host before Matplotlib calls; clarifies bit_depth caveats in docs. |
| cubic/metrics/spectral/README.md | Updates module list to reflect new/expanded iterator/reducer responsibilities. |
| cubic/metrics/spectral/plot.py | Narrows exception handling and emits explicit warnings on expected analysis failures. |
| cubic/metrics/spectral/iterators.py | Removes stale iterators/helpers; adds overflow-exclusion controls; simplifies iterators. |
| cubic/metrics/spectral/analysis.py | Removes stale option helpers; improves iteration safety; makes error handling and “no resolution” behavior explicit. |
| cubic/metrics/skimage_metrics.py | Makes masks keyword-only; fixes SSIM mask erosion windowing; hardens scale-invariant path validations. |
| cubic/metrics/pcc.py | Makes mask keyword-only to prevent signature ambiguities with decorators/kwargs. |
| cubic/metrics/ms_ssim.py | Promotes integer inputs safely; adds _ssim_scale_component helper; reduces unnecessary work per scale. |
| cubic/metrics/microssim/ssim_elements.py | Tightens validation around window size + covariance and cropping edge cases. |
| cubic/metrics/microssim/micro_ssim.py | Refactors scoring prep; fixes kwargs pass-through; adds fit_and_score API. |
| cubic/metrics/microssim/micro_ms3im.py | Reuses shared preparation path; clarifies behavior for unsupported return-components flag. |
| cubic/metrics/microssim/init.py | Exposes validate_alpha_bounds and updates exports accordingly. |
| cubic/metrics/frc/init.py | Removes deprecated backward-compatibility shim module. |
| cubic/metrics/frc/analysis.py | Removes deprecated backward-compatibility shim module. |
| cubic/metrics/frc/dcr.py | Removes deprecated backward-compatibility shim module. |
| cubic/metrics/frc/frc.py | Removes deprecated backward-compatibility shim module. |
| cubic/metrics/frc/iterators.py | Removes deprecated backward-compatibility shim module. |
| cubic/metrics/frc/plot.py | Removes deprecated backward-compatibility shim module. |
| cubic/metrics/frc/radial.py | Removes deprecated backward-compatibility shim module. |
| cubic/metrics/average_precision.py | Improves label validation, GPU kernel reuse, background handling, and avoids redundant scans. |
| cubic/metrics/feature.py | Fixes correlation slicing to use expanded column widths; improves scalar typing consistency. |
| cubic/feature/voxel.py | Makes extract_features return column names; improves property/range validation and dtype stability. |
| cubic/feature/texture.py | Avoids averaging empty directions; validates mask dtype; reduces host/device sync overhead. |
| cubic/feature/mesh.py | Adds spacing support, bounding-box meshing, dependency guards (trimesh/rtree), and geometry validation. |
| cubic/image.py | Replaces assert-based device validation with to_device’s ValueError behavior. |
| cubic/init.py | Fixes top-level package docstring typo (“Morphometric”). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The test pinned NaN for one smoothed-random volume, but whether that Z curve has an interior decorrelation peak depends on the scikit-image version: 0.25.2 finds a cutoff at 2.506 where 0.26.0 finds none. CI runs 0.25.2, so the assertion failed there while passing locally. The invariant the fix actually guarantees is that a missing cutoff never escapes as inf from a divide-by-zero. That is now asserted for both sectors and all spacings, with the deterministic NaN contract left to test_kc_to_resolution_returns_nan_without_a_cutoff, which exercises the conversion directly. Verified still failing when inf is reintroduced.
Lockfile entry for the rtree>=1.0 dependency added to cubic[mesh], so uv sync resolves the same versions the pyproject change requires.
The 7 -> 11 footprint fix hardcoded 11, which is only skimage's window for
sigma=1.5. skimage derives win_size = 2 * int(3.5 * sigma + 0.5) + 1 and
sigma arrives through **kwargs, so any other sigma under-eroded and let
out-of-mask pixels back into the "masked" mean -- the same defect one level
out. On a 64x64 fixture with a 32x32 mask and everything outside corrupted,
sigma=3.0 (skimage window 23) returned 0.2067 where the correct masked value
is 1.0. sigma=5.0 needs a 37 px window, wider than the mask, and returned a
number where NaN is the honest answer; it now reports NaN.
The masked branch forces full=True to obtain the SSIM map, so forwarding
gradient=True made skimage return a 3-tuple into a 2-target unpack
("too many values to unpack"). The batched path already rejected it; the
masked path now does too.
footprint_rectangle requires scikit-image >= 0.25 (absent in 0.22 and 0.24),
so the declared >= 0.16.1 floor would have failed at call time on any
version in between. cuCIM ships only footprint_rectangle, so the pin is the
correct half to move.
Four cases where a fix in this series left, or newly opened, a silent failure: - richardson_lucy_xp: small_value defaults to 1e-6 * image.max(), so an image containing inf produced an inf floor. `not small_value > 0` is False for inf, so validation passed, np.maximum made the whole volume inf, and the first FFT turned it to NaN with no exception. - random_crop: switching to shape[-2:] fixed 2-D but left the slice hard-coded to axes 1 and 2, so 4-D input applied the Y range to axis 1 and returned an empty array. That traded a loud unpack error for silent garbage; the crop now targets the trailing two axes at any rank. - dispatch_device_call: the new host fallback promised device-consistent results but only moved a bare array or a flat tuple, so list returns (find_contours, absent from cuCIM) stayed on the host. It now recurses. Arguments the callee writes into (out/output/distances/indices) were coerced to host copies and silently discarded; passing a GPU array under one of those names now raises instead. - _endpoint_indices: the fftshift-origin change was untested, because the only assertion covering it compares against (size-1)/2 for odd sizes, where the two agree. Reverting it left the whole suite green while silently reshaping the Wiener and Butterworth filters for every even-shaped PSF (N=16, t=2.5 moves the endpoints from (5,10) to (6,11)). Now pinned on even sizes and against the fftshift DC bin.
…bels, warn on dead tiles mask2mesh pads by one voxel to close border-touching surfaces but never subtracted the pad, so every vertex sat one voxel high on each axis -- scaled by spacing, making the offset physical. A mask at indices 2..3 reported centroid z 3.5 instead of 2.5, and 14.0 instead of 10.0 with spacing=(4,1,1). Every feature in mesh_feature_list is translation-invariant so extract_features was unaffected, but mask2mesh is public and its mesh is expected to overlay the image. remove_touching_objects indexes its `touching` flags by raw label value, so a negative id raised IndexError and an all-negative image reached np.zeros(negative). check_labeled_binary now rejects negatives, which also protects remove_large_objects' np.bincount. find_objects passed `max_label or 0` to scipy, where 0 means "all labels", so an explicit max_label=0 was silently inverted from "none" to "all". It also let a float label image fail inside scipy internals; the dtype guard now matches the one mesh.extract_features already had. _crop_slice_resolutions defaults to np.nanmedian, which warns only when every input to an output position is NaN. A tile that measured nothing at any plane therefore vanished from the aggregate silently: on a fixture with one flattened tile, 1 of 4 max_projection and 8 of 32 xy measurements were dropped while the aggregate read fully finite. It now reports how many tiles had no threshold crossing. Also replaces the rationale on test_remove_touching_objects_without_background: it claimed to guard the np.unique(...)[1:] slice, but for this function a skipped lowest id is still flagged as another label's neighbour, so the test could not fail for the stated reason. The _label_ids tests do guard that slice.
…xis length The index-unit frequency grid scaled each axis by its own length (fftfreq(n) * n), so a constant-radius ring became an ellipse in physical frequency as soon as the axes differed in length: one bin averaged unlike frequencies together, and the two spellings of "no physical units" disagreed. A (64, 128) array gave 32 bins for spacing=None against 64 for spacing=1.0, with different per-voxel assignments. Square input happened to agree, which is why it went unnoticed. radial_bin_id already disagreed with radial_k_grid, which used cycles per pixel for None; dcr.py worked around it by substituting [1.0] * 3. Route every None through _spacing_or_unit instead, deleting the duplicate index-unit branches in radial_edges, radial_bin_id, radial_k_grid, sectioned_bin_id and both mask-backend iterators. Consequences: use_max_nyquist is now a no-op without spacing, since one unit per axis gives every axis the same Nyquist however long it is; radial_k_grid derives k_max rather than hardcoding 0.5, which was wrong for odd axes that top out at (n // 2) / n; and _normalization_spacing loses its hardcoded 1.0 for the same reason. radial_edges(shape, spacing=None) now returns edges in cycles per pixel (0 to 0.5) rather than index bins (0 to n/2). Normalized frequency axes and every reported resolution are unchanged for square input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…e Z axis A sector centred on polar angle theta measures the shell radius |k| = k_z / cos(theta), not k_z, and the cascade reports the highest sector below 45 degrees -- 38 degrees by default -- rather than the Z-aligned cone. The reported period therefore understated the axial period by cos(theta). Against a volume with an ideal axial cutoff at 0.5 cycles/um (true axial period 2.0 um) at spacing [0.5, 0.19, 0.19], the uncorrected number read 1.369 um, 1.46x too fine. The Koho et al. (2019) eq. (5) multiplier that used to be applied here, 1 + (spacing_z/spacing_xy - 1)|cos(theta)|, read 3.13 um, 1.56x too coarse: it both double-counted anisotropy the physical-frequency grid already encodes and used the wrong functional form. Dividing the period by cos(theta) reads 1.737 um, and applied to the 22/38/52/68-degree sectors gives 1.745/1.737/1.606/1.670 -- four independent sectors collapsing onto one value, the residual matching the threshold-crossing bias xy also shows. The projection is purely geometric and carries no spacing term. Sectors at or above 45 degrees are XY-limited, so their band edge says nothing about the axial cutoff and 1/cos(theta) would inflate it up to 7x. The cascade no longer falls back to them; z is nan with a warning instead of an in-plane number relabelled as axial. angle_delta=90 now raises, since one sector spanning 0-90 degrees cannot separate the two directions and previously reported the same value for both keys. FourierCorrelationAnalysis.execute(z_correction=...) is left in place for callers reproducing miplib numbers, so z_factor and its plumbing through _resample_isotropic_for_fsc and _fsc_extract_resolution are now unused and removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
pyproject raised the minimum to >=0.25 for morphology.footprint_rectangle, but the lockfile still recorded >=0.16.1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
Minor rather than patch: the sweep changes public signatures, raises the scikit-image floor to >=0.25, and moves reported resolution numbers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…ed volumes The geometric 1/cos(theta) projection applies to a grid whose axes carry their true physical Nyquists. Isotropic resampling is different: interpolating Z up adds no information, so the volume's real axial band limit stays at the *original* Z Nyquist while the grid now runs to the XY one, and Koho et al. (2019) eq. (5) is what converts back. Removing eq. (5) from that path too moved the Fig. 4b pollen stack from 4.363 um -- 12% above the published 3.91 -- to 2.022 um, 48% below, while XY stayed at 0.586 against a published 0.59. Re-running resolution_estimation_3d.ipynb is what surfaced it. _resample_isotropic_for_fsc reports the anisotropy ratio again, now as _fsc_extract_resolution's resampled_anisotropy: None selects the geometric projection, a ratio selects eq. (5). The two are not combined -- applying both gives 5.54 um, further from the paper than either alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
Outputs regenerated against the current code. Results are unchanged or
within noise of the previously committed runs:
2D FRC 287.8 / 238.3 / 101.8 -> 286.9 / 236.6 / 103.1 nm
2D DCR identical (326.2 / 286.8 / 111.0 nm)
pollen FSC XY 583.6 -> 585.6, Z 4363.1 -> 4378.1 nm
(paper reference 590 / 3910 nm)
binomial FSC Z identical at 697.7 nm
feature extraction identical, incl. the 1.72e-03 CPU/GPU delta
segmentation mAP identical (nuclei 0.709, cells 0.588)
The sub-percent FRC/FSC shifts come from the signed frc_from_sums default
and the digitize-edge epsilon, both already documented in this series.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…rable axial FSC
Both notebooks track sectioned FSC on a 30-plane stack at 0.3 um Z
spacing. Now that the axial cascade no longer falls back to sectors at or
above 45 degrees, no Z-dominated sector crosses the threshold and the
axial value is nan throughout:
deconvolution_iterations_3d FSC stopping criterion: iter 41 -> iter 0
Z 423.6 -> 271.5 nm becomes 439.9 -> nan
wb_backprojector_3d the axial subplot is now empty
The previous curves were not merely different, they were unphysical: the
axial Nyquist floor here is 2 * 0.3 um = 600 nm, and they plotted 265-820
nm, i.e. down to 2.3x below the sampling limit. They were in-plane sector
values relabelled as axial, which is the fallback that was removed.
Committed as-is because the outputs honestly reflect what the library now
reports; how the examples should present an unmeasurable axial direction
is a separate decision. Lateral FSC and both DCR axes are unaffected and
still converge sensibly (XY 366.4 -> 264.5 nm). PSNR/SSIM shifts (39.93
-> 41.26 dB) come from the even-PSF backprojector fixes in this series.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
The sectioned FSC and DCR backends each validated angle_delta and then built the same 0-90 degree edge array from the returned sector count. Fold both steps into _sector_edges so the two cannot drift on sector geometry while continuing to agree on the count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…edges _calculate_fsc_sectioned_hist recomputed the normalization Nyquist itself and kept an index-unit branch for spacing=None that measured max(n // 2) or min(n // 2). Since radial_edges started routing spacing=None through _spacing_or_unit, its radii come back in cycles per pixel, so the two were in different units: on a (16, 32, 32) volume max_freq was 8.0 against a 0.5 grid, and the "frequency" axis handed to callers topped out at 0.06 instead of ~1. The factor cancels between the frequency axis and _normalization_spacing, so reported resolutions were unaffected, but the exposed curve was not and spacing=None disagreed with spacing=1.0 -- the equivalence the module README documents. Read the Nyquist off r_edges[-1] instead: radial_edges spans exactly 0..kmax for the same shape, spacing and use_max_nyquist, so the value cannot drift out of step with the radii it normalizes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
_fsc_extract_resolution derives everything it needs from max_freq via _normalization_spacing; spacing_list survived as a required keyword with a docstring entry and two call sites that no code path read. Remove it so a reader is not tracing a physical-spacing argument through a function that has no spacing left in it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
Two full-size intermediates were computed unconditionally and then mostly discarded: - radial_bin_id and sectioned_bin_id allocated the boolean overflow mask on every call but read it only under exclude_overflow, which no production caller sets. On a 512^3 volume that is a 134 MB array per call. - remove_touching_objects rebuilt `a != 0` and `b != 0` inside all 13 offset iterations. The label image is not written until after the loop, so the background mask is loop-invariant: 26 sub-volume comparisons collapse to one full-volume pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
- segment_utils._binary_fill_holes hand-rolled the CPU/GPU import dispatch that cubic.scipy already performs, in a module that imports the proxy as _ndimage two hundred lines above and uses it for median_filter and find_objects. The local comment claimed cupyx must be imported conditionally, but dispatch_device_call already imports lazily and gates on CUDAManager. Verified CPU/GPU parity, including the structure kwarg. - ssim_elements open-coded ms_ssim._crop_slice, down to the rationale for treating pad == 0 as a no-op -- the same latent bug paid for twice. - average_precision carried `counts` purely to hold iou.shape across one statement; it is None exactly when iou is, so unpack iou.shape directly. - feature.morphology_correlations guarded on .any() before a row-count check that already covers the empty case, duplicating the NaN arm. - _iter_label_boxes claimed to be shared by the post-processing filters when only remove_thin_objects calls it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
A flat FSC crossing cannot fall below the sampling limit because no Fourier shell exists past Nyquist. Both axial corrections break that guarantee: the 1/cos(theta) projection and the Koho eq. (5) multiplier each rescale a crossing that *was* on the grid, so the product is unbounded. On the astrocyte stack that reported 0.4395 um axial against a 2 * 0.3 = 0.6 um limit, while DCR -- which is structurally bounded -- gave ~874 nm for the same data. Add axial_floor_factor (default 2.0), applied to the Z spacing captured *before* any resampling: interpolating Z refines the grid but adds no information, so the band limit does not move. A finer result becomes nan with a warning naming the sector and the limit. 0.0 disables the check. Only z needs this; xy is reported as measured, so the frequency grid already bounds it at 2 * spacing_xy. Also fix test_fsc_resolution_respects_two_pixel_floor, which asserted the Z result against the *lateral* floor (2 * spacing[2] = 0.13) instead of the axial one (2 * spacing[0] = 0.4), so every value between the two floors -- exactly the range the projection lands in -- passed. Refs: Koho et al. 2019 (d_pixel <= d_min/(2*sqrt(2))), Rieger et al. 2024, Diebolder et al. 2015 (conical FSC is qualitative, may reflect sampling), Descloux et al. 2019 / ImDecorr (k_c searched over linspace(0, 1)). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…table Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…trocyte stack The stack is sampled at 0.3 um in Z, so sectioned FSC cannot report an axial value: 0.4395 um is below the 2 * 0.3 = 0.6 um Nyquist floor and now comes back as nan. Both notebooks consumed z as a number. deconvolution_iterations_3d: - average the two directions with nanmean, so the stopping criterion degrades to lateral-only instead of propagating nan and never converging; - raise the threshold to 2.0 nm/iteration. The previous 1.0 was applied to an average that included a frozen axial value, which halved every delta; the criterion now converges at iteration 20 (|delta| = 1.94 nm), lateral 366.4 -> 228.6 nm; - add a note on why axial is nan here, pointing at DCR (974 -> 874 nm), which is bounded by construction and matches the ~0.8-0.9 um expected for NA 0.95. deconvolution_wb_backprojector_3d: annotate the axial panel instead of drawing an empty axes, and only add a legend where there are artists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…l floor Text outputs are unchanged: the pollen stack is sampled at 0.25 um in Z, so its axial results (4.3781 / 2.6197 um, and 697.7 nm for the binomial split) all clear the 0.5 um floor. Committed so every notebook's outputs come from the same library state; the diff is re-execution churn in the plot bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
_apply_cutoff_correction divides the raw resolution by _calibration_factor, whose reciprocal is the curve Koho et al. (2019) fit to d_min(ref)/d_min(co1) (Supplementary Fig. 3). That ratio is >= 1 by construction: the checkerboard split's diagonal shift compresses the FRC/FSC curve, putting the one-image crossing at a higher radius than the two-image reference, so the correction can only coarsen. But the fit is an exponential centred on b = 0.98 -- it passes 1.0 at r ~ 0.925 and reaches 1.82 at r = 1 -- so a crossing on the band edge was divided by a factor > 1 and reported up to 45% below the Nyquist period. The paper's calibration points stop near r ~ 0.85 (their coarsest pixel size, 113 nm), where the ratio is already ~1.0, so that whole region is extrapolation. miplib divides by the same unbounded factor. Clamping makes the reported 1 / (root * kmax * factor) bounded below by 1 / kmax for both directions, since analysis.py already range-checks root <= 1. That matters most for xy, which has no floor guard of its own: the astrocyte stack in deconvolution_iterations_3d reported a converged lateral 228.6 nm against a 337.5 nm sampling limit, and nothing downstream caught it. Not the cause, though it was my first suspect: neither axial correction can refine z. 1 / cos(theta) is >= 1 for every sector the cascade can use, and the Koho eq. (5) factor 1 + (anisotropy - 1) * |cos(theta)| is >= 1 whenever z_spacing >= xy_spacing. The sub-Nyquist value arrives before them. Two existing assertions had been written around the bug, slackening their floor by _calibration_factor(1.0); both are now the strict floor and pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…lation Corrects the axial-floor section, which attributed sub-Nyquist z values to the two axial corrections. It has the sign backwards: 1 / cos(theta) and the Koho eq. (5) multiplier are both >= 1 for every sector the z cascade can use, so neither can make z finer. The floor is a downstream net over an upstream defect in the one-image calibration, and it does not protect xy at all -- the claim that "xy is reported as measured, so the frequency grid already bounds it" was simply false before the factor was clamped. Adds a section on the calibration itself: what the factor is, why its reciprocal is >= 1 by construction, where the paper's fitted points end (r ~ 0.85, read off Supplementary Fig. 3), and what a crossing past r ~ 0.925 means -- the curve only decorrelates at the band edge, so the image is sampling-limited rather than resolution-limited, and a criterion that crosses earlier is the better read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
…l notes deconvolution_iterations_3d: - the converged lateral resolution was 228.6 nm against a 337.5 nm sampling limit; with the calibration factor clamped it is 351.1 nm; - drop the invented 2.0 nm/iteration threshold for the -1 nm/it that Koho et al. (2019, Fig. 3c) define as "most of the resolution gain has been made". The criterion now converges at iteration 8. Their stricter -0.2 nm/it is never reached within this iteration budget; - the note claimed the axial direction is unmeasurable because of the 0.3 um Z sampling. Wrong cause: the 38 degree sector only crosses 0.143 at the band edge, and after the Koho eq. (5) correction that lands at 0.58 um, below the 0.6 um floor. What the Z sampling does violate is Koho's adequacy criterion, d_pixel <= d_min / (2 sqrt 2); - add that the same criterion is violated laterally, which the note did not say: 366 nm at 162.5 nm pixels is 2.25x oversampling against the 2.83x required, where the paper's own RL demonstration (Supplementary Fig. 5) runs at 4.5-7.6x. That is why the lateral track only spans ~29 nm here instead of the wide ramp in Fig. 3b. deconvolution_wb_backprojector_3d: the axial panel annotation also blamed the 0.6 um Nyquist floor, but that guard never fires on this stack -- the warning is "No FSC threshold crossing in any Z-dominated sector", so there is no axial cut-off to invert at all. Say that instead. resolution_estimation_2d, resolution_estimation_3d, split_comparison_frc_fsc: text outputs unchanged -- every crossing there sits well inside the band, so the clamp is inert. The pollen reproduction still gives 0.5856 / 4.3781 um against a published 0.59 / 3.91. Re-executed so all notebooks come from one library state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
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.
Repository-wide bug sweep: 77 bugs fixed across all seven modules, plus stale-code removal and simplification.
Every bug below was reproduced by running the code before being fixed, and each has a regression test that fails without the fix. Test suite goes 401 → 756 passing.
ruff check,ruff format --checkandmypy --ignore-missing-imports cubic/all clean, both locally and viauv run(the toolchain CI uses). Verified on an A40, so the CuPy paths genuinely execute rather than skip — CI has no GPU and skips 107 of them.One caveat on the numbers quoted below: they were measured against scikit-image 0.26.0, while CI resolves 0.25.2. The library handles both (the
remove_small_objects/remove_small_holesshims detect which kwarg the installed version accepts), but DCR curve shapes differ enough between them that one test had to assert a version-independent invariant instead of a fixture-specific value.Silently wrong results
These returned plausible numbers rather than failing, so no test caught them:
metrics/feature.pymorphology_correlationsslicedcorrcoefbylen(features), butextract_featuresexpands vector properties. The diagonal read true-vs-true entries and never touched the prediction — identical inputs returned{'area': 0.921, 'centroid': 0.212}instead of1.0.metrics/spectral/radial.pyradial_bin_idtreated all-1.0 spacing as index units whileradial_edgesstayed physical, sospacing=1.0clipped every non-DC voxel into the last bin (1 of 32 occupied).frc_resolution→nan,dcr_resolution→inf,spectral_pccsilently unweighted.metrics/spectral/frc.pymetrics/spectral/frc.py_frc_datasetkept the empty innermost bin, whose log-domain quotient landed at exactly1.0. That fabricated point made the analyzer read curves as starting above threshold, so every binomial FRC returned NaN.preprocessing/backprojector.py(S-1)/2whereifftshiftputsS//2at DC (mask not Hermitian,np.realdiscarding a large imaginary part), andf[::-1]mapsj → -j-1on even axes so the flipped PSF was not the conjugate OTF. Imaginary leakage5.32e-1 → 1.46e-17.preprocessing/backprojector.pygaussianbranch usedfwhmdirectly, sores_flagandi_reshad no effect — all three modes returned bit-identical arrays.metrics/ms_ssim.pynp.padpreserveduint8soi1p * i1pwrapped mod 256, while skimage'sgaussianrescaled by 1/255 againstdata_range=255.ms_ssim(u8, u8+5, data_range=255)→ 0.7446 vs a 0.9998 reference.metrics/skimage_metrics.pyssimeroded with width 7, but skimage's default withgaussian_weights=Trueis 11, leaking out-of-mask pixels: 0.99721 where the correct footprint gives exactly 1.0.feature/texture.pysegmentation/segment_utils.pycleanup_segmentationcast touint16, silently wrapping past 65535 — with 70000 objects distinct labels collapsed and 2 pixels became background._remove_small_holesfilled area<= Non CPU but< Non GPU, so identical calls diverged per device.Code that had never worked
five_crop_resolution/grid_crop_resolution— both passed 2D arrays to the 3D-onlyfsc_resolutionand raisedIndexErrorunconditionally. Exported in__all__, zero tests, broken sinceadc190a. Theirxzoutput was additionally always NaN because reflect-padding to square replicated data 2–16×, holding FRC above threshold; unpadded, per-slice finite counts go 0/8 → 8/8, 1/32 → 31/32, 3/64 → 64/64.Image.to_cpu()and cross-deviceto_same_device—to_device(..., "CPU")usednp.asarray, which CuPy rejects. There was notests/test_image.pyat all.remove_touching_objects— failed on every GPU array (host footprint meeting a CuPy image) and every 2D array (cube(3)is always 3D).cellpose_segmentcalls it unconditionally.AttributeError.tukey_windowon a singleton axis,random_crop/get_xy_block_coordson 2D,fill_label_holes(**kwargs),fill_holes_slicer/select_nonempty_patcheson CuPy,decon_xpy(mask=..., pad_size_z>0),plot_utilson GPU arrays — all raised.Input mutation
richardson_lucy_xpdidimage *= mask, andimg_as_floatreturns the same object for float input, so it destroyed the caller's array. Same class of bug inremove_large_objects,remove_touching_objects,remove_thin_objects,cleanup_segmentation's hole fill, andfill_holes_slicer.Performance (all byte-identical)
remove_touching_objects, 500 labels in 32×128×128find_objects, 1000 labels in 64×256×256Stale code and tooling
ruffselected onlyDandI, so pyflakes never ran and unused imports/variables had accumulated. Now["D", "F", "I"], with example notebooks keeping only the import-order and docstring rules. Removed ~250 lines of never-instantiated spectral iterators and their private helpers, the unusedget_frc_options/nitems, deadpreserve_rangere-casts, unreachable proxy cache checks, commented-out code, and changelog prose from docstrings.rtreeadded to themeshextra —extract_mesh_featuresraisedModuleNotFoundErrorfor anyone following the documented install.The
cubic.metrics.frcshims are removed: they warned that the old path would go in 0.8.0, and v0.8.0 shipped with them still present.Breaking changes
This is not a patch release. ~24 removed public symbols, 13 signatures with a parameter removed,
masknow keyword-only onpcc/nrmse/psnr/ssim,extract_featuresreturning three values,clahe(kernel_size→n_tiles), theimage_statspercentile key spelling,morphology_correlationskeying by expanded column names, and ~14 inputs that were previously accepted and now raise.Several fixes also change previously-returned numbers — that is the point of them, but it means re-running an analysis will not reproduce prior values. Named explicitly, since some are easy to miss:
remove_thin_objectschangedsize_z <= min_zto< min_z, matching its docstring and name.min_zis a defaulted parameter, so every existing caller changes: atmin_z=2, objects spanning 1/2/3/4 Z planes went from keeping{3,4}to keeping{2,3,4}.band_limited_pcc,band_limited_ssimandspectral_pccnow all returnnanfor degenerate input (previously a mix of0.0,nan, and an accidental value), matchingpcc.clear_xy_bordersno longer relabels in 3D, socellpose_segmentoutput ids can now be gapped where they used to be contiguous 1..N.get_true_featuresno longer derives feature ranges from the ground truth whenfeature_ranges is None. The old path normalized the true side but not the prediction, putting them on different scales;cosine_median(..., precomputed_gt_feature_df=...)returns different numbers as a result.frc_from_sums(signed=True)replaced four copies of the FRC formula, three of which were previously unsigned (FRC.execute,DirectionalFSC.execute, and the checkerboardhistpath). Signed is the miplib form and the crop fix depends on it, but it is a default change: ~0% impact where the curve stays positive, up to 2.3% on axial FSC, and it matters most at the noise floor.fsc_resolutionaxial numbers move by1 / cos(theta)— 1.27× at the default reporting sector — unlessresample_isotropic=True, which is unchanged.angle_delta=90now raises instead of returning the same value forxyandz, andzisnan(with aRuntimeWarning) when no sector below 45° crosses the threshold, where it used to fall back to an in-plane sector.radial_edges(shape, spacing=None)returns edges in cycles per pixel (0 to 0.5) rather than index bins (0 to n/2), anduse_max_nyquistis now a no-op without spacing. Normalized frequency axes are unchanged, so reported resolutions on square input are unaffected._normalization_spacinglost itsspacingparameter, and_fsc_extract_resolutionitsz_factorparameter;_resample_isotropic_for_fscreturns a 3-tuple.scikit-imageminimum moves to>=0.25(morphology.footprint_rectangle, which replacessquare/cubeand is the only variant cuCIM provides).One coverage caveat: the
_remove_small_holesCPU off-by-one fix is only live on scikit-image 0.26, since 0.25.2 takes the olderarea_threshold=branch. CI resolves 0.25.2, so that specific shift is not exercised by CI.Review notes
image_utils.pad_imagebefore its spectral and segmentation callers) and are best reviewed individually; the series is intended to be evaluated as a whole.BackProjector.m, which assumes odd-sized PSFs (odd shapes stay bit-identical, verified 41/45 combinations againstHEAD); andms_ssim's float64 cast shiftsMicroMS3IMby 1.4e-07, toward the upstream fixture (4.172e-07 → 3.732e-07).dcr_resolutionreturnsnanrather thaninfwhen no interior cutoff exists. On an axially unsmoothed volume the curve legitimately rises to Nyquist, so there is nothing to measure — the oldinfwas a divide-by-zero escaping, and the existing test asserted only>= 0, whichinfsatisfied.🤖 Generated with Claude Code
https://claude.ai/code/session_01BtaDGLN7D3L6Xn2YXAbbck
Review outcome
Reviewed by GitHub Copilot plus four independent adversarial reviewers, each given a disjoint slice, told to assume the fixes themselves were buggy, and asked to revert fixes and confirm the corresponding tests actually fail.
Copilot raised 3 comments; all 3 were false positives, refuted against the installed upstream cellpose source and by executing the code (
np.copytodoes dispatch to CuPy;cubic.segmentation.segment_cellposeis a valid path;shape[:3]matches upstreammodels.py:384byte-for-byte).The adversarial pass found 9 genuine defects, two of them inside fixes in this PR, all now fixed in later commits:
sigma=1.5— atsigma=3.0(window 23) it returned 0.2067 where 1.0 is correct. The same defect one level out from the one being fixed.mask2meshpadded to close border surfaces but never subtracted the pad, offsetting every vertex by one voxel — 4.0 physical units atspacing=(4,1,1).nanmedianaggregation dropped tiles that measured nothing at any plane, with no warning: 8 of 32xymeasurements silently excluded while the aggregate read fully finite.small_value = 1e-6 * image.max()produced aninffloor for an image containinginf, yielding an all-NaN volume with no exception.random_crop's 2-D fix left the slice hard-coded to axes 1/2, so 4-D input silently returned an empty array where it used to raise.listreturns on the host and silently discarded GPU output arguments.max_label=0(semantics inverted), and float label images infind_objects.It also found two tests that could not fail: reverting the
_endpoint_indicesfftshift-origin fix left the entire suite green, andtest_remove_touching_objects_without_backgroundpassed against the pre-rewrite implementation with a rationale that was factually wrong. Both now have teeth.Verified clean under attack, with the riskiest change first:
remove_touching_objectsrewrite — byte-identical to the per-label dilation across 74 fixtures (diagonal-only, 3D corner-only, chains, border-touching, non-contiguous ids, no-background, single-voxel, 40 random), with an exhaustive proof that the offset filter keeps(3**n - 1)/2antipodal representatives, i.e. full 26-connectivity. Two deliberately-injected wrong rewrites were both caught by its test._frc_dataset's zero-count drop keepscorrelation/frequency/points-x-bin/thresholdaligned by construction — verified on both backends and sectioned FSC across 20 configurations._intersection_over_union's contract change is numerically identical on non-background entries, on both devices, and all three call sites agree.preserve_range=Trueprovably does not alter float32/float64 results; the deleted_flatten_elementsreduces bit-identically, including on non-contiguous cropped views.graycopropsto 1.8e-15, and the direction stride cannot blend directions.The two escalated issues, now resolved
Both changed published resolution numbers, so they were held for an explicit
decision rather than patched quietly. Both are now fixed:
spacing=Nonenow bins identically tospacing=1.0. The index-unit gridscaled each axis by its own length (
fftfreq(n) * n), so a constant-radiusring was an ellipse in physical frequency once the axes differed in length —
one bin averaged unlike frequencies together.
(64, 128)gave 32 bins forNoneagainst 64 for1.0, with different per-voxel assignments;(192, 256)with
zero_padding=Falsegave 2.9573 vs 3.9966.radial_bin_idalreadydisagreed with
radial_k_grid(which used cycles per pixel), anddcr.pyworked around it by substituting
[1.0] * 3, so the cycles-per-pixel readingwas already the intended one. All
Nonepaths now route through_spacing_or_unit. Square input, and therefore every padded top-levelFRC/FSC path, is unchanged.
Sectioned FSC
zis now projected onto the Z axis by1 / cos(theta).Neither the old nor the intermediate behaviour was right. A volume with an
ideal axial cutoff at 0.5 cycles/µm (true axial period 2.0 µm), at spacing
[0.5, 0.19, 0.19], measures per sector:The cascade reports the 38° sector, not the Z-aligned cone, so the measured
quantity is a shell radius
|k| = k_z / cos(theta)rather thank_z:z_factor,1 + (0.5/0.19 − 1)·cos 38°= 2.286: 3.13 µm — 1.56× too coarse1 / cos(theta): 1.737 µm1 / cos(theta)applied to the 22°/38°/52°/68° sectors gives 1.745 / 1.737 /1.606 / 1.670 — four independent sectors collapsing onto one value, with the
residual gap to 2.0 matching the threshold-crossing bias
xyshows (0.629 vs0.667 expected). Note it carries no spacing term, which is why the
anisotropy ratio was never the right coefficient.
Two consequences: sectors at or above 45° are XY-limited, so their band edge
says nothing about the axial cutoff and
1 / cos(82°)would inflate it 7× —the cascade no longer falls back to them, and
zisnanwith a warninginstead. And
angle_delta=90now raises, since one sector spanning 0–90°cannot separate axial from in-plane at all (it previously reported the same
number for both keys).
resample_isotropic=Truekeeps the Koho eq. (5) correction instead. Myfirst attempt removed it from that path too, and re-running
resolution_estimation_3d.ipynbcaught the regression: axial resolution onthe Fig. 4b pollen stack went from 4.363 µm (12% above the published 3.91) to
2.022 µm (48% below), while XY stayed at 0.586 against a published 0.59.
Interpolating Z up to isotropic voxels adds no information, so the real axial
band limit stays at the original Z Nyquist while the grid now runs to the XY
one —
1 + (anisotropy − 1)·|cos(theta)|converts back, which is a differenterror from the sector-geometry projection. The two are not combined (doing so
gives 5.54 µm, further from the paper than either). So the geometric
projection applies only where the grid already carries true per-axis Nyquists,
which is where the oracle above measures it; the resampled path is restored
bit-for-bit to the paper-validated behaviour.
FourierCorrelationAnalysis.execute(z_correction=...)is untouched too, sothe eq. (5) form remains directly available for reproducing miplib numbers.
Found while re-running the notebooks, not fixed here
Nothing validates the reported axial resolution against the axial sampling
limit. On the astrocyte crop at spacing
(0.3, 0.1625, 0.1625)the 38-degreesector reports Z = 324 nm against a
2 * 0.3 um= 600 nm floor — 1.85x finerthan the data can carry.
test_fsc_resolution_respects_two_pixel_floorcannotcatch it: it asserts
result["z"] >= 2 * spacing[2], the XY pixel floor(0.13 um), rather than
2 * spacing[0](0.4 um). Pre-existing, and independent ofthe sector cascade — the sub-45-degree sector produces it directly.
Consequence for the examples: both deconvolution notebooks now report
nanaxially, because no sector below 45 degrees crosses the threshold on a 30-plane
stack (halved to 15 by the checkerboard split before resampling).
deconvolution_iterations_3d's FSC stopping criterion goes from iteration 41 toiteration 0, and
wb_backprojector_3d's axial subplot is empty. The previouslycommitted curves were unphysical for the same reason as above — they plotted
265-820 nm against that 600 nm floor — and by elimination came from the >=45-degree
fallback, since the sectors the cascade still uses produce no crossing at all.
angle_deltaof 30 and 45 were both tried; both also givenan(45 makes XYnantoo), so this data cannot support an axial FSC measurement by this method.A floor guard (
nanplus a warning below2 * spacing_z) would have caught theoriginal fallback bug and both notebook regressions automatically. Not added here
because it moves axial numbers a third time in this series and wants an explicit
decision, same as the two above.
Still inconsistent, deliberately
Sectioned DCR reports its
zsector's shell radius with no projection. Thesame geometric argument would suggest
1 / cos(22.5°)(an 8% shift), but DCR'sestimator is a cumulative decorrelation over a 45° cone rather than a per-shell
correlation, so the projection is not a clean transfer and I have no ground-truth
oracle for it. Left alone rather than applying an unvalidated correction.