Skip to content

feat(dynacell): score interior-only foreground metrics by default - #527

Merged
alxndrkalinin merged 9 commits into
dynacell-modelsfrom
dynacell-eval-fg-interior-default
Oct 14, 2026
Merged

alxndrkalinin merged 9 commits into
dynacell-modelsfrom
dynacell-eval-fg-interior-default

Conversation

@alxndrkalinin

@alxndrkalinin alxndrkalinin commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

What changes

  • eval.yaml pixel_metrics.foreground: enabled: true, feather_sigma_um: 0.0 (hard GT interior mask). source: smooth_otsu and smooth_sigma_um: null (per-target smoothing default) are unchanged. Set feather_sigma_um: null for the per-target feathered recipe (metrics.FOREGROUND_SIGMAS_UM).
  • Comment above the block, the evaluation README defaults table, and the reporting/tables.py comment no longer say "off by default".
  • New test test_default_eval_scores_interior_only_foreground composes the real eval.yaml the way the CLI does (_compose_eval_cfg) and checks enabled is True, feather_sigma_um == 0.0, and that _foreground_settings resolves to {source: smooth_otsu, smooth_sigma_um: 1.0, feather_sigma_um: 0.0} for er. The existing FG tests build configs with OmegaConf.create and cannot see the packaged default, so this test is the only guard on it.

Why

Spotlight v2 judged models on interior-only FG metrics. Whole-image pixel metrics are dominated by background voxels, so the FG columns should be in every eval by default.

What I verified in metrics.py

  • foreground_weight builds the weight from the GT alone (smooth, then Otsu, then an optional feather). With feather_sigma_um=0 the Gaussian feather is skipped, so the weight is the binary Otsu mask (1 inside, 0 outside). Constant z-planes (padding) are 0.
  • foreground_pixel_metrics never zeroes the images. The weighted means, std, gain, PCC and MSE are all sum(weight * x) / sum(weight), so background voxels get weight 0 and drop out. _weighted_ssim uses normalized convolution: the local statistics come from weight-weighted windows, and the SSIM map is averaged with the weight, so only interior voxels count.
  • The whole-image columns are computed separately and are unaffected.
  • Every target_name in the tracked configs (er, membrane, mitochondria, nucleus) has an entry in FOREGROUND_SIGMAS_UM. The smooth_sigma_um: null default therefore resolves and does not raise.

No FOREGROUND_METRICS_VERSION bump

Its rule is to bump only when an FG_* value moves for a given recipe, and none does. The recipe (source, sigmas, version, spacing) is already stamped as pixel_foreground in metrics_provenance.json, so the cache gate catches the new default without a bump. A bump would also invalidate every existing FG cache, including the Spotlight-v2 FG buckets.

One-time cost

On its next non-forced run, any existing save dir without a matching pixel_foreground stamp recomputes its final-metrics pass once. That is about 48 single-condition canonical leaves. Grouped leaves already set force_recompute.final_metrics. The Spotlight-v2 FG buckets from generate_fg_buckets.py always write feather_sigma_um explicitly, so the new default does not change their recipe.

Note: the experiment-local trackh/make_eval_leaf.py (untracked, under experiments/2026-09-24_spotlight-v2/) writes only foreground.enabled: true. Leaves it generates after this merges will use the hard mask instead of the per-target feather. Pin feather_sigma_um there if those runs need the feathered recipe.

Merge conditions — met (2026-10-13)

  1. feat(dynacell): 2D benchmark on one focus slab — slab3-only predict + focus2d eval #526 merged (996adb4). dynacell-models was merged into this branch in 5867793, with no rebase and no force-push, and the tests were rerun (below).
  2. Spotlight-v2 Phase-1 evals closed on 2026-10-10. Phase-B evals that run after this merge get hard-mask FG_* columns in their canonical dirs. Whole-image and seg metrics are unchanged. The FG re-score buckets keep their explicit feather_sigma_um.

Review

  • /simplify: 4 fixes.
    • The otsu test is back on a target with no sigma default.
    • One parametrized FG-default test now covers the evaluator's own target list (segmentation.EVAL_TARGETS, which is also used by prepare_segmentation_model).
    • The nucleoli and lysosomes sigmas are derived from mitochondria.
    • The default is stated once.
  • /code-review: 2 fixes, the copied (not aliased) sigma dicts and the docs describing the default weight as the hard interior mask. Eight findings were skipped:
    • Intentional and documented: the one-time cache recompute, the code-side enabled fallback for hand-built configs, and the nucleoli/lysosomes defaults Copilot asked for.
    • Measured not to bite:
      • real test volumes are 44 (iPSC) and 48 (A549) planes, so the 11-voxel FG SSIM window fits; whole-image SSIM already fails below 7;
      • FG-on evals ran on A40 with no OOM.
    • Out of scope for this PR: recipe checks in reporting, and a GT weight cache.
  • Copilot: re-reviewed at 26a38cd with 0 open findings.

