Repository navigation
Make pooled means and standard deviations independent of CPU threads (#67) - #69
Merged
Merged
Conversation
…67) The pooled mean and standard deviation (and so every zscore fit) summed with np.dot, which OpenBLAS splits across its threads above 10,000 elements, so the last bits of a fit changed with OMP_NUM_THREADS/OPENBLAS_NUM_THREADS. sums.weighted_sum multiplies element by element (each product rounded once, as np.dot did) and adds the products with math.fsum: the correctly rounded sum, identical for any thread count, BLAS or platform. All seven np.dot calls of the library go through it (PooledHistogram and WeightedHistogram mean/std, the tail-aware mean/std, and the per-value tail sums). The other reductions (np.sum, cumsum, bincount, add.at) are single-threaded NumPy loops. Scientific precision: tools/dump_outputs.py changes on 86 of 1,788 lines (134 numbers, pooled mean/std and zscore offset/scale only), by at most 3.7e-14 relative; 89 numbers by 1 ulp, the largest 317 ulps (int64 planes near 2**53). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a
guijacquemet
marked this pull request as ready for review
November 5, 2026 08:01
…error (#67) math.fsum raises ValueError on inf + -inf and OverflowError on an overflowing sum, where np.dot returned nan or inf and fit refused the group by name. Fall back to NumPy's single-threaded sum in those cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a
Contributor
Author
|
Independent review: merged as Finding fixed ( Verified:
Follow-up (pre-existing on main, not this PR): a non-finite z-score scale is refused with "its intensity spread is zero (all pixels have the same value)", which is wrong for an overflow. Generated by Claude Code |
guijacquemet
pushed a commit
that referenced
this pull request
Nov 5, 2026
Resolves the README conflict: the v0.3.4 notes carry #69, and the migration note no longer says valid inputs give bit-identical results; mean, std and z-score change in their last bits (at most 3.7e-14 relative). docs/plan.md says the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WX6rdMV1r5jUPUR94kxV5a
Merged
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.
Fixes #67.
Problem.
PooledHistogram/WeightedHistogrammeanandstd(and so everyzscorefit, at every scope) summed withnp.dot. OpenBLAS threads a dot product above 10,000 elements and adds the partial sums in a thread-dependent order, so the fitted offset/scale changed in their last bits withOMP_NUM_THREADS/OPENBLAS_NUM_THREADS(MorphoFeature #130 finding: MorphoEmbed's per-plane z-score fit gave…030vs…531).Fix. New pure module
sums.py:weighted_sum(weights, values)multiplies element by element (each product rounded once, asnp.dotdid) and adds the products withmath.fsum, the correctly rounded sum. Same bits for any thread count, BLAS, NumPy version or platform; never less accurate than before. No new dependency.Every thread-dependent reduction (audit): all seven
np.dotcalls insrc/go through it:PooledHistogram.mean/std,WeightedHistogram.mean/std,_mean_with_tails,_std_with_tails, andhistogram._table_tail(the per-value tail sums). The remaining reductions (np.sumpairwise sums in_TailSums/integrity,cumsum,bincount,add.at,unique) are single-threaded NumPy loops whose order does not depend on threads. Nomatmul/einsum/linalg/scipyuse.Scientific precision (bits change).
python tools/dump_outputs.py: digestd82a9f97…on main →dfd5e740…here. 86 of 1,788 lines differ, all pooledmean/stdorzscoreoffset/scale; 134 numbers in total, max relative difference 3.7e-14; ulp histogram: 1 ulp ×89, 2 ×12, 3 ×1, 4 ×3, 5 ×11, 9 ×2, 10 ×1, 16 ×2, 17 ×2, 41 ×1, 52 ×3, 76 ×6, 317 ×1 (the large ones are int64 planes whose values are near 2**53). Every other output (percentiles, median, MAD, min/max, histograms, CSV text, other methods) is bitwise unchanged. The new values are the correctly rounded sums of the same products, so they are at least as accurate as the old ones. README "Unreleased" and contract §2 table say so.Tests (
tests/test_sums.py):test_fits_are_bitwise_equal_with_one_and_many_threads: runs the pooled/weighted/tail mean and std andzscorefits (image and biological_repeat scopes, >10,000 pooled values) in two subprocesses withOMP/OPENBLAS/MKL_NUM_THREADS= 1 and 4; asserts identical hex. Fails on main (tails std…d07fvs…d07e).test_weighted_sum_is_exactly_rounded_and_order_free: equals an exactFractionsum, rounded once, and is permutation-invariant (fails against annp.dotstand-in:…462402vs…462654).test_pooled_sums_are_correctly_rounded.Gate:
python tools/check.py(Python 3.12): exit 0; 630 passed, 2 skipped (csbdeep not installed:tests/test_rgb.py:78). Code-rules baseline unchanged (new modulesumsdeclaredpure).Release: v0.3.4 is not tagged. Merge this into main before the release PR #68; #68 must then be brought up to date with main (its README turns "Unreleased" into the v0.3.4 section, so it must carry this entry and the migration notes must say the z-score/mean/std bits change, instead of "valid inputs give bit-identical results"). MorphoFeature's pin then moves to v0.3.4 with the finals.
Generated by Claude Code