Skip to content

Fixes #35, fixes #39, fixes #23 - #92

Merged
guijacquemet merged 4 commits into
mainfrom
claude/validation-batch-2026-11-11
Nov 11, 2026
Merged

guijacquemet merged 4 commits into
mainfrom
claude/validation-batch-2026-11-11

Conversation

@guijacquemet

Copy link
Copy Markdown
Contributor

Validation batch on main 9db7c14. One commit per issue; each was reproduced with a failing test on main before the fix.

Fixes #35, fixes #39, fixes #23.

#35: exact percentiles in the written method name (fb39641)

  • Reproduced: NormalizationSettings(percentile=(1.000001, 99.8)).method_name and (1.000002, 99.8) were both 'percentile 1-99.8', and Parameters.from_columns read a row fitted with one under the other's settings. 12 new tests failed on main.
  • Fix: each percentile is now written as the shortest positional decimal that reads back as exactly the same float, with no exponent form. This is the new normalization.percentile_name, which percentile_range and NormalizationSettings.method_name both use, so the name is built in one place. -0.0 is written as 0. The name now tells apart any two percentile settings, so the existing from_columns check on normalization_method refuses rows whose percentiles differ by any amount.
  • What changes: only the normalization_method text, and only for percentiles that have more than six significant digits or are below 0.0001. For example percentile 1-99.8 becomes percentile 1.000001-99.8, and percentile 1e-05-50 becomes percentile 0.00001-50. The default name percentile 1-99.8 and every other ordinary name stay the same. No fitted offset or scale changes.
  • Old records: with no backward compatibility (owner's ruling of 10 November 2026, contract §9.1), a row written before this change with affected percentiles has the old short name. from_columns refuses it with the existing message, for example "Column normalization_method is 'percentile 1-99.8', but the settings use 'percentile 1.000001-99.8'. Read the parameters with the settings of the run that wrote them." A test covers this. The fix is to fit again. There is no reader or converter for old names.
  • Contract: §3.1 used to say names are formatted with :g. It now gives the exact-decimal rule. This changes a written contract rule, so it needs owner approval (AGENTS §2.6).

#39: NormalizationParameters.apply validation (5225fd5)

  • Reproduced: clip=(1, 0) turned every value into the same number, and dtype=np.uint8, bool, complex64, object, float16 and 'U4' were all accepted. 16 new tests failed on main.
  • Fix: before any pixel is normalized, apply raises a named ValueError in these cases:
    • clip is not two numbers: "clip must be two numbers, (low, high); got ...";
    • a limit is not finite: "clip limits must be finite numbers; got ...";
    • low >= high: "clip must be (low, high) with low < high; got low=1.0, high=0.0 ...". Equal limits are refused too, because they would give every pixel the same value. This is now written down.
    • any result type other than float32 or float64: "dtype must be float32 or float64; got 'uint8' ...".
  • Valid calls give bit-identical results. The input is never modified, and non-finite pixels behave as before. Contract §1.2.4 and §9.1 are updated.

#23: RGB limited to 8- and 16-bit integers (f25b605)

  • Reproduced: check_rgb accepted int32, uint32, int64 and uint64 with three channels, even though its own message (the contract's §9.1 message) asks for 8- or 16-bit colour channels. fit of such RGB pixels or plane rows went through. 5 new tests failed on main.
  • Fix: I chose option A from the issue. check_rgb now accepts only uint8, uint16, int8 and int16 (new RGB_DTYPES; byte order does not matter) and refuses the others with the existing contract message, which names the count and type. Reason: apply_rgb repeats CSBDeep's float32 arithmetic, which cannot hold every value above 2**24. Contract §5 and §9.1 are updated; plan §5 already said 8- or 16-bit.
  • Not compatible: RGB images of 32- or 64-bit integers are now refused. Save them as 8- or 16-bit, or use image type fluorescence.
  • Open question for the owner: signed int8 and int16 RGB are still accepted, as they were before. They are 8- and 16-bit and exact in float32. Say if they should be refused as well.

Output identity

  • tools/dump_outputs.py: 1723cafb4ff21fc92146725f5fdbdf0d8e3589d13569d68169d072d2eaddfff9 (1788 lines). --corpus: 8bb7c3c1a7e1a4f3f1f19b5b921b3e45e3644258bdc9e771f0cd49314772256c (4764 lines). Both are byte-identical to main 9db7c14 (cmp).
  • The 400 golden fits (tests/test_fit_goldens.py) pass unchanged.

Gate

python tools/check.py passes locally on Python 3.13: ruff format, 1627 passed, 2 skipped, code rules and ruff check. The 2 skips are tests/test_rgb.py:80, because CSBDeep is not installed locally; CI runs them.

Overlap with draft PR #90 (Stage 3)

Follow-ups (not done here)

  • morphobricks-preprocessing contract §13 says RGB needs "an integer pixel type". Its _rgb_problem uses check_rgb, so it gets the new refusal, but that wording should become 8- or 16-bit there. I did not modify another repository.
  • checks.py still uses :g for percentiles in warning text. This is only display text, not identity.

Generated by Claude Code

normalization_method wrote each percentile with :g (six significant
digits), so percentile=(1.000001, 99.8) and (1.000002, 99.8) both wrote
"percentile 1-99.8" and Parameters.from_columns accepted parameters
fitted with one under the other's settings. Each percentile is now the
shortest positional decimal that reads back as the same float
(normalization.percentile_name, shared by percentile_range and
NormalizationSettings.method_name); -0.0 is written 0.

Names of percentiles with at most six significant digits and at least
0.0001 (the default "percentile 1-99.8" among them) are unchanged. Rows
written earlier with other percentiles carry the short name and are
refused by from_columns with the existing message; no backward
compatibility. No fitted number and no dump output changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
NormalizationParameters.apply passed clip and dtype through unchecked:
clip=(1, 0) gave every pixel the same value, and an integer dtype
truncated and wrapped the normalized values. apply now refuses, with a
ValueError naming the value and before any pixel is normalized, clip
limits that are not two finite numbers with low < high (equal limits
too) and every result type but float32 and float64. Valid calls are
unchanged bit for bit; no dump output changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
check_rgb accepted every integer type although its message, the
contract's, asks for three 8- or 16-bit colour channels, and apply_rgb
repeats CSBDeep's float32 arithmetic, which cannot hold every 32- or
64-bit value. check_rgb, and with it fit of RGB pixels and plane rows,
now refuses int32, uint32, int64 and uint64 with that message, naming
the channel count and type. uint8, uint16, int8 and int16 are accepted
as before with bit-identical results; no dump output changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
The #39 check compared the result type with the native float32 and float64, so
'>f4' and '>f8', accepted before the fix, were refused with a message naming
'float32'. The check now goes by type name, as check_rgb does.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc

Copy link
Copy Markdown
Contributor Author

Independent review: approve, with one small fix pushed (fa4c993)

Reviewed f25b605 against main 9db7c14 in a fresh clone and venv (Python 3.13).

#35, percentile names. percentile_name is correct. I checked 200,000 random percentiles (uniform, tiny, and rounded to 0-8 digits). Every name read back as exactly the same float, none used exponent form, and no two different floats shared a name. Adjacent floats (nextafter of 0, 1, 50, 99.8 and 5e-324) all got distinct names. -0.0 gives 0, and the default percentile 1-99.8 and rgb percentile 1-99.8 are unchanged. NormalizationSettings.method_name and percentile_range share the one function. from_columns refuses the old-name rows with the existing named message, and the tests cover mismatches for fluorescence and rgb.

#39, apply validation. Refusals come before any pixel is read. I tried reversed, equal, NaN, inf, one-element, three-element, scalar, string and complex clips. I also tried uint8, bool, float16, 'U4', object, complex64, longdouble and 'nonsense' as dtype. Each raised a named ValueError, with no other exception type. Valid calls are bit-identical (dump_outputs is identical, below).

  • Defect found and fixed. The new check compared the dtype with the native float32 and float64. That refused dtype='>f4' and '>f8', which main accepts. The message said got 'float32', which is confusing. The fix compares by type name, as check_rgb does, and adds a regression test. The test fails on the PR head without the fix and passes with it.

#23, RGB dtypes. RGB_DTYPES is uint8, uint16, int8 and int16, and byte order does not matter (>u2 is accepted). uint32, int32, int64, uint64, float32 and bool are refused with the contract message naming the count and type. The int8 and int16 question is still open for the owner; I have no objection to accepting them.

Output identity, run myself against main 9db7c14.

  • tools/dump_outputs.py is byte-identical (cmp): 1723cafb4ff21fc92146725f5fdbdf0d8e3589d13569d68169d072d2eaddfff9, 1788 lines.
  • --corpus is byte-identical (cmp): 8bb7c3c1a7e1a4f3f1f19b5b921b3e45e3644258bdc9e771f0cd49314772256c, 4764 lines.
  • Both digests are also unchanged at the new head fa4c993.
  • The 400 golden fits pass.

New tests fail on main. I ran the PR's test files against main's src: 34 failed (the PR counted 33; I did not look into the difference), 210 passed and 2 skipped.

Gate at fa4c993. python tools/check.py exits 0: ruff format, 1631 passed, 2 skipped, code rules and ruff check. Both skips are tests/test_rgb.py:80 (no CSBDeep locally). CI was green on f25b605 (workflow run 91). At the time of writing, CI had not started on the new head fa4c993; please confirm it is green before merging.

Non-blocking: the checks.py :g formatting in warning text is display only, as the PR says. The preprocessing contract §13 wording follow-up is also still open.

Ready to merge once the owner approves the contract §3.1 change (AGENTS.md §2.6).


Generated by Claude Code

@guijacquemet
guijacquemet merged commit 0b35530 into main Nov 11, 2026
8 checks passed
@guijacquemet
guijacquemet deleted the claude/validation-batch-2026-11-11 branch November 11, 2026 07:08
guijacquemet pushed a commit that referenced this pull request Nov 11, 2026
The #91 tests for 64-bit integers and overflowing tail squares and the #92 test of
exact percentile names build PlaneRecords and Parameters; they now pass channel IDs
(PlaneRecord channels=, write_plane_histograms channel_ids=, Parameters channels by
ID). The code-rules baseline drops the two entries the merge left behind.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants