From ec1024b0f0a8404f78c81aac68bad0cf782dea72 Mon Sep 17 00:00:00 2001
From: Lee Rhodes <leerho@gmail.com>
Date: Sat, 6 Apr 2024 18:24:38 -0700
Subject: [PATCH] Fixes due to review.

---
 .../datasketches/kll/KllHeapFloatsSketch.java | 12 +++---
 .../DoublesSketchSortedView.java              |  2 +-
 .../FloatsSketchSortedView.java               |  2 +-
 .../kll/KllDirectDoublesSketchTest.java       | 11 +++++-
 .../kll/KllDirectFloatsSketchTest.java        | 11 +++++-
 .../kll/KllDoublesSketchTest.java             | 37 ++++++++++--------
 .../datasketches/kll/KllFloatsSketchTest.java | 39 ++++++++++---------
 7 files changed, 69 insertions(+), 45 deletions(-)

diff --git a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java
index cc192b7f1..a7ca7a72f 100644
--- a/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java
+++ b/src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java
@@ -170,10 +170,10 @@ String getItemAsString(final int index) {
   public int getK() { return k; }
 
   //MinMax Methods
-  
+
   @Override
  float getMaxItemInternal() { return maxFloatItem; }
-  
+
   @Override
   public float getMaxItem() {
     if (isEmpty()) { throw new SketchesArgumentException(EMPTY_MSG); }
@@ -187,7 +187,7 @@ String getMaxItemAsString() {
 
   @Override
   float getMinItemInternal() { return minFloatItem; }
-  
+
   @Override
   public float getMinItem() {
     if (isEmpty() || Float.isNaN(minFloatItem)) { throw new SketchesArgumentException(EMPTY_MSG); }
@@ -214,7 +214,7 @@ byte[] getMinMaxByteArr() {
   void setMinItem(final float item) { this.minFloatItem = item; }
 
   //END MinMax Methods
-  
+
   @Override
   public long getN() { return n; }
 
@@ -286,10 +286,10 @@ void incNumLevels() {
   void setFloatItemsArrayAt(final int index, final float item) { this.floatItems[index] = item; }
 
   @Override
-  void setFloatItemsArrayAt(final int dstIndex, final float[] srcItems, final int srcOffset, final int length) { //TODO
+  void setFloatItemsArrayAt(final int dstIndex, final float[] srcItems, final int srcOffset, final int length) {
     System.arraycopy(srcItems, srcOffset, floatItems, dstIndex, length);
   }
-  
+
   @Override
   void setLevelZeroSorted(final boolean sorted) { this.isLevelZeroSorted = sorted; }
 
diff --git a/src/main/java/org/apache/datasketches/quantilescommon/DoublesSketchSortedView.java b/src/main/java/org/apache/datasketches/quantilescommon/DoublesSketchSortedView.java
index 564f8aa10..9f8060274 100644
--- a/src/main/java/org/apache/datasketches/quantilescommon/DoublesSketchSortedView.java
+++ b/src/main/java/org/apache/datasketches/quantilescommon/DoublesSketchSortedView.java
@@ -26,7 +26,7 @@
 import org.apache.datasketches.common.SketchesArgumentException;
 
 /**
- * The SortedView of the Quantiles Classic DoublesSketch and the KllDoublesSketch.
+ * The SortedView for the Quantiles Classic DoublesSketch and the KllDoublesSketch.
  * @author Alexander Saydakov
  * @author Lee Rhodes
  */
diff --git a/src/main/java/org/apache/datasketches/quantilescommon/FloatsSketchSortedView.java b/src/main/java/org/apache/datasketches/quantilescommon/FloatsSketchSortedView.java
index fbcfc8870..7dfb40531 100644
--- a/src/main/java/org/apache/datasketches/quantilescommon/FloatsSketchSortedView.java
+++ b/src/main/java/org/apache/datasketches/quantilescommon/FloatsSketchSortedView.java
@@ -26,7 +26,7 @@
 import org.apache.datasketches.common.SketchesArgumentException;
 
 /**
- * The SortedView of the Quantiles Classic DoublesSketch and the KllDoublesSketch.
+ * The SortedView for the KllFloatsSketch and the ReqSketch.
  * @author Alexander Saydakov
  * @author Lee Rhodes
  */
diff --git a/src/test/java/org/apache/datasketches/kll/KllDirectDoublesSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllDirectDoublesSketchTest.java
index 3d5b31609..33219a806 100644
--- a/src/test/java/org/apache/datasketches/kll/KllDirectDoublesSketchTest.java
+++ b/src/test/java/org/apache/datasketches/kll/KllDirectDoublesSketchTest.java
@@ -28,6 +28,7 @@
 import static org.testng.Assert.fail;
 
 import org.apache.datasketches.common.SketchesArgumentException;
+import org.apache.datasketches.kll.KllSketch.SketchStructure;
 import org.apache.datasketches.memory.DefaultMemoryRequestServer;
 import org.apache.datasketches.memory.Memory;
 import org.apache.datasketches.memory.WritableMemory;
@@ -644,6 +645,11 @@ public void checkVectorUpdate() {
     double[] v = new double[21];
     for (int i = 0; i < 21; i++) { v[i] = i + 1; }
     sk.update(v, 0, 21);
+    println(sk.toString(true, true));
+    int[] levelsArr = sk.getLevelsArray(SketchStructure.UPDATABLE);
+    assertEquals(levelsArr[0], 22);
+    double[] doublesArr = sk.getDoubleItemsArray();
+    assertEquals(doublesArr[22], 21);
   }
 
   @Test
@@ -654,6 +660,9 @@ public void checkWeightedUpdate() {
       sk.update(i + 1, 16);
     }
     println(sk.toString(true, true));
+    assertEquals(sk.getN(), 256);
+    assertEquals(sk.getMaxItem(), 16.0);
+    assertEquals(sk.getMinItem(), 1.0);
   }
 
   private static KllDoublesSketch getUpdatableDirectDoublesSketch(int k, int n) {
@@ -672,7 +681,7 @@ public void checkMergeExceptionsWrongType() {
     try { sk1.merge(sk2); fail(); } catch (ClassCastException e) { }
     try { sk2.merge(sk1); fail(); } catch (ClassCastException e) { }
   }
-  
+
   private final static boolean enablePrinting = false;
 
   /**
diff --git a/src/test/java/org/apache/datasketches/kll/KllDirectFloatsSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllDirectFloatsSketchTest.java
index f4f716e8f..2c9e0dca7 100644
--- a/src/test/java/org/apache/datasketches/kll/KllDirectFloatsSketchTest.java
+++ b/src/test/java/org/apache/datasketches/kll/KllDirectFloatsSketchTest.java
@@ -28,6 +28,7 @@
 import static org.testng.Assert.fail;
 
 import org.apache.datasketches.common.SketchesArgumentException;
+import org.apache.datasketches.kll.KllSketch.SketchStructure;
 import org.apache.datasketches.memory.DefaultMemoryRequestServer;
 import org.apache.datasketches.memory.Memory;
 import org.apache.datasketches.memory.WritableMemory;
@@ -644,6 +645,11 @@ public void checkVectorUpdate() {
     float[] v = new float[21];
     for (int i = 0; i < 21; i++) { v[i] = i + 1; }
     sk.update(v, 0, 21);
+    println(sk.toString(true, true));
+    int[] levelsArr = sk.getLevelsArray(SketchStructure.UPDATABLE);
+    assertEquals(levelsArr[0], 22);
+    float[] floatsArr = sk.getFloatItemsArray();
+    assertEquals(floatsArr[22], 21);
   }
 
   @Test
@@ -654,6 +660,9 @@ public void checkWeightedUpdate() {
       sk.update(i + 1, 16);
     }
     println(sk.toString(true, true));
+    assertEquals(sk.getN(), 256);
+    assertEquals(sk.getMaxItem(), 16F);
+    assertEquals(sk.getMinItem(), 1F);
   }
 
   private static KllFloatsSketch getUpdatableDirectFloatSketch(int k, int n) {
@@ -672,7 +681,7 @@ public void checkMergeExceptionsWrongType() {
     try { sk1.merge(sk2); fail(); } catch (ClassCastException e) { }
     try { sk2.merge(sk1); fail(); } catch (ClassCastException e) { }
   }
-  
+
   private final static boolean enablePrinting = false;
 
   /**
diff --git a/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java
index 743b9620d..0b3818f1f 100644
--- a/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java
+++ b/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java
@@ -477,18 +477,18 @@ public void checkCDF_PDF() {
     println("INCLUSIVE:");
     double[] cdf = sketch.getCDF(sp, INCLUSIVE);
     double[] pmf = sketch.getPMF(sp, INCLUSIVE);
-    printf("%10s%10s\n", "CDF", "PMF");
+    printf("%10s%10s" + LS, "CDF", "PMF");
     for (int i = 0; i < cdf.length; i++) {
-      printf("%10.2f%10.2f\n", cdf[i], pmf[i]);
+      printf("%10.2f%10.2f" + LS, cdf[i], pmf[i]);
       assertEquals(cdf[i], cdfI[i], toll);
       assertEquals(pmf[i], pmfI[i], toll);
     }
     println("EXCLUSIVE");
     cdf = sketch.getCDF(sp, EXCLUSIVE);
     pmf = sketch.getPMF(sp, EXCLUSIVE);
-    printf("%10s%10s\n", "CDF", "PMF");
+    printf("%10s%10s" + LS, "CDF", "PMF");
     for (int i = 0; i < cdf.length; i++) {
-      printf("%10.2f%10.2f\n", cdf[i], pmf[i]);
+      printf("%10.2f%10.2f" + LS, cdf[i], pmf[i]);
       assertEquals(cdf[i], cdfE[i], toll);
       assertEquals(pmf[i], pmfE[i], toll);
     }
@@ -615,7 +615,7 @@ public void checkVectorUpdate() {
     boolean withLevels = false;
     boolean withLevelsAndItems = true;
     int k = 20;
-    int n = 108;//108;
+    int n = 108;
     int maxVsz = 40;  //max vector size
     KllDoublesSketch sk = KllDoublesSketch.newHeapInstance(k);
     int j = 1;
@@ -629,6 +629,9 @@ public void checkVectorUpdate() {
     println(LS + "#<<< END STATE # >>>");
     println(sk.toString(withLevels, withLevelsAndItems));
     println("");
+    assertEquals(sk.getN(), 108);
+    assertEquals(sk.getMaxItem(), 108.0);
+    assertEquals(sk.getMinItem(), 1.0);
   }
 
   @Test
@@ -657,14 +660,14 @@ public void vectorizedUpdates() {
     }
     final long runTime = System.nanoTime() - startTime;
     println("Vectorized Updates");
-    printf("  Vector size : %,12d\n", N);
-    printf("  Num Vectors : %,12d\n", M);
-    printf("  Total Input : %,12d\n", totN);
-    printf("  Run Time mS : %,12.3f\n", runTime / 1e6);
+    printf("  Vector size : %,12d" + LS, N);
+    printf("  Num Vectors : %,12d" + LS, M);
+    printf("  Total Input : %,12d" + LS, totN);
+    printf("  Run Time mS : %,12.3f" + LS, runTime / 1e6);
     final double trialTime = runTime / (1e6 * trials);
-    printf("  mS / Trial  : %,12.3f\n", trialTime);
+    printf("  mS / Trial  : %,12.3f" + LS, trialTime);
     final double updateTime = runTime / (1.0 * totN * trials);
-    printf("  nS / Update : %,12.3f\n", updateTime);
+    printf("  nS / Update : %,12.3f" + LS, updateTime);
   }
 
   @Test
@@ -695,14 +698,14 @@ public void nonVectorizedUpdates() {
     }
     final long runTime = System.nanoTime() - startTime;
     println("Vectorized Updates");
-    printf("  Vector size : %,12d\n", N);
-    printf("  Num Vectors : %,12d\n", M);
-    printf("  Total Input : %,12d\n", totN);
-    printf("  Run Time mS : %,12.3f\n", runTime / 1e6);
+    printf("  Vector size : %,12d" + LS, N);
+    printf("  Num Vectors : %,12d" + LS, M);
+    printf("  Total Input : %,12d" + LS, totN);
+    printf("  Run Time mS : %,12.3f" + LS, runTime / 1e6);
     final double trialTime = runTime / (1e6 * trials);
-    printf("  mS / Trial  : %,12.3f\n", trialTime);
+    printf("  mS / Trial  : %,12.3f" + LS, trialTime);
     final double updateTime = runTime / (1.0 * totN * trials);
-    printf("  nS / Update : %,12.3f\n", updateTime);
+    printf("  nS / Update : %,12.3f" + LS, updateTime);
   }
 
   private final static boolean enablePrinting = false;
diff --git a/src/test/java/org/apache/datasketches/kll/KllFloatsSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllFloatsSketchTest.java
index 652bd51ac..a3271e181 100644
--- a/src/test/java/org/apache/datasketches/kll/KllFloatsSketchTest.java
+++ b/src/test/java/org/apache/datasketches/kll/KllFloatsSketchTest.java
@@ -477,18 +477,18 @@ public void checkCDF_PDF() {
     println("INCLUSIVE:");
     double[] cdf = sketch.getCDF(sp, INCLUSIVE);
     double[] pmf = sketch.getPMF(sp, INCLUSIVE);
-    printf("%10s%10s\n", "CDF", "PMF");
+    printf("%10s%10s" + LS, "CDF", "PMF");
     for (int i = 0; i < cdf.length; i++) {
-      printf("%10.2f%10.2f\n", cdf[i], pmf[i]);
+      printf("%10.2f%10.2f" + LS, cdf[i], pmf[i]);
       assertEquals(cdf[i], cdfI[i], toll);
       assertEquals(pmf[i], pmfI[i], toll);
     }
     println("EXCLUSIVE");
     cdf = sketch.getCDF(sp, EXCLUSIVE);
     pmf = sketch.getPMF(sp, EXCLUSIVE);
-    printf("%10s%10s\n", "CDF", "PMF");
+    printf("%10s%10s" + LS, "CDF", "PMF");
     for (int i = 0; i < cdf.length; i++) {
-      printf("%10.2f%10.2f\n", cdf[i], pmf[i]);
+      printf("%10.2f%10.2f" + LS, cdf[i], pmf[i]);
       assertEquals(cdf[i], cdfE[i], toll);
       assertEquals(pmf[i], pmfE[i], toll);
     }
@@ -615,7 +615,7 @@ public void checkVectorUpdate() {
     boolean withLevels = false;
     boolean withLevelsAndItems = true;
     int k = 20;
-    int n = 108;//108;
+    int n = 108;
     int maxVsz = 40;  //max vector size
     KllFloatsSketch sk = KllFloatsSketch.newHeapInstance(k);
     int j = 1;
@@ -629,6 +629,9 @@ public void checkVectorUpdate() {
     println(LS + "#<<< END STATE # >>>");
     println(sk.toString(withLevels, withLevelsAndItems));
     println("");
+    assertEquals(sk.getN(), 108);
+    assertEquals(sk.getMaxItem(), 108F);
+    assertEquals(sk.getMinItem(), 1F);
   }
 
   @Test
@@ -657,14 +660,14 @@ public void vectorizedUpdates() {
     }
     final long runTime = System.nanoTime() - startTime;
     println("Vectorized Updates");
-    printf("  Vector size : %,12d\n", N);
-    printf("  Num Vectors : %,12d\n", M);
-    printf("  Total Input : %,12d\n", totN);
-    printf("  Run Time mS : %,12.3f\n", runTime / 1e6);
+    printf("  Vector size : %,12d" + LS, N);
+    printf("  Num Vectors : %,12d" + LS, M);
+    printf("  Total Input : %,12d" + LS, totN);
+    printf("  Run Time mS : %,12.3f" + LS, runTime / 1e6);
     final double trialTime = runTime / (1e6 * trials);
-    printf("  mS / Trial  : %,12.3f\n", trialTime);
+    printf("  mS / Trial  : %,12.3f" + LS, trialTime);
     final double updateTime = runTime / (1.0 * totN * trials);
-    printf("  nS / Update : %,12.3f\n", updateTime);
+    printf("  nS / Update : %,12.3f" + LS, updateTime);
   }
 
   @Test
@@ -695,16 +698,16 @@ public void nonVectorizedUpdates() {
     }
     final long runTime = System.nanoTime() - startTime;
     println("Vectorized Updates");
-    printf("  Vector size : %,12d\n", N);
-    printf("  Num Vectors : %,12d\n", M);
-    printf("  Total Input : %,12d\n", totN);
-    printf("  Run Time mS : %,12.3f\n", runTime / 1e6);
+    printf("  Vector size : %,12d" + LS, N);
+    printf("  Num Vectors : %,12d" + LS, M);
+    printf("  Total Input : %,12d" + LS, totN);
+    printf("  Run Time mS : %,12.3f" + LS, runTime / 1e6);
     final double trialTime = runTime / (1e6 * trials);
-    printf("  mS / Trial  : %,12.3f\n", trialTime);
+    printf("  mS / Trial  : %,12.3f" + LS, trialTime);
     final double updateTime = runTime / (1.0 * totN * trials);
-    printf("  nS / Update : %,12.3f\n", updateTime);
+    printf("  nS / Update : %,12.3f" + LS, updateTime);
   }
-  
+
   private final static boolean enablePrinting = false;
 
   /**