Tests (head 26a38cd)

  • The FG-default test fails under two mutations, each checked: setting feather_sigma_um: null in eval.yaml fails 6/6 targets, and dropping the nucleoli/lysosomes defaults fails 2/6.
  • Targeted eval and reporting tests: 475 tests, 0 failed, 0 errors.
  • Full applications/dynacell (tests + src): 4802 tests, 4603 passed, 2 failed, 0 errors, 197 skipped (junitxml).
  • CI has no dynacell job, so these counts are the dynacell evidence.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Gwn9hwgT8uJSoWaCxCAm8J

Spotlight v2 judged models on foreground-limited pixel metrics, because
whole-image pixel metrics are dominated by background voxels. Turn the
FG_* columns on in eval.yaml with a hard GT interior mask
(feather_sigma_um: 0.0); feather_sigma_um: null keeps the per-target
feathered recipe from metrics.FOREGROUND_SIGMAS_UM.

The weight map is built from the GT alone and only weights the scoring;
neither image is zeroed, so whole-image columns are untouched.

FOREGROUND_METRICS_VERSION is not bumped: no FG_* value moves for a given
recipe, and the recipe is already stamped in metrics_provenance.json, so
an existing save dir recomputes its final metrics once on its next run.

Hand-built test configs omit the block, so a compose test of the real
eval.yaml pins the new default.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new default raises for supported nucleoli and lysosome evaluations because their smoothing defaults are undefined.

1 open finding
What changed in this PR

Enables hard-mask foreground metrics by default in DynaCell evaluations.

Changes:

  • Enables unfeathered foreground scoring in eval.yaml.
  • Updates documentation and reporting comments.
  • Adds a composed-default regression test.
File Description
test_evaluate_compose.py Tests the resolved ER foreground defaults.
reporting/​tables.py Updates the foreground-default comment.
evaluation/​README.md Documents the new defaults.
_configs/​eval.yaml Enables hard-mask foreground metrics.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

With FG metrics on by default, an eval whose target has no
FOREGROUND_SIGMAS_UM entry raises before scoring, and nucleoli and
lysosomes are accepted targets without one. They take mitochondria's
sigmas (a region mask around compact or punctate bodies), marked
untuned because DynaCell has no GT for them. A composition test now
resolves the default FG settings for every accepted target; the
null-sigma error test moves to a target outside the table.

New entries move no existing FG value, so FOREGROUND_METRICS_VERSION
stays.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Its stated merge conditions remain unresolved, including the pending #526 integration and Spotlight-v2 evaluations.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

alxndrkalinin and others added 7 commits October 13, 2026 22:08
test_otsu_source_resolves_no_smoothing_sigma switched to lysosomes to
prove the otsu source needs no smooth_sigma_um default. This PR gives
lysosomes a default, so the check passed whether or not otsu looks one
up. Use golgi, as the sibling resolution test already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gwn9hwgT8uJSoWaCxCAm8J
…r's list

Merge the two new FG-default tests into one parametrized test that
checks the exact resolved settings per target. The expected smoothing
sigma comes from FOREGROUND_SIGMAS_UM, and the target list from the new
segmentation.EVAL_TARGETS, which prepare_segmentation_model now uses
too, so a target added to the evaluator is covered automatically.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gwn9hwgT8uJSoWaCxCAm8J
…mito

They take mitochondria's sigmas by design; copying the literal would
let them drift silently if mitochondria is retuned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gwn9hwgT8uJSoWaCxCAm8J
eval.yaml named FOREGROUND_SIGMAS_UM twice in one comment, and the
tables.py comment copied the default, which is how its old "off by
default" went stale. The default now lives in eval.yaml and the README
defaults table only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gwn9hwgT8uJSoWaCxCAm8J
The previous derivation bound both entries to mitochondria's dict, so an
in-place edit of one (a test monkeypatch or a sensitivity sweep) changed
all three.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gwn9hwgT8uJSoWaCxCAm8J
eval.yaml and the README metrics table still called the weight a soft
map; under the new default (feather_sigma_um 0) it is binary, feathered
only when feather_sigma_um is positive or null.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The stated merge dependencies remain pending, including an overlapping change from PR #526.

0 open findings

🧠 Review effort: Balanced

@alxndrkalinin
alxndrkalinin merged commit e29f8f8 into dynacell-models Oct 14, 2026
27 checks passed
@alxndrkalinin
alxndrkalinin deleted the dynacell-eval-fg-interior-default branch October 14, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants