diff --git a/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java b/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java index 19260882d..45dda8d0c 100644 --- a/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java +++ b/src/main/java/org/apache/datasketches/kll/KllFloatsHelper.java @@ -259,7 +259,7 @@ static void mergeSortedFloatArrays( } /** - * Validation Method. This must be modified to test validation + * Validation Method. This must be modified to use the validation test * @param buf the items array * @param start data start * @param length items length @@ -278,7 +278,7 @@ static void randomlyHalveDownFloats(final float[] buf, final int start, final in } /** - * Validation Method. This must be modified to test validation + * Validation Method. This must be modified to use the validation test * @param buf the items array * @param start data start * @param length items length diff --git a/src/main/java/org/apache/datasketches/kll/KllHelper.java b/src/main/java/org/apache/datasketches/kll/KllHelper.java index 38d85e5c5..dfbcdf7a5 100644 --- a/src/main/java/org/apache/datasketches/kll/KllHelper.java +++ b/src/main/java/org/apache/datasketches/kll/KllHelper.java @@ -161,142 +161,85 @@ static void compressWhileUpdatingSketch(final KllSketch mine) { final int adjPop = oddPop ? rawPop - 1 : rawPop; final int halfAdjPop = adjPop / 2; - // level zero might not be sorted, so we must sort it if we wish to compact it - float[] myFloatItemsArr; - double[] myDoubleItemsArr; - if (mine.sketchType == DOUBLES_SKETCH) { - myFloatItemsArr = null; - myDoubleItemsArr = mine.getDoubleItemsArray(); - if (level == 0) { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - Arrays.sort(myDoubleItemsArr, adjBeg, adjBeg + adjPop); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - Arrays.sort(mine.getDoubleItemsArray(), adjBeg, adjBeg + adjPop); - } + final double[] myDoubleItemsArr = mine.getDoubleItemsArray(); + if (level == 0) { // level zero might not be sorted, so we must sort it if we wish to compact it + Arrays.sort(myDoubleItemsArr, adjBeg, adjBeg + adjPop); } if (popAbove == 0) { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.randomlyHalveUpDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - KllDoublesHelper.randomlyHalveUpDoubles(mine.getDoubleItemsArray(), adjBeg, adjPop, KllSketch.random); - } + KllDoublesHelper.randomlyHalveUpDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); } else { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.randomlyHalveDownDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - KllDoublesHelper.randomlyHalveDownDoubles(mine.getDoubleItemsArray(), adjBeg, adjPop, KllSketch.random); - } - if (mine.updatableMemFormat ) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.mergeSortedDoubleArrays( - myDoubleItemsArr, adjBeg, halfAdjPop, - myDoubleItemsArr, rawEnd, popAbove, - myDoubleItemsArr, adjBeg + halfAdjPop); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - myDoubleItemsArr = mine.getDoubleItemsArray(); - KllDoublesHelper.mergeSortedDoubleArrays( - myDoubleItemsArr, adjBeg, halfAdjPop, - myDoubleItemsArr, rawEnd, popAbove, - myDoubleItemsArr, adjBeg + halfAdjPop); - } + KllDoublesHelper.randomlyHalveDownDoubles(myDoubleItemsArr, adjBeg, adjPop, KllSketch.random); + KllDoublesHelper.mergeSortedDoubleArrays( + myDoubleItemsArr, adjBeg, halfAdjPop, + myDoubleItemsArr, rawEnd, popAbove, + myDoubleItemsArr, adjBeg + halfAdjPop); } - } else { //Float sketch - myFloatItemsArr = mine.getFloatItemsArray(); - myDoubleItemsArr = null; - if (level == 0) { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - Arrays.sort(myFloatItemsArr, adjBeg, adjBeg + adjPop); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - Arrays.sort(mine.getFloatItemsArray(), adjBeg, adjBeg + adjPop); - } + + int newIndex = myLevelsArr[level + 1] - halfAdjPop; // adjust boundaries of the level above + mine.setLevelsArrayAt(level + 1, newIndex); + + if (oddPop) { + mine.setLevelsArrayAt(level, myLevelsArr[level + 1] - 1); // the current level now contains one item + myDoubleItemsArr[myLevelsArr[level]] = myDoubleItemsArr[rawBeg]; // namely this leftover guy + } else { + mine.setLevelsArrayAt(level, myLevelsArr[level + 1]); // the current level is now empty + } + + // verify that we freed up halfAdjPop array slots just below the current level + assert myLevelsArr[level] == rawBeg + halfAdjPop; + + // finally, we need to shift up the data in the levels below + // so that the freed-up space can be used by level zero + if (level > 0) { + final int amount = rawBeg - myLevelsArr[0]; + System.arraycopy(myDoubleItemsArr, myLevelsArr[0], myDoubleItemsArr, myLevelsArr[0] + halfAdjPop, amount); + } + for (int lvl = 0; lvl < level; lvl++) { + newIndex = myLevelsArr[lvl] + halfAdjPop; //adjust boundary + mine.setLevelsArrayAt(lvl, newIndex); + } + mine.setDoubleItemsArray(myDoubleItemsArr); + } + else { //Float sketch + final float[] myFloatItemsArr = mine.getFloatItemsArray(); + if (level == 0) { // level zero might not be sorted, so we must sort it if we wish to compact it + Arrays.sort(myFloatItemsArr, adjBeg, adjBeg + adjPop); } if (popAbove == 0) { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.randomlyHalveUpFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - KllFloatsHelper.randomlyHalveUpFloats(mine.getFloatItemsArray(), adjBeg, adjPop, KllSketch.random); - } + KllFloatsHelper.randomlyHalveUpFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); } else { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.randomlyHalveDownFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - KllFloatsHelper.randomlyHalveDownFloats(mine.getFloatItemsArray(), adjBeg, adjPop, KllSketch.random); - } - if (mine.updatableMemFormat ) { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.mergeSortedFloatArrays( - myFloatItemsArr, adjBeg, halfAdjPop, - myFloatItemsArr, rawEnd, popAbove, - myFloatItemsArr, adjBeg + halfAdjPop); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - myFloatItemsArr = mine.getFloatItemsArray(); - KllFloatsHelper.mergeSortedFloatArrays( - myFloatItemsArr, adjBeg, halfAdjPop, - myFloatItemsArr, rawEnd, popAbove, - myFloatItemsArr, adjBeg + halfAdjPop); - } + KllFloatsHelper.randomlyHalveDownFloats(myFloatItemsArr, adjBeg, adjPop, KllSketch.random); + KllFloatsHelper.mergeSortedFloatArrays( + myFloatItemsArr, adjBeg, halfAdjPop, + myFloatItemsArr, rawEnd, popAbove, + myFloatItemsArr, adjBeg + halfAdjPop); } - } - int newIndex = myLevelsArr[level + 1] - halfAdjPop; // adjust boundaries of the level above - mine.setLevelsArrayAt(level + 1, newIndex); - if (oddPop) { - mine.setLevelsArrayAt(level, myLevelsArr[level + 1] - 1); // the current level now contains one item - if (mine.sketchType == DOUBLES_SKETCH) { - mine.setDoubleItemsArrayAt( - myLevelsArr[level], mine.getDoubleItemsArray()[rawBeg]); // namely this leftover guy + int newIndex = myLevelsArr[level + 1] - halfAdjPop; // adjust boundaries of the level above + mine.setLevelsArrayAt(level + 1, newIndex); + + if (oddPop) { + mine.setLevelsArrayAt(level, myLevelsArr[level + 1] - 1); // the current level now contains one item + myFloatItemsArr[myLevelsArr[level]] = myFloatItemsArr[rawBeg]; // namely this leftover guy } else { - mine.setFloatItemsArrayAt( - myLevelsArr[level], mine.getFloatItemsArray()[rawBeg]); // namely this leftover guy + mine.setLevelsArrayAt(level, myLevelsArr[level + 1]); // the current level is now empty } - } else { - mine.setLevelsArrayAt(level, myLevelsArr[level + 1]); // the current level is now empty - } - - // verify that we freed up halfAdjPop array slots just below the current level - assert myLevelsArr[level] == rawBeg + halfAdjPop; - - // finally, we need to shift up the data in the levels below - // so that the freed-up space can be used by level zero - if (level > 0) { - final int amount = rawBeg - myLevelsArr[0]; - if (mine.sketchType == DOUBLES_SKETCH) { - if (mine.updatableMemFormat) { - myDoubleItemsArr = mine.getDoubleItemsArray(); - System.arraycopy(myDoubleItemsArr, myLevelsArr[0], myDoubleItemsArr, myLevelsArr[0] + halfAdjPop, amount); - mine.setDoubleItemsArray(myDoubleItemsArr); - } else { - System.arraycopy(myDoubleItemsArr, myLevelsArr[0], myDoubleItemsArr, myLevelsArr[0] + halfAdjPop, amount); - } - } else { - if (mine.updatableMemFormat) { - myFloatItemsArr = mine.getFloatItemsArray(); - System.arraycopy(myFloatItemsArr, myLevelsArr[0], myFloatItemsArr, myLevelsArr[0] + halfAdjPop, amount); - mine.setFloatItemsArray(myFloatItemsArr); - } else { - System.arraycopy(myFloatItemsArr, myLevelsArr[0], myFloatItemsArr, myLevelsArr[0] + halfAdjPop, amount); - } + // verify that we freed up halfAdjPop array slots just below the current level + assert myLevelsArr[level] == rawBeg + halfAdjPop; + + // finally, we need to shift up the data in the levels below + // so that the freed-up space can be used by level zero + if (level > 0) { + final int amount = rawBeg - myLevelsArr[0]; + System.arraycopy(myFloatItemsArr, myLevelsArr[0], myFloatItemsArr, myLevelsArr[0] + halfAdjPop, amount); } for (int lvl = 0; lvl < level; lvl++) { newIndex = myLevelsArr[lvl] + halfAdjPop; //adjust boundary mine.setLevelsArrayAt(lvl, newIndex); } + mine.setFloatItemsArray(myFloatItemsArr); } } diff --git a/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java b/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java index 1588112a6..9eb25195d 100644 --- a/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java +++ b/src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java @@ -74,7 +74,7 @@ final class KllMemoryValidate { boolean singleItem; final boolean level0Sorted; final boolean doublesSketch; - final boolean updatableMemFormat; + boolean updatableMemFormat = false; final boolean readOnly; final int k; final int m; diff --git a/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java index ca534bd5d..5684084ac 100644 --- a/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java +++ b/src/test/java/org/apache/datasketches/kll/KllDoublesSketchTest.java @@ -417,17 +417,17 @@ public void checkReset() { @Test public void coverInheritanceArtifacts() { - float[] dblArr = new float[0]; - float dblV = 1.0f; + float[] fltArr = new float[0]; + float fltV = 1.0f; int idx = 1; KllDoublesSketch sk = KllDoublesSketch.newHeapInstance(20); try { sk.getFloatItemsArray(); fail(); } catch (SketchesArgumentException e) { } try { sk.getMaxFloatValue(); fail(); } catch (SketchesArgumentException e) { } try { sk.getMinFloatValue(); fail(); } catch (SketchesArgumentException e) { } - try { sk.setFloatItemsArray(dblArr); fail(); } catch (SketchesArgumentException e) { } - try { sk.setFloatItemsArrayAt(idx,dblV); fail(); } catch (SketchesArgumentException e) { } - try { sk.setMaxFloatValue(dblV); fail(); } catch (SketchesArgumentException e) { } - try { sk.setMinFloatValue(dblV); fail(); } catch (SketchesArgumentException e) { } + try { sk.setFloatItemsArray(fltArr); fail(); } catch (SketchesArgumentException e) { } + try { sk.setFloatItemsArrayAt(idx,fltV); fail(); } catch (SketchesArgumentException e) { } + try { sk.setMaxFloatValue(fltV); fail(); } catch (SketchesArgumentException e) { } + try { sk.setMinFloatValue(fltV); fail(); } catch (SketchesArgumentException e) { } } @Test diff --git a/src/test/java/org/apache/datasketches/kll/KllSketchTest.java b/src/test/java/org/apache/datasketches/kll/KllSketchTest.java index 1801a7dc9..f99bf9cdb 100644 --- a/src/test/java/org/apache/datasketches/kll/KllSketchTest.java +++ b/src/test/java/org/apache/datasketches/kll/KllSketchTest.java @@ -69,7 +69,8 @@ public void checkWritableWrapCase6And2Doubles() { @Test public void checkWritableWrapFloats() { KllFloatsSketch sk = KllFloatsSketch.newHeapInstance(20); - for (int i = 1; i <= 21; i++) { sk.update(i); } + for (int i = 1; i <= 20; i++) { sk.update(i); } + sk.update(21); WritableMemory wmem = WritableMemory.writableWrap(KllHelper.toUpdatableByteArrayImpl(sk)); KllFloatsSketch sk2 = KllFloatsSketch.writableWrap(wmem, memReqSvr); assertFalse(wmem.isReadOnly());