small changes in the update single and update weighted methods.

This fixes a problem where the update to min and max might not occur on merging. This also simplifies the code and the number of checks in the update path.
apache · Jan 8, 2024 · 77a2c6c · 77a2c6c
1 parent 9c2f8d9
commit 77a2c6c
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 8 deletions.
diff --git a/src/main/java/org/apache/datasketches/kll/KllDoublesHelper.java b/src/main/java/org/apache/datasketches/kll/KllDoublesHelper.java
@@ -25,7 +25,6 @@
 import static org.apache.datasketches.common.Util.isOdd;
 import static org.apache.datasketches.kll.KllHelper.findLevelToCompact;
 import static org.apache.datasketches.kll.KllSketch.DEFAULT_M;
-import static org.apache.datasketches.kll.KllSketch.SketchStructure.UPDATABLE;
 
 import java.util.Arrays;
 import java.util.Random;
@@ -317,6 +316,7 @@ private static void randomlyHalveUpDoubles(final double[] buf, final int start,
 
   //Called from KllDoublesSketch::update and merge
   static void updateDouble(final KllDoublesSketch dblSk, final double item) {
+    dblSk.updateMinMax(item);
     int freeSpace = dblSk.levelsArr[0];
     assert (freeSpace >= 0);
     if (freeSpace == 0) {
@@ -331,10 +331,12 @@ static void updateDouble(final KllDoublesSketch dblSk, final double item) {
     dblSk.setDoubleItemsArrayAt(nextPos, item);
   }
 
+  //Called from KllDoublesSketch::update with weight
   static void updateDouble(final KllDoublesSketch dblSk, final double item, final int weight) {
-    if (weight < dblSk.getLevelsArray(UPDATABLE)[0]) {
-      for (int i = 0; i < weight; i++) { dblSk.update(item); }
+    if (weight < dblSk.levelsArr[0]) {
+      for (int i = 0; i < weight; i++) { updateDouble(dblSk, item); }
     } else {
+      dblSk.updateMinMax(item);
       final KllHeapDoublesSketch tmpSk = new KllHeapDoublesSketch(dblSk.getK(), DEFAULT_M, item, weight);
       dblSk.merge(tmpSk);
     }

diff --git a/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java b/src/main/java/org/apache/datasketches/kll/KllDoublesSketch.java
@@ -321,7 +321,6 @@ public String toString(final boolean withSummary, final boolean withDetail) {
   public void update(final double item) {
     if (Double.isNaN(item)) { return; } //ignore
     if (readOnly) { throw new SketchesArgumentException(TGT_IS_READ_ONLY_MSG); }
-    updateMinMax(item);
     KllDoublesHelper.updateDouble(this, item);
     kllDoublesSV = null;
   }
@@ -335,8 +334,7 @@ public void update(final double item, final int weight) {
     if (Double.isNaN(item)) { return; } //ignore
     if (readOnly) { throw new SketchesArgumentException(TGT_IS_READ_ONLY_MSG); }
     if (weight < 1) { throw new SketchesArgumentException("Weight is less than one."); }
-    if (Double.isNaN(item)) { return; } //ignore
-    updateMinMax(item);
+    if (weight == 1) { KllDoublesHelper.updateDouble(this, item); }
     KllDoublesHelper.updateDouble(this, item, weight);
     kllDoublesSV = null;
   }
@@ -407,7 +405,7 @@ private final void refreshSortedView() {
 
   abstract void setMinItem(double item);
 
-  private void updateMinMax(final double item) {
+  void updateMinMax(final double item) {
     if (isEmpty()) {
       setMinItem(item);
       setMaxItem(item);

diff --git a/src/test/java/org/apache/datasketches/kll/KllMiscDoublesTest.java b/src/test/java/org/apache/datasketches/kll/KllMiscDoublesTest.java
@@ -773,7 +773,7 @@ public void printlnTest() {
     printf("%s\n", s);
   }
 
-  private final static boolean enablePrinting = true;
+  private final static boolean enablePrinting = false;
 
   /**
    * @param format the format