jmalkin commented on code in PR #541:
URL: https://github.com/apache/datasketches-java/pull/541#discussion_r1554533306
##########
src/test/java/org/apache/datasketches/kll/KllDirectFloatsSketchTest.java:
##########
@@ -637,14 +638,25 @@ public void checkMergeExceptions() {
}
@Test
- public void checkMergeExceptionsWrongType() {
- KllFloatsSketch sk1 = KllFloatsSketch.newHeapInstance(20);
- KllDoublesSketch sk2 = KllDoublesSketch.newHeapInstance(20);
- try { sk1.merge(sk2); fail(); } catch (ClassCastException e) { }
- try { sk2.merge(sk1); fail(); } catch (ClassCastException e) { }
+ public void checkVectorUpdate() {
+ WritableMemory dstMem = WritableMemory.allocate(6000);
+ KllFloatsSketch sk = KllFloatsSketch.newDirectInstance(20, dstMem,
memReqSvr);
+ float[] v = new float[21];
+ for (int i = 0; i < 21; i++) { v[i] = i + 1; }
+ sk.update(v, 0, 21);
}
- private static KllFloatsSketch getUpdatableDirectFloatSketch(final int k,
final int n) {
+ @Test
+ public void checkWeightedUpdate() {
+ WritableMemory dstMem = WritableMemory.allocate(6000);
+ KllFloatsSketch sk = KllFloatsSketch.newDirectInstance(8, dstMem,
memReqSvr);
+ for (int i = 0; i < 16; i++) {
+ sk.update(i + 1, 16);
+ }
+ println(sk.toString(true, true));
Review Comment:
This test also doesn't really test anything. The println is generally set
not to print at release time.
##########
src/main/java/org/apache/datasketches/quantilescommon/FloatsSketchSortedView.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.datasketches.quantilescommon;
+
+import static
org.apache.datasketches.quantilescommon.QuantileSearchCriteria.INCLUSIVE;
+import static org.apache.datasketches.quantilescommon.QuantilesAPI.EMPTY_MSG;
+import static
org.apache.datasketches.quantilescommon.QuantilesUtil.getNaturalRank;
+
+import org.apache.datasketches.common.SketchesArgumentException;
+
+/**
+ * The SortedView of the Quantiles Classic DoublesSketch and the
KllDoublesSketch.
Review Comment:
s/Doubles/Floats/g
##########
src/test/java/org/apache/datasketches/kll/KllFloatsSketchTest.java:
##########
@@ -608,6 +610,101 @@ public void checkSortedViewAfterReset() {
try { sk.getSortedView(); fail(); } catch (SketchesArgumentException e) { }
}
+ @Test
+ public void checkVectorUpdate() {
+ boolean withLevels = false;
+ boolean withLevelsAndItems = true;
+ int k = 20;
+ int n = 108;//108;
+ int maxVsz = 40; //max vector size
+ KllFloatsSketch sk = KllFloatsSketch.newHeapInstance(k);
+ int j = 1;
+ int rem;
+ while ((rem = n - j + 1) > 0) {
+ int vecSz = min(rem, maxVsz);
+ float[] v = new float[vecSz];
+ for (int i = 0; i < vecSz; i++) { v[i] = j++; }
+ sk.update(v, 0, vecSz);
+ }
+ println(LS + "#<<< END STATE # >>>");
+ println(sk.toString(withLevels, withLevelsAndItems));
+ println("");
+ }
+
+ @Test
+ public void vectorizedUpdates() {
+ final int trials = 1;
+ final int M = 1; //number of vectors
+ final int N = 1000; //vector size
+ final int K = 256;
+ final float[] values = new float[N];
+ float vIn = 1.0F;
+ long totN = 0;
+ final long startTime = System.nanoTime();
+ for (int t = 0; t < trials; t++) {
+ final KllFloatsSketch sketch = KllFloatsSketch.newHeapInstance(K);
+ for (int m = 0; m < M; m++) {
+ for (int n = 0; n < N; n++) {
+ values[n] = vIn++; //fill vector
+ }
+ sketch.update(values, 0, N); //vector input
+ }
+ totN = sketch.getN();
+ assertEquals(totN, M * N);
+ assertEquals(sketch.getMinItem(), 1.0F);
+ assertEquals(sketch.getMaxItem(), totN);
+ assertEquals(sketch.getQuantile(0.5), (float)(totN / 2.0), totN *
PMF_EPS_FOR_K_256 * 2.0); //wider tolerance
+ }
+ final long runTime = System.nanoTime() - startTime;
+ println("Vectorized Updates");
Review Comment:
This seems like characterization code accidentally left in from testing?
##########
src/test/java/org/apache/datasketches/kll/KllFloatsSketchTest.java:
##########
@@ -608,6 +610,101 @@ public void checkSortedViewAfterReset() {
try { sk.getSortedView(); fail(); } catch (SketchesArgumentException e) { }
}
+ @Test
+ public void checkVectorUpdate() {
+ boolean withLevels = false;
+ boolean withLevelsAndItems = true;
+ int k = 20;
+ int n = 108;//108;
+ int maxVsz = 40; //max vector size
+ KllFloatsSketch sk = KllFloatsSketch.newHeapInstance(k);
+ int j = 1;
+ int rem;
+ while ((rem = n - j + 1) > 0) {
+ int vecSz = min(rem, maxVsz);
+ float[] v = new float[vecSz];
+ for (int i = 0; i < vecSz; i++) { v[i] = j++; }
+ sk.update(v, 0, vecSz);
+ }
+ println(LS + "#<<< END STATE # >>>");
Review Comment:
This test doesn't test anything
##########
src/main/java/org/apache/datasketches/kll/KllHeapFloatsSketch.java:
##########
@@ -272,13 +286,12 @@ void incNumLevels() {
void setFloatItemsArrayAt(final int index, final float item) {
this.floatItems[index] = item; }
@Override
- void setLevelZeroSorted(final boolean sorted) { this.isLevelZeroSorted =
sorted; }
-
- @Override
- void setMaxItem(final float item) { this.maxFloatItem = item; }
-
+ void setFloatItemsArrayAt(final int dstIndex, final float[] srcItems, final
int srcOffset, final int length) { //TODO
Review Comment:
Is this still a TODO item or just forgot to remove the comment?
##########
src/test/java/org/apache/datasketches/kll/KllDirectFloatsSketchTest.java:
##########
@@ -637,14 +638,25 @@ public void checkMergeExceptions() {
}
@Test
- public void checkMergeExceptionsWrongType() {
- KllFloatsSketch sk1 = KllFloatsSketch.newHeapInstance(20);
- KllDoublesSketch sk2 = KllDoublesSketch.newHeapInstance(20);
- try { sk1.merge(sk2); fail(); } catch (ClassCastException e) { }
- try { sk2.merge(sk1); fail(); } catch (ClassCastException e) { }
+ public void checkVectorUpdate() {
+ WritableMemory dstMem = WritableMemory.allocate(6000);
+ KllFloatsSketch sk = KllFloatsSketch.newDirectInstance(20, dstMem,
memReqSvr);
+ float[] v = new float[21];
+ for (int i = 0; i < 21; i++) { v[i] = i + 1; }
+ sk.update(v, 0, 21);
Review Comment:
This test don't actually check anything beyond "this 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
