Re: [PR] First implementation of weighted update for KllDoubles. (datasketches-java)

["jmalkin (via GitHub)" <gi...@apache.org>]

jmalkin commented on code in PR #487:
URL: https://github.com/apache/datasketches-java/pull/487#discussion_r1439848913


##########
src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java:
##########
@@ -324,6 +324,19 @@ public void update(final double item) {
     kllDoublesSV = null;
   }
 
+  /**
+   * Updates this sketch with the given item the number of times specified by the given weight.

Review Comment:
   I know it's already an `int`, but I think we should also emphasize in the description that it takes only integral values. We've had a feature request for an arbitrary-weight sketch, and this is not that (which is perfectly fine; I'm just trying to be super clear to users).



##########
src/main/java/org/apache/datasketches/kll/KllHeapDoublesSketch.java:
##########
@@ -81,6 +81,25 @@ final class KllHeapDoublesSketch extends KllDoublesSketch {
     this.doubleItems = new double[k];
   }
 
+  /**
+   * Used for creating a temporary sketch for use with weighted updates.
+   */
+  KllHeapDoublesSketch(final int k, final int m, final double item, final int weight) {
+    super(UPDATABLE);
+    KllHelper.checkM(m);
+    KllHelper.checkK(k, m);
+    this.levelsArr = KllHelper.createLevelsArray(weight);
+    this.readOnly = false;
+    this.k = k;
+    this.m = m;
+    this.n = weight;
+    this.minK = k;
+    this.isLevelZeroSorted = false;

Review Comment:
   `true` -- they're all equal so it is sorted. Maybe it won't matter since they're all single-item levels, but if we can avoid degenerate calls to sort that's useful.



##########
src/test/java/org/apache/datasketches/kll/KllMiscDoublesTest.java:
##########
@@ -193,8 +195,75 @@ public void viewHeapCompactions() {
     println("");
   }
 
+  @Test //set static enablePrinting = true for visual checking
+  public void checkWeightedUpdates() {

Review Comment:
   This test doesn't seem to do anything other than "it doesn't throw"?



-- 
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-unsubscribe@datasketches.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org