PerumalsamyR opened a new pull request, #742: URL: https://github.com/apache/datasketches-java/pull/742
Addresses #702 ### Problem When merging a value into an existing centroid, the mean was updated incrementally: ```java centroidMeans_[i] += ((values[current] - centroidMeans_[i]) * weights[current]) / centroidWeights_[i]; ``` The intermediate `(value - mean)` (or its product with the weight) can overflow to infinity even when both operands are finite — e.g. means near opposite ends of the double range. Once a centroid mean becomes infinite, a subsequent update produces `Inf + (-Inf) = NaN`, which is then stored, breaking the sort/binary-search invariants and getting serialized. The same overflow pattern existed in `weightedAverage()`, used for quantile interpolation. This mirrors the fix already applied to the Rust implementation in apache/datasketches-rust#23. ### Changes - `merge()`: keep the incremental mean update in the common path (bit-identical results), but fall back to an overflow-safe weighted average `m1*(w1/w) + m2*(w2/w)` when the result is not finite. Each term of that form is bounded by the magnitude of its input, so the stored mean stays finite for finite inputs. - `weightedAverage()`: normalize the weights before multiplying so quantile interpolation between centroids of large magnitude cannot overflow. - `update()`: ignore ±Infinity in addition to NaN. With mixed infinities in the input, no update formula can keep centroid means well-defined, and rejecting them was the approach endorsed in the Rust PR discussion. - `heapify()` / compat formats: validate deserialized state as requested in the issue — min/max, centroid means and buffered values must be finite, centroid weights must be positive — throwing `SketchesArgumentException` otherwise, since serialized input cannot be assumed to be well-formed. ### Testing - New test `extremeValuesDoNotProduceNaN` feeds 10k alternating `±Double.MAX_VALUE` values: on current main it produces non-finite quantiles (and NaN centroid means); with this change all centroid means and quantiles stay finite. - New tests for `update()` ignoring NaN/±Inf and for `heapify()` rejecting corrupted bytes (NaN/Inf mean, NaN min, zero weight, NaN single value). - Full suite passes: `mvn clean test` — 2174 tests, 0 failures. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
