tisonkun commented on code in PR #88:
URL: https://github.com/apache/datasketches-rust/pull/88#discussion_r2787931869
##########
datasketches/src/tdigest/sketch.rs:
##########
@@ -1251,15 +1251,24 @@ impl Centroid {
fn add(&mut self, other: Centroid) {
let (self_weight, other_weight) = (self.weight(), other.weight());
let total_weight = self_weight + other_weight;
- self.weight = self.weight.saturating_add(other.weight.get());
+ self.weight = self
+ .weight
+ .checked_add(other.weight.get())
+ .expect("weight overflow");
let (self_mean, other_mean) = (self.mean, other.mean);
- let ratio_self = self_weight / total_weight;
let ratio_other = other_weight / total_weight;
- self.mean = self_mean.mul_add(ratio_self, other_mean * ratio_other);
+ let delta = other_mean - self_mean;
+ self.mean = if delta.is_finite() {
+ delta.mul_add(ratio_other, self_mean)
+ } else {
+ let ratio_self = self_weight / total_weight;
+ self_mean.mul_add(ratio_self, other_mean * ratio_other)
+ };
Review Comment:
If `other_mean - self_mean` doesn't overflow/underflow, save one mul + one
div to reduce precision loss.
This can overflow/underflow, so we fallback to conservative weighted average
computed in that case. In both cases, use `mul_add` to best effort reduce
precision loss.
--
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]
