Skip to content

fix(tdigest): preserve stored min and max when merging - #769

Merged
leerho merged 1 commit into
apache:mainfrom
jaideeppyne:fix/tdigest-merge-minmax
Sep 17, 2026
Merged

leerho merged 1 commit into
apache:mainfrom
jaideeppyne:fix/tdigest-merge-minmax

Conversation

@jaideeppyne

Copy link
Copy Markdown
Contributor

Summary

  • Copy the stored min/max from the other digest in TDigestDouble.merge before compressing the combined centroid list.
  • Add a regression that heapifies an image whose stored min/max sit strictly outside the extreme centroid means, then merges it into an empty digest and into a non-empty one.

Why

merge previously re-derived extrema from the extreme centroid means. That is exact only when those centroids have unit weight. Deserialized images (and the reference implementation format) can have weighted tails, so min/max drifted inward and rank/quantile tail interpolation used the wrong endpoints.

Same defect as apache/datasketches-rust#277. Independent of #767.

Validation

  • TDigestDoubleTest#mergePreservesDeserializedMinMaxWithWeightedTails

Merging a digest whose extreme centroids have weight greater than one
re-derived min and max from those centroid means. Copy the stored extrema
from the other digest first so rank and quantile tail interpolation stay
correct, especially for deserialized images.

@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.

Thank you for uncovering and fixing this!

@leerho
leerho merged commit 3857c0c into apache:main Sep 17, 2026
6 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