Jax kmeans implementation, sklearn removal from main deps - #461
Open
kylejcaron wants to merge 16 commits into
Open
Jax kmeans implementation, sklearn removal from main deps#461kylejcaron wants to merge 16 commits into
kylejcaron wants to merge 16 commits into
Conversation
Handles empty clusters without producing NaN centroids, stops on an inertia tolerance rather than exact centroid equality, and selects the best of n_init restarts by inertia.
_squared_distances expanded ||x - c||^2 to ||x||^2 - 2 x.c + ||c||^2 directly on the raw coordinates. That expansion is not offset-stable in float32: once the data sits a few orders of magnitude from the origin, the two large terms nearly cancel and the result can come out negative, even though a true squared distance can never be negative. Since kmeans picks the restart with the lowest inertia, a spuriously negative inertia from this bug would silently win over a correct restart, defeating the point of n_init. Fix centers both the samples and the centroids on the samples' mean before doing the expansion, which keeps the intermediate terms small and numerically well-behaved, and clamps the result to zero as a numerical safety floor. Added a regression test that reproduces the failure at a 1e5 offset and checks that inertia stays non-negative and the well-separated blobs are still partitioned correctly. Also fixed two minor issues from review: _update_centroids used to increment its per-cluster counts with a bare Python float, which JAX warns about casting into an integer-dtype array; it now uses a value built from X's own dtype. And added a comment on the restarts test clarifying that the single-vs-many inertia comparison is empirical (they draw from independent key streams) rather than a hard invariant.
The kmeans initialization path had no test coverage for any model.
Replaces the inline scikit-learn KMeans calls in every emissions class that supports kmeans initialization.
Nothing in the library imports scikit-learn now that k-means runs in JAX; only the demos and notebooks still need it.
When initializing a LogisticRegressionHMM's emission biases with kmeans, a cluster whose assigned binary emissions happened to be all 0 or all 1 produced a cluster mean of exactly 0.0 or 1.0. Passing that straight through the logit gave an infinite bias, even though this is a different (and more common) case than the already-handled empty cluster. Now the cluster mean is clipped just inside (0, 1) before the logit so the bias always stays finite. Added a regression test that constructs emissions guaranteed to saturate a cluster and checks the resulting biases are finite.
The saturated-cluster test only exercised clusters that end up at emission mean 0.0. It never checked a cluster whose emissions are all 1, so a change that widened the clip's upper bound back toward 1.0 would slip through undetected. Now one blob is all 0s and the other is all 1s, and the test checks that both a negative and a positive bias show up and stay finite.
Restarts now run one at a time via lax.map instead of all at once via vmap. Peak memory no longer scales with the number of restarts, and each restart stops as soon as it converges instead of the whole batch waiting for the slowest one.
Sequential restarts still stack each restart's assignments, so peak memory grows with the restart count rather than staying flat; it is the slope that drops, by a factor of the cluster count. Also scope the speed comparison to CPU, which is where it was measured.
The kmeans branch of LogisticRegressionHMMEmissions.initialize computes per-cluster emission means with jnp.mean(..., where=...) specifically so it works inside jax.jit and jax.vmap. The seemingly equivalent boolean-mask form (flat_emissions[mask].mean()) also passes in eager mode but breaks under those transformations because it produces a variable-shaped intermediate. No existing test caught this since every prior test calls initialize() eagerly. This adds a test that runs kmeans initialization through both jax.jit and jax.vmap and checks the resulting biases stay finite, so a future switch back to the boolean-mask form fails loudly.
Sequential restarts stack the whole returned state, not just the assignments; the assignments merely dominate it at the sample counts this is used with.
The k-equals-sample-count test was named for a case it did not cover and only asserted finiteness, which the neighbouring empty-cluster test already implies; assert the property that boundary actually has, namely that every sample ends up as its own centroid.
Draw n_local_trials candidate centroids per k-means++ step and keep whichever minimizes total inertia, instead of a single candidate draw. A single candidate can land in a bad local optimum on well-separated data; greedy selection removes those failures. Carries the running closest-centroid distance vector forward across steps instead of recomputing it, keeping the extra cost proportional to n_local_trials rather than the cluster count.
Greedy seeding cuts the restarts needed to reach sklearn-quality optima. n_local_trials controls candidates evaluated per k-means++ step (static, defaults to 2 + int(log(k))); n_init drops from 10 to 3, which matches or beats prior quality across measured workloads since restarts still recover from bad early commitments that no amount of per-step trials can undo. Repointed the restart-quality regression test onto a fixture where restarts still matter after greedy seeding, and added a test that n_local_trials=1 reproduces plain k-means++ while a higher value fixes it.
kylejcaron
marked this pull request as ready for review
August 20, 2026 15:14
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.
Address #459 and #315
Background
This PR removes sklearn from the main dependencies by implementing KMeans in jax.
Performance
K-means++ normally picks each new starting point by drawing one random candidate. This PR instead draws a few candidates at each step (
n_local_trials, controlled byn_init) and keeps whichever one lowers the clustering error the most. Better starting points mean we need fewer restarts to get a good result, so the defaultn_initdrops from 10 to 3, with equal or better clustering quality. See thekmeansdocstring indynamax/utils/cluster.pyfor the full writeup and measured range (2.8x-40x faster depending on how separated the clusters are). Unsure if its worth keeping that detail in the docstring.I have a few pseudo-reproducible benchmark scripts that measures three things on the same test data (a
GaussianHMMwith 10 states, 10-dimensional emissions, 100k timesteps): this PR's k-means, the k-means it replaces (one candidate per step,n_init=10- what's onmain's branch history right before this PR), and scikit-learn'sKMeans(whatmainactually uses today).How long clustering alone takes. This skips the rest of HMM setup, which costs the same amount of time no matter which k-means you use, and is dwarfed by the EM fitting step anyway — see below.
Measured (Apple M3 Pro, CPU only, min of 3 runs after warmup):
maintoday - sklearnKMeansn_init=10)n_init=3)The k-means implementation this PR replaces was already about 2x slower than scikit-learn's. This PR doesn't just close that gap — it makes our clustering about 2.4x faster than scikit-learn and about 4.8x faster than the k-means already on
main, while finding clusters of the same quality.How long a full model fit takes (clustering + 50 rounds of EM). Included for completeness, but the speedup above doesn't show up here.
main- sklearnAll three take about the same time, because the EM fitting step — not clustering — is over 95% of the total time at this data size. So fitting one model won't feel any faster. The speedup matters when you run clustering many times without a full model fit each time, e.g. comparing different starting points, running many small fits, or trying different hyperparameters.