You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@datasketches.apache.org by "jaideeppyne (via GitHub)" <gi...@apache.org> on 2026/09/16 05:10:43 UTC
[PR] fix(tdigest): preserve min/max when merging digests (datasketches-rust)
jaideeppyne opened a new pull request, #279:
URL: https://github.com/apache/datasketches-rust/pull/279
## Summary
- Update `TDigestMut::merge` to take `self.min`/`self.max` from `other.min`/`other.max` before compression.
- Compression only sees centroid means, which can differ from the stored extrema (e.g. after deserialization with heavy extreme centroids), so merge previously drifted `min_value()`/`max_value()` and degraded tail rank/quantile interpolation.
- Add a regression that merges digests whose stored min/max differ from extreme centroid means.
- Document the fix in the changelog.
Fixes #277
## Note for reviewers
This branch also touches `.github/workflows/ci.yml` only because the fork was behind `main` and the OAuth token lacks the `workflow` scope needed to push a tip that restores the current upstream workflow file. Please treat that hunk as unrelated fork-sync noise; the intended change is the three non-workflow files. Happy to refresh the branch once `workflow` scope is available.
## Validation
- Regression constructed from the existing `deserialize_with_centroids` helper (empty digest merge + two-way merge with drifted extrema).
- Diff against `main` for the t-digest sources is the two-line min/max update plus the new test and changelog line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] fix(tdigest): preserve min/max when merging digests (datasketches-rust)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun merged PR #279:
URL: https://github.com/apache/datasketches-rust/pull/279
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] fix(tdigest): preserve min/max when merging digests (datasketches-rust)
Posted by "jaideeppyne (via GitHub)" <gi...@apache.org>.
jaideeppyne commented on PR #279:
URL: https://github.com/apache/datasketches-rust/pull/279#issuecomment-5692367316
cc @tisonkun — would appreciate a review when you have a moment. Core fix is the two-line min/max update in `merge`; please disregard the unrelated `.github/workflows/ci.yml` hunk (fork sync / missing `workflow` OAuth scope).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] fix(tdigest): preserve min/max when merging digests (datasketches-rust)
Posted by "jaideeppyne (via GitHub)" <gi...@apache.org>.
jaideeppyne commented on PR #279:
URL: https://github.com/apache/datasketches-rust/pull/279#issuecomment-5707128088
Updated my fork main to upstream main (fast-forward) and removed the obsolete CI workflow changes from this PR. Its diff now contains only the TDigest fix, regression test, and changelog. I also corrected the test’s endpoint-rank expectations to match TDigest half-sample semantics. Local cargo x check, cargo x test, and cargo x lint pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org
Re: [PR] fix(tdigest): preserve min/max when merging digests (datasketches-rust)
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #279:
URL: https://github.com/apache/datasketches-rust/pull/279#issuecomment-5696243069
@jaideeppyne You should be able to update fork repo's main branch so that you don't bring back old CI hacks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@datasketches.apache.org
For additional commands, e-mail: dev-help@datasketches.apache.org