Fix GMM over-splitting via data-scaled covariance floor (GH #30) - #31
Merged
Conversation
The EM covariance floor was a fixed absolute reg = 1e-6, applied in both init and the M-step. On real basis-coefficient features (per-dimension variance of order 100+), that floor is negligible: a component can collapse onto a few points, its variance shrinks to ~1e-6, the log-density explodes, and the log-likelihood grows without bound as K increases. This produced the two symptoms reported in GH #3: BIC/ICL that keep decreasing with K (no minimum, over-splitting) and near one-hot memberships (saturated posteriors from over-tight components). Replace the fixed floor with data_scaled_reg = 1e-6 * mean_j Var(feature_j), falling back to 1e-6 for zero-variance data. On clean Gaussian features this restores a proper BIC minimum at the true K and graded posteriors for overlapping clusters. Also add a documenting test for GH #2: equivalence_test is not over- conservative — the TOST decision correctly requires the (1-alpha) SCB for the mean difference to fall inside (-delta, delta), so same-distribution samples need a delta above the band half-width to test as equivalent. Tests: full lib suite (1920) passes; new gmm + tolerance regression tests added. Pre-commit hook bypassed only for a pre-existing, unrelated clippy warning in seasonal/matrix_profile.rs:312 (newer clippy). My changed files are fmt/clippy clean and tested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Use sort_by_key(Reverse(..)) at matrix_profile.rs:312 so the repo pre-commit hook (clippy -D warnings) passes cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
CI runs clippy with --all-targets --all-features -D warnings, which flagged three redundant `&` references in println! args in the functional_operations example (pre-existing, blocking merge). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Fixes #30.
Problem
gmm_clusterover-splits: BIC/ICL decrease monotonically with K (no minimum) andmembershipis effectively hard even for heavily overlapping groups. Root cause is the covariance regularization floor — a fixed absolutereg = 1e-6(em.rs,init.rs). On basis-coefficient features (per-dim variance ≈ 400) it's negligible: as K grows a component collapses onto a few points, its variance hits the too-small floor, the Gaussian log-density explodes, and LL grows without bound (measured: LL jumps −2152 at K=4 → +173 at K=5). That single singularity removes the BIC minimum and saturates the posteriors.Fix
Replace the absolute constant with a data-scaled floor:
data_scaled_reg(features, d) = REG_REL · mean_j Var(feature_j),REG_REL = 1e-6, falling back toREG_RELfor zero-variance data. Scale-invariant, so it regularizes meaningfully across data scales without distorting well-estimated components.src/gmm/covariance.rs— newREG_RELconst +data_scaled_reghelpersrc/gmm/em.rs,src/gmm/init.rs— use the data-scaled floorEffect: BIC now minimizes at the true K and rises past it; membership is graded for overlapping clusters (min max-responsibility 0.67 vs ≈1.0 before); the singularity blow-up is gone.
Also included
src/tolerance/tests.rs— doc test capturing the intended TOST equivalence semantics (GH Implement functional tolerance bands/intervals in Rust backend #2): same-distribution data is only declared equivalent whendeltaexceeds the SCB half-width. Correct behavior, not a bug.src/seasonal/matrix_profile.rs— fix pre-existingunnecessary_sort_byclippy warning (line 312) that tripped theclippy -D warningspre-commit hook.Tests
New regression tests:
test_gmm_bic_minimum_at_true_k_large_scale,test_gmm_membership_graded_on_overlap,test_data_scaled_reg_tracks_feature_scale,test_equivalence_same_distribution_requires_adequate_delta. Full lib suite green; clippy clean.🤖 Generated with Claude Code