Skip to content

fix(tdigest): correct rank and quantile tails - #531

Merged
leerho merged 1 commit into
apache:masterfrom
jaideeppyne:fix/tdigest-rank-quantile-tails
Oct 1, 2026
Merged

leerho merged 1 commit into
apache:masterfrom
jaideeppyne:fix/tdigest-rank-quantile-tails

Conversation

@jaideeppyne

Copy link
Copy Markdown
Contributor

The C++ t-digest still disagrees with the Java, Go, and Rust sketches on three quantile and rank cases those ports already fixed.

  • get_rank() did not divide the left tail by the total centroid weight, so a value below the first centroid could report a rank greater than 1. The right tail was already normalized.
  • get_quantile() added the right-tail offset onto the maximum, so estimates could land above the stored max. The tail is measured back from the maximum. A last centroid of weight 2 makes that division 0/0, and the only rank that reaches it is the maximum itself, so return the maximum.
  • Interpolation weighted each centroid by the distance to itself. Weighting by the distance to the other centroid pulls the estimate toward the nearer one.

The new tests follow the Java cases: quantiles over 10k updates stay monotonic and inside [min, max], a byte-patched heavy first centroid keeps ranks inside [0, 1], a heavy last centroid never exceeds max, weight 2 returns max rather than NaN, and an asymmetric interior point returns 13 rather than 17.

Tests: tdigest_test (55 cases).

The left tail of get_rank() was not divided by the total weight, and the
right tail of get_quantile() was added onto the maximum. A last centroid
of weight 2 makes that division 0/0, so return the maximum. Interpolation
now weights each centroid by the distance to the other one.

@leerho leerho left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude]: This is small and self-contained, and its three changes match the Java implementation line for line and fixes some real bugs.

  • get_rank() left tail. Values below the first centroid weren't divided by the total weight, so the rank could exceed 1. That's clearly a bug, and the code even had a // ? comment on that line.
  • get_quantile() right tail. The tail was added onto the maximum instead of measured back from it, so results could exceed the stored max. It also now returns max when the last centroid has weight 2, instead of dividing by zero and returning NaN.
  • Interpolation weights. They were swapped, which pulled estimates toward the farther centroid.

@leerho
leerho merged commit b93fc57 into apache:master Oct 1, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants