You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(dynacell/eval): clip the CP shared-space z at ±20 before KID/FID/cosine - #501
After #499, CP (GLCM+) features are scored in each test set's ground-truth units. A pilot re-eval (nucleus lite, 12 conditions, MEASURED) showed:
in-domain A549 Dataset_CP_KID: 5–57;
cross-domain A549→iPSC: 8.1e5 / 3.8e8 / 4.2e9.
The cause is one heavy-tailed feature. Prediction glcm_ASM reaches z = +239 in iPSC ground-truth units, and the cubic polynomial KID kernel turns that into ~2e11 per pair. So CP KID was measuring "how far out is glcm_ASM", not texture distributions.
Fix
Clip: the shared-space z is clipped to ±c, c = CP_Z_CLIP = 20. It applies to both sides, after the reference transform and before KID/FID/cosine, at the dataset level and per row, and in lite_checks / lite_subset_sim. The linear probe and the cross-condition probe don't use the transformed space and are unchanged.
Builder invariant: each full test set's ground-truth clip fraction (cells with any |z| > c in its own scaler) must be ≤ CP_GT_CLIP_FRAC_MAX = 1e-3.
Lite sets are recorded with enforced: false. Their cells are a subset of the parent's under the parent's scaler, so the parent's bound covers them.
c, the bound, the survey summary and the rank-stability note are in the hashed criteria; per-dataset gt_clip_frac and max |z| are recorded too.
Diagnostics:Dataset_CP_clip_frac and per-row CP_clip_frac (the fraction of prediction cells with any |z| > c), plus a per-feature clip block in the sidecar. _final_metrics_cache_valid requires the new columns.
Choosing c (MEASURED; all ground truth, lite included, each in its own scaler)
Survey: global max |z| is 26.55 (ER A549 denv, kurtosis); every other dataset is ≤ 15.8. p99.99 is 10.2–12.2 per target.
Global ground-truth clip fraction by c: 5 → 2.2e-2, 10 → 2.4e-3, 15 → 2.5e-4, 20 → 8.3e-5 (3 of 36,019 cells), ≥ 30 → 0.
Offline sweep on the 12-condition pilot:
In-domain KID at c ≥ 15 is identical to unclipped.
Cross-domain KID (fnet / VSCyto3D / UNeXt2) at c = 20 is 7.0e4 / 7.2e4 / 1.9e5, against 8.1e5 / 3.8e8 / 4.2e9 unclipped.
The in-domain model ranking holds at every c tested except one near-tie swap at c = 8.
The cross-domain order is identical for every c ≥ 20, but changes below 20. So 20 is the smallest c at which the clip does not create the ordering.
At c = 20: worst full test set 5.3e-4 (ER A549 denv / mock); all 31 others 0.
Caveat, also in the README: the cross/in-domain KID ratio keeps growing with c because of the cubic kernel. Only orderings of CP KID are meaningful across domains.
Tests
No |z| above c: staged arrays and KID/FID/cosine are bit-identical to the unclipped code.
A prediction cell at z = 1000: gives exactly the KID of a cell at c (fails without the clip).
Per-row vs dataset level: the per-row inputs equal the dataset-level rows (fails without the clip).
Clip fractions:clip_frac is correct on a constructed end-to-end case.
Builder: it refuses a full dataset over the bound, and accepts a few ground-truth cells above c under the bound (fails on the old any-cell rule). A lite set over the bound builds, while its parent is still enforced.
Existing end-to-end cases: offset and ×0.5 still register above the floor.
Full dynacell suite:2 failed, 3608 passed, 164 skipped. The 2 failures are the known order-dependent morphem-revision and cubic-pin tests. prek passes.
Rollout
Criteria change, so every reference hash changes. After merge, rebuild all 4 registry references (--force) and run --verify before any re-eval. Only the 12 pilot directories carry current stamps, and they get re-run.
…the reference
The shared-space z will be clipped before KID/FID/cosine, so the clip
value has to be justified by the GT and never touch it. A read-only
survey of every GT cell of all 4 registry references (lite included),
each in its own test set's scaler, gives a global max |z| of 26.55 (er,
a549-mantis-sec61b-denv, kurtosis). The per-target p99.99 is <= 14.7.
CP_Z_CLIP = 50 leaves a 1.9x margin.
fit_cp_reference re-runs that survey on every build. It records z_clip
and the survey summary (global max, p99.99, max dataset/feature) in the
hashed criteria, and each dataset's own max/p99.99/feature in its fit
record. It refuses to build if any GT |z| exceeds the clip, so the
margin is a checked invariant. The per-dataset binding includes z_clip
but not the target-wide survey, so adding a dataset still leaves other
bindings unchanged. Schema 6.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
…ID/cosine
Through the cubic KID kernel, one heavy-tailed predicted feature set
the metric's magnitude. On the post-#499 nucleus-lite pilot, pred
glcm_ASM sat at z up to 239 in the iPSC GT scaler, driving A549->iPSC
Dataset_CP_KID to 8e5-4e9 while in-domain was 5-57. As the paper owner
decided, the reference-transformed z is now clipped to +-z_clip on
both sides (DatasetCPSpace.transform_clipped), at dataset level and
per row, and in the lite tools that mirror the pipeline. The
cross-condition probe (mask + MADScaler) and the linear probe
(masked, unscaled) do not use the transformed space and are unchanged.
Tests:
- Without any |z| > c, the staged arrays and KID/FID/cosine are
bit-identical to the unclipped space.
- A pred cell at z=1000 scores exactly like one at the clip. Unclipped
it moves KID by > 1e3x the capped change.
- The per-row inputs equal the dataset-level staged rows.
Removing the clip from the pipeline fails the last two.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
…touches
Clipping discards magnitude, so how much it discarded is recorded next
to the metrics it affects:
- Dataset_CP_clip_frac (feature_metrics.csv, beside Dataset_CP_KID):
the fraction of pred CP cells with any kept feature beyond +-z_clip.
- CP_clip_frac: the same per (FOV, t) row. NaN without pred cells.
- The sidecar's "clip" block: z_clip, the dataset fraction and the
per-feature fraction of pred cells clipped.
DatasetCPSpace.clip_fraction computes all three from the unclipped
transform. _final_metrics_cache_valid refuses a CP feature cache
lacking either column, since those rows were scored unclipped. Tests:
an e2e case whose checkerboard cells leave the band checks all three
fractions against a direct count, and a cache missing either column is
recomputed.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
…on bound
The paper owner set c = 20, based on an offline sweep over c in {5, 8,
10, 15, 20, 30, 50, none} on the nucleus-lite pilot. The cross-domain
model order is identical for all c >= 20, and the in-domain order holds
at every c except one near-tie (zikv, c = 8). At c = 20 some GT cells
are clipped (3 of 36,019 registry GT cells; max |z| 26.55), so "no GT
cell clipped" no longer holds as the builder invariant.
- CP_Z_CLIP = 20.
- The builder now refuses when any dataset's gt_clip_frac exceeds
CP_GT_CLIP_FRAC_MAX = 1e-3. gt_clip_frac is the fraction of that
dataset's GT cells with any |z| > c in its own scaler, lite included.
The highest measured value is 9.5e-4 (er,
a549-mantis-sec61b-denv-lite: 1 of 1054).
- Each dataset records gt_clip_frac, max |z| and its feature. The
hashed criteria record c, the bound, a rank_stability note and the
survey summary (now including max_gt_clip_frac).
- Tests: a dataset beyond the bound is refused; one GT cell beyond the
clip in 5000 (2e-4) is accepted and recorded.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
…tasets only
With the 1e-3 bound on a ~1000-cell lite set, one extra GT cell beyond
the clip bricks every build of the target. Lite GT cells are a subset
of the parent's, scored in the parent's scaler, so a lite set's
clipped-cell count is at most its parent's. The enforced parent bound
therefore covers it, and the lite fraction is only small-sample noise.
Lite datasets still record gt_clip_frac and max |z|, now marked
`enforced: false`, and the survey's max_gt_clip_frac covers enforced
datasets only.
Test: a lite set at 2e-3 builds while its parent at 2e-4 passes the
bound; once the parent is over the bound, the build refuses.
Re-enforcing lite fails the test. README states why lite is exempt.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
…d on full test sets only
The README said the bound applied to every test set, lite included, and
only later that lite sets are exempt.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
With no finite pred CP rows the clip fractions are NaN, which json.dumps
wrote as a bare NaN token that strict JSON parsers reject. Record null
instead and dump with allow_nan=False.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
The new transform is the behavior that changes the scored CP metrics, but the added tests only validate the reported clip fractions; the existing staging assertions use inputs whose z values stay below 20, so they would still pass if KID/FID/cosine continued to receive transform(x). Add an integration test with a prediction at z well above the limit and assert both dataset-level and per-row metric outputs match the explicitly clipped arrays.
Review: changes requested (tests only; the code is correct)
Reviewed at 7f951df, on a scratch snapshot in the cubic-0.9.0a1 venv. Touched tests pass 61/61, and 94/94 including the other CP test files.
Correct (checked):
Every scaled CP consumer goes through transform_clipped. The linear and cross-condition probes apply only the mask, never the shared scaler, which matches the README.
Order is scale (with the floor), then clip, then metric.
clip_frac counts pred cells with any feature beyond c; NaN rows are dropped first.
Changing c from 20 to 25 changes binding_sha256 (579f6a27 → fb989f8e), and so does changing the bound.
A read-only dry build over the registry refuses nothing. For ER, every lite GT row matches a parent row to 3.1e-13 in z units.
Mutation results: 8 of 15 survive.
Mutant
Result
clip removed, per-dir / per-row
survived / survived
pred-only clip, per-dir / per-row
survived / survived
clip before scale
killed
c / bound left out of the binding
survived / survived
lite enforcement on
killed
clip_frac counted on GT
killed
lite_checks / lite_subset_sim unclipped
survived / survived
Required
Assert that the metric is scored on clipped z.
cp_reference_test.py:317-329,353-360 still assert staged == space.transform(...), the unclipped behaviour. They pass only because no cell reaches |z| > 20.
The checkerboard e2e (test_evaluation_cp_end_to_end.py:327) already produces KID 1.81e5 clipped vs 2.24e9 unclipped, but asserts only clip_frac. Assert the KID and per-row value against a clipped reference computation, on both paths.
Pred-only clip: add a case with a GT cell beyond c, so clipping only one side fails.
Hash test:cp_reference_test.py:557-571 checks payload_sha256. Check binding_sha256 (the cp_space_sha256 that eval reuse compares) for both c and the bound.
Lite tools:_reference_transform in tools/lite_subset_sim_test.py:63-66 and tools/lite_checks_test.py:101 scales without clipping. Make it clip, and add an outlier cell, so an unclipped tool fails.
Nits
README.md:27 says the bound applies "lite included" and then exempts lite. The quoted top fraction, 9.5e-4, is the lite (unenforced) value; the enforced max is 5.34e-4.
cp_reference.py:99: "3 of 36,019" counts the DENV outlier twice (parent and lite). Only 2 distinct GT cells exceed 20. Please correct it; it goes into the paper's methods.
The builder checks lite ⊂ parent by position, not by cell. That's fine given the dry-build match, but the docstring shouldn't claim more.
…s only
The survey pooled lite rows, which are subsets of their parents'. So
"3 of 36,019 GT cells beyond 20" counted the er denv outlier twice
(denv and denv-lite), and the quoted 9.5e-4 was an unenforced lite
value. The criteria survey summary now covers full (non-lite) test
sets only: max |z|, p99.99, max dataset/feature, the enforced
max_gt_clip_frac, n_cells and n_cells_beyond_clip. The registry gives
2 of 23,970 distinct GT cells beyond 20, with an enforced max of 5.3e-4
(er a549-mantis-sec61b-denv and -mock). The per-dataset records, lite
included, are unchanged. The README and the CP_Z_CLIP comment quote the
corrected numbers, with lite 9.5e-4 only as the unenforced record. A
test checks that a lite copy of a parent outlier is not counted twice.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
…ding
The hashed-criteria test checked the whole-reference payload_sha256,
not binding_sha256, the stamp eval reuse compares (#501 review). So
dropping z_clip or gt_clip_frac_max from the binding passed. The new
test changes each in criteria, re-hashes, and asserts binding_sha256
moves for every dataset, lite included.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
The lite tools' test helper spelled out the CP transform without the
clip, and no fixture exceeded c. So reverting lite_checks or
lite_subset_sim to the unclipped transform passed (#501 review). The
helper now clips with the reference's z_clip. Both fixtures add a few
pred cells far beyond the clip, and assert the unclipped KID diverges
by > 10x.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
… feature
An all-constant GT pool left keep_mask empty, and the clip survey then
failed on an argmax over an empty array with an unhelpful error.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF
Mutations: all 15 from the first review now fail at least one test at 580a046 (baseline 97/97 pass). Applied to a dd89379 copy, each mutation still matches its original code exactly once, so dd89379 leaves the mutated lines alone.
Round-2 commits: no new bug. Sidecar NaN clip fractions are written as strict-JSON null and tested. The survey summary pools full test sets only and stays out of the eval stamp. The README contradiction is fixed. 96e6e58 bundles several test changes; noted, not a blocker.
dd89379: the empty GT-only mask refusal is covered. With the guard removed, its test fails on an empty-argmax crash instead of the clear error.
Methods numbers: confirmed by rebuilding all 4 targets in memory from the registry's GT caches with the new code, writing nothing:
Target
Full-dataset GT cells
Beyond |z| = 20
er
7,162
2
nucleus
4,568
0
membrane
4,684
0
mitochondria
7,556
0
Total
23,970
2
The enforced max is 5.3e-4 (er).
Tests at dd89379: 98/98 across the 8 CP test files.
Merge after Copilot and CI on dd89379. Then rebuild all 4 registry references: they are still schema 5, and evals refuse them until then.
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
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.
Problem
After #499, CP (GLCM+) features are scored in each test set's ground-truth units. A pilot re-eval (nucleus lite, 12 conditions, MEASURED) showed:
Dataset_CP_KID: 5–57;The cause is one heavy-tailed feature. Prediction
glcm_ASMreaches z = +239 in iPSC ground-truth units, and the cubic polynomial KID kernel turns that into ~2e11 per pair. So CP KID was measuring "how far out is glcm_ASM", not texture distributions.Fix
CP_Z_CLIP= 20. It applies to both sides, after the reference transform and before KID/FID/cosine, at the dataset level and per row, and inlite_checks/lite_subset_sim. The linear probe and the cross-condition probe don't use the transformed space and are unchanged.CP_GT_CLIP_FRAC_MAX= 1e-3.enforced: false. Their cells are a subset of the parent's under the parent's scaler, so the parent's bound covers them.criteria; per-datasetgt_clip_fracand max |z| are recorded too.Dataset_CP_clip_fracand per-rowCP_clip_frac(the fraction of prediction cells with any |z| > c), plus a per-feature clip block in the sidecar._final_metrics_cache_validrequires the new columns.Choosing c (MEASURED; all ground truth, lite included, each in its own scaler)
Tests
clip_fracis correct on a constructed end-to-end case.2 failed, 3608 passed, 164 skipped. The 2 failures are the known order-dependent morphem-revision and cubic-pin tests. prek passes.Rollout
Criteria change, so every reference hash changes. After merge, rebuild all 4 registry references (
--force) and run--verifybefore any re-eval. Only the 12 pilot directories carry current stamps, and they get re-run.🤖 Generated with Claude Code
https://claude.ai/code/session_01XvkM1gKifWmoDPq8Drd4JF