Repository navigation
CP-4: no silent rescale or channel repetition by default in the torch adapters (breaking) - #73
Merged
Merged
Conversation
…ing) PatchTensorConfig.scale_policy has no default: an omitted one (also through PatchTorchDataset/ExportedPatchTorchDataset without tensor_config, and patch_array_to_tensor without config) is a TypeError naming "none" and "auto". A required choice rather than a "none" default, so code written for the old "auto" default stops instead of receiving values up to 65535 times larger without a word (contract §15). channel_policy defaults to "keep"; "repeat_to_rgb" only when named, recorded per item as channel_repeat beside scale_divisor. ScalePolicyDeprecationWarning stays for the named, deprecated "auto". Docs (api_examples, quick_start, contract §3/§15, plan §6.8) name their policies; CHANGELOG has a Breaking entry with migration notes and the downstream calls. Baseline shrinks by three entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet
marked this pull request as ready for review
November 2, 2026 12:40
…CP-4 review) patch_array_to_tensor repeated the channel with its own check while each dataset item's channel_repeat came from channel_repeat(); the tensor now uses channel_repeat() too, as it uses scale_divisor(), so the recorded value is the applied one. Adds a stand-in-torch test that PatchTorchDataset records channel_repeat and keeps 1, 2 and 4 channels with their values (it fails on main). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
…ow on the old default (CP-4 review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Contributor
Author
|
Independent review: merged as Verified
Fixes (small commits, no force-push)
Gate (uv venvs, Python 3.11, head
CI is green with the same three counts. CI has no torch job. §9 figures
Generated by Claude Code |
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.
This PR implements the owner decision of 2026-11-02, the open question from the PR #72 review. The torch adapters stop rescaling and stop repeating channels unless the caller asks.
v0.2.0rc1ships with the old defaults; this change is for the finalv0.2.0. Breaking.Chosen default:
scale_policyis required (no default)Omitting
scale_policynow raises aTypeError. This applies toPatchTensorConfig(),PatchTensorConfig(channel_policy=...),patch_array_to_tensor(..., config=None), andPatchTorchDataset/ExportedPatchTorchDatasetbuilt withouttensor_config. The message names both choices:'none'keeps the values.'auto'(deprecated) divides an integer patch by its dtype maximum. It was the default before v0.2.0.Why required rather than defaulting to
"none": the science rules forbid an implicit normalization change. A"none"default would silently hand code written for the old"auto"default values up to 65535 times larger, which is an implicit change in the other direction. A required choice stops that code with an error, and every caller's choice is then written in its code. Contract §15 anddocs/plan.md§6.8 record this.Behaviour changes
scale_policy: there is no default; omitting it raises theTypeErrorabove. A rescale still happens only with"auto", which is recorded per item asscale_divisor.ScalePolicyDeprecationWarningis kept and repurposed: it now marks the named, deprecated"auto"option, not a default. It is still shown under Python's default filters.channel_policy: the default is now"keep"; before it was"repeat_to_rgb".(1, H, W)."repeat_to_rgb"works only when named. Each item records it aschannel_repeat: 3 when repeated, otherwise 1. It is new, sits next toscale_divisor, and is also available as the publiccellpatcher.torch_dataset.channel_repeat.PatchTensorConfig(scale_policy="none")it is now(1, 4, 4)holding 1000.0, withscale_divisor1.0 andchannel_repeat1.Migration (also in the CHANGELOG):
config.normalization):PatchTensorConfig(scale_policy="none").PatchTensorConfig(channel_policy="repeat_to_rgb", scale_policy="auto"). It warns when it rescales and is recorded per item.PatchTensorConfig(channel_policy="repeat_to_rgb", scale_policy="none").Callers in CellPatcher
torch_sessionand the PyTorch notebook: both already passchannel_policy="keep"andscale_policy="none". No change.notebook_torchgoes throughtorch_session. The other notebooks do not use torch.PatchTensorConfigindocs/api_examples.mdanddocs/quick_start.mdnow names itsscale_policy. The examples that feed RGB/DINO models also namechannel_policy="repeat_to_rgb". OnePatchTorchDatasetexample had notensor_configand now has one.docs/plan.md§6.8, thePatchTorchDatasetdocstring, and AGENTS.md (the delivery bullet and the health snapshot).Tests that fail on old code
The new tests are in
tests/test_deprecations.py. They run without torch through the existing stand-in, so CI runs them:test_an_omitted_scale_policy_is_refused_naming_the_choicestest_the_default_channel_policy_keeps_one_channel_and_its_valuestest_the_torch_datasets_need_a_tensor_config_with_a_scale_policytest_the_old_defaults_are_reachable_by_name_and_recorded_per_item: checksscale_divisor65535,channel_repeat3 and the warning.test_the_live_dataset_records_channel_repeat_and_keeps_any_channel_count(added in review):PatchTorchDatasetrecordschannel_repeatand keeps 1, 2 and 4 channels with their values.Against
main'storch_dataset.py:test_deprecations.py,test_torch_dataset.pyandtest_biological_repeat.py(re-measured in review).Existing tests now name their policies. Tests of the old behaviour use it explicitly (
OLD_DEFAULTS, orrepeat_to_rgbwith"none"). The subprocess test checks that the default-filter warning is still shown for a named"auto".Gate
Local uv venvs, Python 3.11, Linux, head
81edaa0(after review):tools/check.pyexits 0 (full run, in the torch venv).tools/check.py --rulesexits 0..[test].[test,zarr].[test,zarr,notebook].[test,zarr,notebook]+ CPU torch 2.15.0Each environment has 5 more passes than at main (1075 / 1096 / 1124 / 1173). I inspected every skip, and each one is a missing extra:
.[test]: zarr 21, notebook extra 10, torch 10..[test,zarr]: notebook extra 10, torch 10..[test,zarr,notebook]: torch 11.CI is green on
81edaa0with the same three counts. CI has no torch job, so the torch tests and the PyTorch notebook kernel run were run locally only (the 1177 row).§9 figures
src/cellpatcher/torch_dataset.py: 478 lines (425 on main). It is the largest file touched;tests/test_torch_dataset.pyis 592, of which only call sites changed.channel_repeat: 4 executable lines, complexity 1, 3 parameters.PatchTensorConfig.__post_init__: 15 lines, complexity 7 (was 6), 11 statements.test_the_old_defaults_are_reachable_by_name_and_recorded_per_item, 13 lines, 12 statements.patch_array_to_tensornow takes the repetition fromchannel_repeat(review fix): 35 executable lines, complexity 9, 31 statements, 3 parameters.__getitem__gains one line.PatchTorchDataset.__init__gains docstring lines only.undocumented-public-classintorch_dataset.py, and twoline-too-longentries intests/test_torch_dataset.py. No# noqawas added.Downstream calls affected (no other repository edited)
Each repository was fetched and its
origin/mainsearched forPatchTorchDataset,ExportedPatchTorchDataset,PatchTensorConfig,scale_policy,channel_policy,scale_divisorandpatch_array_to_tensor.a8e88df:src/morphoembed/provider_crops.py:59-63,139-144passesPatchTensorConfig(**TENSOR_CONFIG)withchannel_policy="keep",dtype_policy="float32",scale_policy="none",return_metadata=False. Code is unaffected.ROADMAP.md:126-127describes the old defaults (repeat_to_rgb/auto) and needs a doc update in its migration.4e6970f: documentation only.AGENTS.md:245-247already recommendsscale_policy="none".docs/audits/pytorch-encoder-phenome-audit.md:121,200describes the old default RGB replication; it is a dated audit, so it is history. No code calls.ea19bb0, fiberfeatures362f1ba, MorphoObjects2080ff5: no uses.Independent review
b61f4ff:patch_array_to_tensorrepeated the channel with its own check while items recordedchannel_repeat(); it now useschannel_repeat()(as it usesscale_divisor()), so the recorded value is the applied one. Plus the live-dataset regression test (stand-in torch, runs in CI).c01cdf2: CHANGELOG downstream list: MorphoObjects is now2080ff5; the MorphoFeature audit's PT08 row describes the old default.81edaa0: AGENTS.md health snapshot.Not tagged.
https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
Generated by Claude Code