Fixed bug in JaccardSimilarity.

Made ReservoirSize package private.  It will eventually be removed
entirely.

Author: lrhodes

src/main/java/com/yahoo/sketches/sampling/ReservoirSize.java

@@ -25,7 +25,7 @@
  *
  * @author Jon Malkin
  */
-public final class ReservoirSize {
+final class ReservoirSize {
   /**
    * Number of bins per power of two.
    */
@@ -57,7 +57,7 @@ private ReservoirSize() {}
    * @return reservoir size as 16-bit encoded value
    */
   public static short computeSize(final int k) {
-    if (k < 1 || k > MAX_ABS_VALUE) {
+    if ((k < 1) || (k > MAX_ABS_VALUE)) {
       throw new SketchesArgumentException("Can only encode strictly positive sketch sizes "
               + "less than " + MAX_ABS_VALUE + ", found: " + k);
     }
@@ -70,11 +70,11 @@ public static short computeSize(final int k) {
     }
 
     // mantissa is scalar in range [1,2); can reconstruct k as m * 2^p
-    final double m = Math.pow(2.0, Math.log(k) * INV_LN_2 - p);
+    final double m = Math.pow(2.0, (Math.log(k) * INV_LN_2) - p);
 
     // Convert to index offset: ceil(m * BPO) - BPO
     // Typically in range range 0-(BINS_PER_OCTAVE-1) (but see note below)
-    final int i = (int) Math.floor(m * BINS_PER_OCTAVE) - BINS_PER_OCTAVE + 1;
+    final int i = ((int) Math.floor(m * BINS_PER_OCTAVE) - BINS_PER_OCTAVE) + 1;
 
     // Due to ceiling, possible to overflow BINS_PER_OCTAVE
     // E.g., if BPO = 2048 then for k=32767 we have p=14. Except that 32767 > decodeValue
@@ -83,7 +83,7 @@ public static short computeSize(final int k) {
       return (short) ((((p + 1) & EXPONENT_MASK) << EXPONENT_SHIFT) & OUTPUT_MASK);
     }
 
-    return (short) (((p & EXPONENT_MASK) << EXPONENT_SHIFT) | (i & INDEX_MASK) & OUTPUT_MASK);
+    return (short) (((p & EXPONENT_MASK) << EXPONENT_SHIFT) | ((i & INDEX_MASK) & OUTPUT_MASK));
   }
 
   /**
@@ -103,6 +103,6 @@ public static int decodeValue(final short encodedSize) {
     final int p = (value >> EXPONENT_SHIFT) & EXPONENT_MASK;
     final int i = value & INDEX_MASK;
 
-    return (int) ((1 << p) * (i * INV_BINS_PER_OCTAVE + 1.0));
+    return (int) ((1 << p) * ((i * INV_BINS_PER_OCTAVE) + 1.0));
   }
 }

src/main/java/com/yahoo/sketches/theta/JaccardSimilarity.java

@@ -8,7 +8,11 @@
 import static com.yahoo.sketches.BoundsOnRatiosInThetaSketchedSets.getEstimateOfBoverA;
 import static com.yahoo.sketches.BoundsOnRatiosInThetaSketchedSets.getLowerBoundForBoverA;
 import static com.yahoo.sketches.BoundsOnRatiosInThetaSketchedSets.getUpperBoundForBoverA;
+import static com.yahoo.sketches.Util.MAX_LG_NOM_LONGS;
+import static com.yahoo.sketches.Util.MIN_LG_NOM_LONGS;
 import static com.yahoo.sketches.Util.ceilingPowerOf2;
+import static java.lang.Math.max;
+import static java.lang.Math.min;
 
 /**
  * Jaccard similarity of two Theta Sketches.
@@ -26,6 +30,9 @@ public final class JaccardSimilarity {
    * distinct from each other. A Jaccard of .95 means the overlap between the two
    * populations is 95% of the union of the two populations.
    *
+   * <p>Note: For very large pairs of sketches, where the configured nominal entries of the sketches
+   * are 2^25 or 2^26, this method may produce unpredictable results.
+   *
    * @param sketchA given sketch A
    * @param sketchB given sketch B
    * @return a double array {LowerBound, Estimate, UpperBound} of the Jaccard ratio.
@@ -42,8 +49,11 @@ public static double[] jaccard(final Sketch sketchA, final Sketch sketchB) {
     final int countB = sketchB.getRetainedEntries();
 
     //Create the Union
+    final int minK = 1 << MIN_LG_NOM_LONGS;
+    final int maxK = 1 << MAX_LG_NOM_LONGS;
+    final int newK = max(min(ceilingPowerOf2(countA + countB), maxK), minK);
     final Union union =
-        SetOperation.builder().setNominalEntries(ceilingPowerOf2(countA + countB)).buildUnion();
+        SetOperation.builder().setNominalEntries(newK).buildUnion();
     union.update(sketchA);
     union.update(sketchB);
     final Sketch unionAB = union.getResult();

src/test/java/com/yahoo/sketches/theta/JaccardSimilarityTest.java

@@ -69,7 +69,7 @@ public void checkExactMode() {
     UpdateSketch measured = UpdateSketch.builder().setNominalEntries(k).build();
     UpdateSketch expected = UpdateSketch.builder().setNominalEntries(k).build();
 
-    for (int i = 0; i < u-1; i++) { //one short
+    for (int i = 0; i < (u-1); i++) { //one short
       measured.update(i);
       expected.update(i);
     }
@@ -118,7 +118,7 @@ public void checkEstMode() {
     state = exactlyEqual(measured, expected);
     assertTrue(state);
 
-    for (int i = u; i < u + 50; i++) { //empirically determined
+    for (int i = u; i < (u + 50); i++) { //empirically determined
       measured.update(i);
     }
 
@@ -200,7 +200,23 @@ private static String jaccardString(double[] jResults) {
     double lb = jResults[0];
     double est = jResults[1];
     double ub = jResults[2];
-    return lb + "\t" + est + "\t" + ub + "\t" + (lb/est - 1.0) + "\t" + (ub/est - 1.0);
+    return lb + "\t" + est + "\t" + ub + "\t" + ((lb/est) - 1.0) + "\t" + ((ub/est) - 1.0);
+  }
+
+  @Test
+  public void checkMinK() {
+    UpdateSketch skA = UpdateSketch.builder().build(); //4096
+    UpdateSketch skB = UpdateSketch.builder().build(); //4096
+    skA.update(1);
+    skB.update(1);
+    double[] result = JaccardSimilarity.jaccard(skA, skB);
+    println(result[0] + ", " + result[1] + ", " + result[2]);
+    for (int i = 1; i < 4096; i++) {
+      skA.update(i);
+      skB.update(i);
+    }
+    result = JaccardSimilarity.jaccard(skA, skB);
+    println(result[0] + ", " + result[1] + ", " + result[2]);
   }
 
   @Test