Repository navigation
Bucket A (1/2): refuse corrupt statistics: blocks, counts, ranges, tails, plane identity - #61
Merged
Merged
Conversation
Bucket-A integrity fixes for the statistics side of the brick. Valid inputs give bit-identical outputs; invalid ones are refused with a named error instead of being silently reinterpreted, wrapped or double-counted. - Blocks: every non-empty block must hold the plane's pixel type; a block in the other byte order is converted by value, so big-endian 8/16-bit planes are no longer read as native-endian bytes (#25, #26). - New integrity module: PlaneHistogram and HistogramTail check their invariants when made (int64 non-negative counts, exact unwrapped totals, valid <= total, all-or-none ordered finite limits, feasible tail sums with float64 overflow accepted only where the range allows it) (#45, #46, #47). - plane_histograms.csv: a header-only table, overflowing counts and repeated plane_id rows are refused; write_plane_histograms, scope_groups, fit and image_summaries refuse repeated planes too (#43, #45, #48). - PlaneRecord: z and t must be whole numbers 0 or above (#44). - PooledHistogram adds counts as integers (exact above 2**53) and refuses a group above 2**63 - 1 pixels (#31). - scope_groups refuses empty or non-text repeat-group labels and two biological repeats of one condition in one repeat group (#42). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
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.
Bucket-A integrity fixes, statistics side: refuse corrupt pixel blocks, counts, ranges, tails and plane identity. The other half (parameters, scope column, unchecked channels) is #62. The two PRs merge cleanly into each other.
Fixes #25, fixes #26, fixes #31, fixes #42, fixes #43, fixes #44, fixes #45, fixes #46, fixes #47, fixes #48
Changes
Blocks ([Sad Catapella N1][Correctness] Big-endian int8/int16 histogram blocks are reinterpreted as native-endian pixel values #25, [Sad Catapella N1][Correctness] Block histogram accepts dtype-mismatched pixel arrays and can silently change pixel cardinality #26):
integrity.checked_blocksis used in every pass ofhistogram.py. A non-empty block of another pixel type (kind, width or signedness) raisesValueErrornaming the block and both types. A block in the other byte order is converted by value.>u2/>i2planes were previously read as native-endian bytes, which gave min/max 256/768 for the values 1..3.Histogram invariants ([Sad Catapella N5][Scientific integrity] CSV accepts negative and int64-overflowing histogram bin counts #45, [Sad Catapella N5][Correctness] Nonempty histogram with all intensity limits missing passes CSV validation #47), new
integrity.py:PlaneHistogram.__post_init__checks the following:bin_countnon-negative values;[2**62]*4+[3]is caught);Tails ([Sad Catapella N5][Scientific integrity] CSV parser accepts NaN, infinity and impossible nonempty tail moments #46):
HistogramTail.__post_init__refuses:n·ssq < sum².PlaneHistogramalso checks that each tail's mean and sum of squares are possible within its range (relative slack 1e-9). A float64 overflow of the sum of squares ([Sad Catapella N2][Scientific correctness] Tail second moments and pooled standard deviation can overflow despite finite true spread #33) is accepted only where the range makes it possible.CSV:
plane_idrow is refused, naming both lines ([Sad Catapella N4][Scientific integrity] Duplicate plane_id rows are accepted and silently double-counted in normalization #43);CsvFormatErrorinstead of an uncaughtOverflowError;The old
_consistent_rangehas moved into the histogram invariants, and_recordis down from 72 to 56 lines.Repeated planes ([Sad Catapella N4][Scientific integrity] Duplicate plane_id rows are accepted and silently double-counted in normalization #43):
write_plane_histograms,scope_groups,fitandimage_summariesalso refuse repeatedplane_ids (records.duplicate_planes).Coordinates ([Sad Catapella N4][Data integrity] PlaneRecord and CSV reader accept negative plane coordinates; direct records also accept fractional coordinates #44):
PlaneRecordz/t must be whole numbers ≥ 0.Nonestill becomes 0; bool, float (including 1.0), text and negative values are refused. The CSV reader refuses-1.Pooling ([Sad Catapella N2][Correctness] PooledHistogram loses exact pixel counts through float64 weighted bincount #31):
PooledHistogrammerges counts with integernp.add.at, not float64bincountweights, so counts above 253 stay exact. A group above 263-1 pixels is refused. The float policy ofWeightedHistogramis now documented.Repeat groups ([Sad Catapella N4][Scientific identity] scope_groups accepts two biological repeats of one condition in the same paired group #42):
scope_groupsrefuses two biological repeats of one condition mapped to one repeat-group label, and empty or non-text labels. It does not normalize labels. Unused mapping entries are still ignored.Docs: contract §1.2.2 invariants, §4.2 pairing, §6.2, §9.1 rows; AGENTS map.
One existing test changed:
test_failed_write_leaves_previous_file_untouchedused to write the same plane twice, which is the [Sad Catapella N4][Scientific integrity] Duplicate plane_id rows are accepted and silently double-counted in normalization #43 defect. It now writes two different planes.Evidence
tests/test_integrity.pyand two tests intest_scopes.py. Run againstmain(f784c95), 70 of the 91 integrity tests and all 4 new scope tests fail. The remaining 21 are controls that check valid inputs are still accepted.f7604ef1…c091417on main, this branch and main + both PRs. It covers:WeightedHistogram;fit,applyandfrom_columnsround trips for 4 methods × 4 scope/pooling/per-time combinations;ruff check .passes;ruff format --check .passes;ruff check --ignore-noqa .reports 3 E402 intests/data/make_v2_fixture.py, the same 3 as on main;pytest: 569 passed, 2 skipped (csbdeep not installed locally; the CI csbdeep job passed), 0 failed.tools/check.pyortools/code_rules.pyin this repo.histogram.pywas already over the limit at 863 lines and is now 864. The_small_integer_histogramrefactor offsets most of the new lines.PooledHistogramof 200 uint16 planes takes 483 ms here against 509 ms on main (best of 5). This is a sanity check, not a performance claim.🤖 Generated with Claude Code
https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc