Skip to content

Add K-means++ - #12976

Open
leolamien wants to merge 6 commits into
TheAlgorithms:masterfrom
leolamien:add_kmeans++
Open

leolamien wants to merge 6 commits into
TheAlgorithms:masterfrom
leolamien:add_kmeans++

Conversation

@leolamien

Copy link
Copy Markdown

Describe your change:

Adding K-means++ algorithm based on the existing naive K-means algorithm (k_means_clust.py)

  • Implements k-means++ initialization for better centroid selection
  • Improves convergence speed and avoids poor cluster initialization
  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Add or change doctests? -- Note: Please avoid changing both code and tests in a single pull request.
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

@algorithms-keeper algorithms-keeper Bot added the tests are failing Do not merge until tests pass label Sep 17, 2025
@algorithms-keeper algorithms-keeper Bot removed the tests are failing Do not merge until tests pass label Sep 17, 2025
@leolamien

Copy link
Copy Markdown
Author

Hi!
This PR adds a new K-means++ algorithm based on the naive K-means implementation.
The results are the same, but it converges faster than naive K-means, and in some cases the clustering quality is clearly better.
Please let me know if any improvements are needed. Thanks!

@cclauss

cclauss commented Sep 13, 2026

Copy link
Copy Markdown
Member

@priya-sundaram-dev, please review.

@priya-sundaram-dev priya-sundaram-dev 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.

Thanks for the contribution, @leolamien — I ran the code locally and the core K-means++ seeding works well: on a synthetic 3-cluster dataset it recovers all three centroids cleanly, and the D²-weighted seeding is implemented correctly. A few suggestions to bring it in line with the repo's conventions and make it more robust:

1. Add doctests to the algorithmic functions. Right now only report_generator has one. TheAlgorithms convention is that each function carries a small, deterministic doctest — and get_initial_centroids_kmeans_plus_plus, assign_clusters, revise_centroids, and compute_heterogeneity are all easy to test with a tiny fixed array + seed. For example:

>>> import numpy as np
>>> data = np.array([[0.0, 0.0], [0.0, 1.0], [10.0, 10.0], [10.0, 11.0]])
>>> centroids = get_initial_centroids_kmeans_plus_plus(data, 2, seed=42)
>>> centroids.shape
(2, 2)

2. Empty-cluster robustness. If a cluster loses all its members during iteration, revise_centroids computes member_data_points.mean(axis=0) over an empty slice, which produces nan centroids (I reproduced this). That failure is currently hidden by the module-level warnings.filterwarnings("ignore"). I'd suggest removing the global warning filter (it silences all warnings for anyone importing this module) and instead guarding the empty case explicitly, e.g. keep the previous centroid when a cluster is empty.

3. Dead code. In get_initial_centroids_kmeans_plus_plus, probabilities = [] is assigned and then immediately overwritten a few lines later — it can be dropped.

4. Type hints. The repo requires type hints on function signatures; most functions here are missing them (data: np.ndarray, k: int, seed: int | None = None -> np.ndarray, etc.).

5. Scope. The if False: mock-test block and the large report_generator pandas utility aren't really part of the K-means++ algorithm and add a lot of surface area to the file. Consider replacing the if False: block with proper doctests, and either dropping report_generator or moving it out — a focused, self-contained algorithm file is easier to review and maintain. (Minor: the header lists Python: 3.5, which is well past EOL.)

None of this takes away from the algorithm itself, which is solid. Happy to re-review once the doctests/type hints are in.

(Disclosure: I'm an AI agent helping triage and review contributions; a human maintainer makes the final merge call.)

@priya-sundaram-dev priya-sundaram-dev 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.

Thanks for the contribution, @Leonce-Wilson — K-means++ seeding is a genuinely nice addition. I reviewed it carefully; a couple of things should be addressed before merge.

1. This file largely duplicates machine_learning/k_means_clust.py

Nine of the ten functions here (get_initial_centroids, centroid_pairwise_dist, assign_clusters, revise_centroids, compute_heterogeneity, kmeans, the plot_* helpers, and the ~180-line report_generator) are copied almost verbatim from the existing k_means_clust.py. The only genuinely new logic is get_initial_centroids_kmeans_plus_plus.

Carrying two near-identical k-means implementations is a maintenance hazard (fixes/bugs have to be kept in sync). I'd suggest one of:

  • add just get_initial_centroids_kmeans_plus_plus() to the existing k_means_clust.py and expose it as an optional init="k-means++" path, or
  • keep a small standalone module that contains only the k-means++ seeding function and imports the shared helpers from k_means_clust.

Either way, please drop the copied report_generator — it's unrelated to k-means++.

2. Correctness bug: division by zero when D² sums to zero

When every remaining point already coincides with a chosen centroid (e.g. k greater than the number of distinct points, or duplicate/identical rows), np.sum(squared_distances) is 0, so probabilities becomes all-NaN and rng.choice raises. Reproducer:

import numpy as np
dup = np.array([[0.0, 0.0]] * 5 + [[1.0, 1.0]] * 5)  # only 2 distinct points
get_initial_centroids_kmeans_plus_plus(dup, 4, seed=1)
# ValueError: probabilities contain NaN

Minimal guard (falls back to a uniform draw when all D² are zero):

total = np.sum(squared_distances)
probabilities = (
    np.full(n, 1.0 / n) if total == 0 else squared_distances / total
)

I confirmed this makes both the k > #distinct and all-identical cases return cleanly.

3. Minor

  • probabilities = [] at the top of the loop is dead — it's immediately overwritten. Remove it.
  • Module-level warnings.filterwarnings("ignore") hides warnings globally for anyone importing this file; better to not suppress, or scope it narrowly.
  • The new seeding logic has no doctest, so CI never exercises it. A tiny deterministic doctest (fixed seed=, well-separated points, asserting shape / that the two far clusters are picked) would lock in behavior — the repo's contributing guide also asks for doctests + type hints on new functions.

Happy to re-review once the duplication is trimmed and the zero-distance guard is in. Nice work getting the D² weighting right.

@cclauss cclauss added the awaiting changes A maintainer has requested changes to this PR label Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes A maintainer has requested changes to this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants