Fix 1/N floor in effective-rank collapse score after mean-centering

[rubenG1009]

Fixes #8996

Description

_effective_rank_score mean-centers the embedding matrix (L290), which caps its attainable rank at min(N-1, D), but normalized the effective rank by min(N, D) (L298).

Since the score is 1 - eff_rank/max_rank, that imposed a hard floor of 1/N whenever N <= D: isotropic embeddings at N=4, D=768 scored 0.2503, and two maximally-spread points scored exactly 0.50 ("50% collapsed"), where the correct answer is ~0. The metric could not report "no collapse" in precisely the regime it was designed for. _per_class_rank is affected more, since per-class N is smaller and the floor is therefore larger.

The observed floor matches 1 - (N-1)/N = 1/N exactly.

Why min(N-1, D)

Mean-centering forces the rows to sum to zero, which is a linear dependency: if you know N-1 rows, the last one is determined. So rank(centered) <= min(N-1, D). Normalizing by the attainable maximum removes the artifact.

The fix is a no-op for N > D, where min(N-1, D) and min(N, D) both equal D. Verified: a 1024x768 matrix scores 0.1234 before and after.

Tests

Three tests added to TestEffectiveRankScore, all RED on dev and GREEN after the fix (except the third, which is a regression guard and passes both ways):

• test_effective_rank_no_collapse_when_n_leq_d - 4x768 isotropic: 0.2503 -> ~0.0004
• test_effective_rank_two_distinct_samples_report_no_collapse - 2x768: 0.5 -> 0.0
• test_effective_rank_unchanged_when_n_gt_d - 1024x768 unchanged at 0.1234 (proves no regression)

Full suite: 48 passed, 2 skipped (sklearn not installed locally).

Note on the modified test

test_formula_matches_manual mirrored the old normalization inline (min(20, 16)). Its fixture is N=20 > D=16, so it passes either way, but it would otherwise encode the old formula. Updated to min(20 - 1, 16) - flagging it explicitly rather than changing an assertion silently.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.

[coderabbitai]

📝 Walkthrough

Walkthrough

The effective-rank collapse score now normalizes by min(N - 1, D), reflecting the rank limit introduced by mean-centering. Existing manual expectations were updated, and tests cover high-dimensional samples, two maximally separated samples, and the unchanged N > D behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main fix: removing the 1/N floor in effective-rank collapse after mean-centering.
Description check	✅ Passed	The description matches the template well with a clear summary, linked issue, and completed change/test checklist.
Linked Issues check	✅ Passed	The changes implement the requested min(N-1, D) normalization and add tests for N<=D, two-sample, and N>D behavior.
Out of Scope Changes check	✅ Passed	The PR stays focused on effective-rank normalization and related regression tests, with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/metrics/test_embedding_collapse.py`:
- Around line 231-251: Replace the inline comments in
test_effective_rank_no_collapse_when_n_leq_d,
test_effective_rank_two_distinct_samples_report_no_collapse, and
test_effective_rank_unchanged_when_n_gt_d with Google-style method docstrings
describing the test inputs, expected score behavior, return value, and any
raised exceptions as applicable. Preserve the existing test logic and
assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: de1aab56-425f-4a98-9a56-cfe1a5dd5bd2

📥 Commits

Reviewing files that changed from the base of the PR and between 3a458fe and f76d5b1.

📒 Files selected for processing (2)

• monai/metrics/embedding_collapse.py
• tests/metrics/test_embedding_collapse.py

[vikashg]

Mathematically correct fix. Mean-centering introduces one linear dependency (rows sum to 0), so rank(centered) \u2264 min(N-1, D) \u2014 normalizing by min(N, D) was imposing a spurious 1/N floor. The tests clearly demonstrate the bug (0.25 and 0.50 for isotropic inputs) and the fix. No-op for N > D confirmed. LGTM.

[vikashg]

Mathematically correct fix. Mean-centering introduces one linear dependency (rows sum to 0), so rank(centered) \u2264 min(N-1, D) \u2014 normalizing by min(N, D) was imposing a spurious 1/N floor. The tests clearly demonstrate the bug (0.25 and 0.50 for isotropic inputs) and the fix. No-op for N > D confirmed. LGTM.