Skip to content

Commit

Permalink
Close tdunning#169 Re-order terms in division to avoid periodic decim…
Browse files Browse the repository at this point in the history
…al and rounding issues in Centroid#add, fixing out-of-order centroids
  • Loading branch information
kinow committed May 8, 2023
1 parent 72acae1 commit 74996c7
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
10 changes: 9 additions & 1 deletion core/src/main/java/com/tdunning/math/stats/Centroid.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,15 @@ public void add(double x, int w) {
actualData.add(x);
}
count += w;
centroid += w * (x - centroid) / count;
// NOTE: The previous calculation was computed with `w * (x - centroid) / count;`.
// Now we calculate a factor here for cases where `count` might be a
// number like 3 that could lead to a repeating decimal and possible
// rounding issues. e.g. `count` starts as `0` (zero) and `w` as `3`.
// That could cause a calculation like `3 * (0.8046947099707735 - 0.0) / 3`.
// Calculating this we would get `0.8046947099707736` (same error for `w`
// equals to `6`, `11`, etc.)
double factor = (double) w / count;
centroid += factor * (x - centroid);
}

public double mean() {
Expand Down
9 changes: 9 additions & 0 deletions core/src/test/java/com/tdunning/math/stats/TDigestTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,15 @@ public void testSorted() {
}
}

/**
* Issue: https://github.com/tdunning/t-digest/issues/169
*/
public void testCentroidAdd() {
double x = 0.8046947099707735;
Centroid centroid = new Centroid(x, 3);
assertEquals(x, centroid.mean(), 0.0);
}

@Test
public void testNaN() {
final TDigest digest = factory().create();
Expand Down

0 comments on commit 74996c7

Please sign in to comment.