Skip to content

Commit d08dd70

Browse files
ms609claude
andcommitted
Move KMeansPP dev artefacts from tests/benchmark/ to dev/
Assessment notes and scaling benchmarks are development aids, not part of the test suite; dev/ matches the pattern used by sibling repos. Add ^dev$ to .Rbuildignore so the directory is excluded from source tarballs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0375e68 commit d08dd70

5 files changed

Lines changed: 80 additions & 10 deletions

File tree

.Rbuildignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
^benchmark$
2+
^dev$
23
^Meta$
34
^doc$
45
^data-raw$

NEWS.md

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,6 @@
55

66
- Improve `KMeansPP()` performance: O(k²×n) → O(k×n).
77

8-
- `KMeansPP()` no longer builds the full _n_ × _n_ distance matrix to seed the
9-
`matrix` / `numeric` methods. k-means++ seeding now computes each centre's
10-
distance row on the fly in O(_n_ × dim), reducing the `matrix` method from
11-
O(_n_²) to O(_n_ × dim) memory and time and letting it cluster large point
12-
sets (_n_ ≫ 10000) that previously exhausted memory. The RNG draw sequence is
13-
preserved, so clusterings are unchanged. The `dist` method is O(_n_²) by
14-
construction (it clusters distance-space rows) and is unaffected, beyond
15-
avoiding one redundant matrix coercion.
16-
178
- Tinkering to get web app working
189

1910

dev/kmeanspp_cpp_assessment.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Is a C++ port of `KMeansPP` worthwhile?
2+
3+
Assessment after the O(n²) → O(n·dim) seeding refactor. Short answer: **the big
4+
win is already banked; a C++ port buys ~1.3–1.5× at most and only at large n, so
5+
it is not justified now.** Porting `stats::kmeans` is specifically *not*
6+
worthwhile unless paired with an accelerated algorithm.
7+
8+
## Where the time goes (evidence)
9+
10+
Decomposition of `KMeansPP.matrix(x, k = 10, nstart = 10)`, dim = 36:
11+
12+
| n | total | seeding loop (distance rows) | kmeans (×10) |
13+
|--------|------:|-----------------------------:|-------------:|
14+
| 6 400 | 1.85 s | 0.47 s (0.45 s) | 1.26 s (~68%) |
15+
| 20 000 | 5.31 s | 1.41 s (1.27 s) | ~5.3 s (dominant) |
16+
| 50 000 | 8.32 s | 3.64 s (3.25 s) | 2.86 s (~34%) |
17+
18+
`Rprof` self-time at n = 50000 (the cleanest apportionment; the table above is
19+
noisy because Hartigan–Wong's iteration count is data-dependent):
20+
21+
| symbol | self % | what it is |
22+
|----------------|-------:|-------------------------------------|
23+
| `.Fortran` | 49 % | `kmeans` Hartigan–Wong core (compiled) |
24+
| `.rowSums` | 40 % | distance-row computation (compiled C) |
25+
| `sample.int` | 4 % | D²-weighted draw (compiled) |
26+
| everything else| ~7 % | kmeans internals (`aperm`/`sweep`/`colMeans`), `pmin.int`, R glue |
27+
28+
**~89 %+ of runtime is already in compiled kernels.** The R-level glue we wrote
29+
(`.DistanceRow` closure, the `for` loops, `pmin.int`) is <1 % self-time. There is
30+
no slow interpreted loop left to eliminate — the earlier refactor already did
31+
that by deleting the O(n²) matrix build.
32+
33+
## What C++ could and could not buy
34+
35+
**1. Fused distance-row kernel (the only low-risk option).**
36+
The current row costs ~3 transient n×dim allocations (`rep`, the subtraction, the
37+
square) plus a separate `sqrt` pass. A fused C++ kernel would stream `x` once,
38+
accumulate `Σ(x[i,j]−c[j])²` per row, and `sqrt` in place — no temporaries — and
39+
could use the OpenMP already in `src/`. Realistic gain on the `.rowSums` portion
40+
~2–3× single-thread (allocation + cache), more with threads.
41+
*Amdahl ceiling:* distance rows are ~40 % at n=50000 but only ~24 % at n=6400, so
42+
overall this is **~1.3–1.5× at large n, ~1.2× at n=6400**. Effort: ~50–100 lines
43+
mirroring existing `src/` patterns. Reproducibility: same minor FP caveat as now
44+
(fusing changes summation order slightly; would need the same identity re-check).
45+
46+
**2. Porting `stats::kmeans` to C++ — not justified.**
47+
It is 49 % (n=50000) to ~68 % (n=6400) of the time, so it looks tempting, but it
48+
is **already compiled, well-tuned Fortran**. A naïve C++ Lloyd/Hartigan–Wong
49+
reimplementation would be a wash at best and likely slower (Hartigan–Wong
50+
converges in fewer passes than textbook Lloyd). A genuine speedup requires an
51+
*accelerated* algorithm — Elkan/Hamerly triangle-inequality bounds — which is
52+
research-grade effort, must handle empty-cluster and convergence edge cases, and
53+
**breaks exact reproducibility** (a C++ RNG cannot match R's `sample.int`, and
54+
bounds-accelerated k-means returns equivalent-not-identical results). Payoff
55+
maybe ~1.5–2× overall; cost and risk are out of proportion for a utility helper.
56+
57+
**3. A pure-R BLAS shortcut (mentioned for completeness, rejected).**
58+
`dist²(i) = ‖xᵢ‖² + ‖c‖² − 2 xᵢ·c` turns the row into a BLAS `gemv` (fast, often
59+
multithreaded) and avoids the `rep`. But it suffers catastrophic cancellation
60+
for near-coincident points (needs `pmax(0, ·)`) and **changes the FP results**,
61+
which would forfeit the bit-identity we just verified and could flip near-tie
62+
draws. Deliberately not used.
63+
64+
## Recommendation
65+
66+
- **Do nothing in C++ for now.** The asymptotic + memory win (O(n²) → O(n·dim))
67+
is the headline prize and it is captured. The residual cost is ~90 % in
68+
already-compiled kernels, so there is no cheap interpreter overhead to remove.
69+
- **If, and only if, a real workload profiles distance-row seeding as a
70+
bottleneck at very large n**, write the fused OpenMP distance-row kernel
71+
(option 1): best effort-to-reward ratio, reproducibility-preservable.
72+
- **Do not port `stats::kmeans`.** If kmeans itself becomes the bottleneck at
73+
scale, the right lever is algorithmic (the already-cited scalable k-means‖
74+
seeding, Bahmani 2012, and/or Elkan/Hamerly refinement), not a transliteration
75+
of the existing algorithm into C++.
76+
77+
_Profiling driver inline in this directory's session notes; reproduce with
78+
`Rprof` around `KMeansPP(matrix(rnorm(50000*36), ncol=36), k=10, nstart=10)`._
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
#
1414
# Run against an installed build, e.g.:
1515
# R CMD INSTALL --library=.dev-kmpp .
16-
# Rscript -e ".libPaths(c('.dev-kmpp', .libPaths())); source('tests/benchmark/kmeanspp_matrix_scaling.R')"
16+
# Rscript -e ".libPaths(c('.dev-kmpp', .libPaths())); source('dev/kmeanspp_matrix_scaling.R')"
1717

1818
library(TreeDist)
1919

File renamed without changes.

0 commit comments

Comments
 (0)