Copilot commented on code in PR #673:
URL: https://github.com/apache/datasketches-java/pull/673#discussion_r2205908917
##########
src/main/java/org/apache/datasketches/tdigest/TDigestDouble.java:
##########
@@ -246,40 +247,41 @@ public double getQuantile(final double rank) {
// at least 2 centroids
final double weight = rank * centroidsWeight_;
if (weight < 1) { return minValue_; }
- if (weight > centroidsWeight_ - 1.0) { return maxValue_; }
+ if (weight > (centroidsWeight_ - 1.0)) { return maxValue_; }
final double firstWeight = centroidWeights_[0];
- if (firstWeight > 1 && weight < firstWeight / 2.0) {
- return minValue_ + (weight - 1.0) / (firstWeight / 2.0 - 1.0) *
(centroidMeans_[0] - minValue_);
+ if ((firstWeight > 1) && (weight < (firstWeight / 2.0))) {
+ return minValue_ + (((weight - 1.0) / ((firstWeight / 2.0) - 1.0)) *
(centroidMeans_[0] - minValue_));
}
final double lastWeight = centroidWeights_[numCentroids_ - 1];
- if (lastWeight > 1 && centroidsWeight_ - weight <= lastWeight / 2.0) {
- return maxValue_ + (centroidsWeight_ - weight - 1.0) / (lastWeight / 2.0
- 1.0) * (maxValue_ - centroidMeans_[numCentroids_ - 1]);
+ if ((lastWeight > 1) && ((centroidsWeight_ - weight) <= (lastWeight /
2.0))) {
+ return maxValue_ + (((centroidsWeight_ - weight - 1.0) / ((lastWeight /
2.0) - 1.0))
+ * (maxValue_ - centroidMeans_[numCentroids_ - 1]));
}
// interpolate between extremes
double weightSoFar = firstWeight / 2.0;
- for (int i = 0; i < numCentroids_ - 1; i++) {
+ for (int i = 0; i < (numCentroids_ - 1); i++) {
final double dw = (centroidWeights_[i] + centroidWeights_[i + 1]) / 2.0;
- if (weightSoFar + dw > weight) {
+ if ((weightSoFar + dw) > weight) {
// the target weight is between centroids i and i+1
double leftWeight = 0;
if (centroidWeights_[i] == 1) {
- if (weight - weightSoFar < 0.5) { return centroidMeans_[i]; }
+ if ((weight - weightSoFar) < 0.5) { return centroidMeans_[i]; }
leftWeight = 0.5;
}
double rightWeight = 0;
if (centroidWeights_[i + 1] == 1) {
- if (weightSoFar + dw - weight <= 0.5) { return centroidMeans_[i +
1]; }
+ if (((weightSoFar + dw) - weight) <= 0.5) { return centroidMeans_[i
+ 1]; }
rightWeight = 0.5;
}
final double w1 = weight - weightSoFar - leftWeight;
- final double w2 = weightSoFar + dw - weight - rightWeight;
+ final double w2 = (weightSoFar + dw) - weight - rightWeight;
return weightedAverage(centroidMeans_[i], w1, centroidMeans_[i + 1],
w2);
}
weightSoFar += dw;
}
- final double w1 = weight - centroidsWeight_ -
centroidWeights_[numCentroids_ - 1] / 2.0;
- final double w2 = centroidWeights_[numCentroids_ - 1] / 2.0 - w1;
+ final double w1 = weight - centroidsWeight_ -
(centroidWeights_[numCentroids_ - 1] / 2.0);
+ final double w2 = (centroidWeights_[numCentroids_ - 1] / 2.0) - w1;
return weightedAverage(centroidWeights_[numCentroids_ - 1], w1, maxValue_,
w2);
Review Comment:
The first argument to weightedAverage should be the last centroid mean
(centroidMeans_) rather than the centroid weight. Using the weight here will
produce incorrect quantile values.
```suggestion
return weightedAverage(centroidMeans_[numCentroids_ - 1], w1, maxValue_,
w2);
```
##########
src/main/java/org/apache/datasketches/tdigest/TDigestDouble.java:
##########
@@ -99,7 +100,7 @@ public short getK() {
*/
public void update(final double value) {
if (Double.isNaN(value)) { return; }
- if (numBuffered_ == centroidsCapacity_ * BUFFER_MULTIPLIER) { compress(); }
+ if (numBuffered_ == (centroidsCapacity_ * BUFFER_MULTIPLIER)) {
compress(); }
Review Comment:
[nitpick] The parentheses around the multiplication are redundant. Consider
simplifying to `if (numBuffered_ == centroidsCapacity_ * BUFFER_MULTIPLIER) {
compress(); }` for improved readability.
```suggestion
if (numBuffered_ == centroidsCapacity_ * BUFFER_MULTIPLIER) {
compress(); }
```
--
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: commits-unsubscr...@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@datasketches.apache.org
For additional commands, e-mail: commits-h...@datasketches.apache.org
