Merge pull request #5 from jmeagher/small-merge-fix

Fix handling of small cardinality cases in CountThenEstimate merges

Author: cburroughs

src/main/java/com/clearspring/analytics/stream/cardinality/CountThenEstimate.java

@@ -333,14 +333,13 @@ public static CountThenEstimate mergeEstimators(CountThenEstimate... estimators)
 
             if(untipped.size() > 0)
             {
-                merged = new CountThenEstimate(0, untipped.get(0).builder);
-                merged.tip();
+                merged = new CountThenEstimate(untipped.get(0).tippingPoint, untipped.get(0).builder);
 
                 for(CountThenEstimate cte : untipped)
                 {
                     for(Object o : cte.counter)
                     {
-                        merged.estimator.offer(o);
+                        merged.offer(o);
                     }
                 }
             }
@@ -351,8 +350,15 @@ public static CountThenEstimate mergeEstimators(CountThenEstimate... estimators)
                 merged.estimator = tipped.remove(0);
             }
 
-            merged.estimator = merged.estimator.merge(tipped.toArray(new ICardinality[tipped.size()]));
-                    
+            if (!tipped.isEmpty())
+            {
+                if (!merged.tipped)
+                {
+                    merged.tip();
+                }
+                merged.estimator = merged.estimator.merge(tipped.toArray(new ICardinality[tipped.size()]));
+            }
+            
         }        
         return merged;
     }

src/test/java/com/clearspring/analytics/stream/cardinality/TestCountThenEstimate.java

@@ -78,6 +78,55 @@ public void testMerge() throws CardinalityMergeException
         assertEquals(0.01, error, 0.01);
 
 
+    }
+    
+    @Test
+    public void testSmallMerge() throws CardinalityMergeException
+    {
+        
+        // Untipped test case
+        int numToMerge = 1000;
+        int cardinalityPer = 5;
+
+        CountThenEstimate[] ctes = new CountThenEstimate[numToMerge];
+
+        for (int i = 0; i < numToMerge; i++)
+        {
+            ctes[i] = new CountThenEstimate(10000, AdaptiveCounting.Builder.obyCount(100000));
+            for (int j = 0; j < cardinalityPer; j++)
+            {
+                ctes[i].offer(Math.random());
+            }
+        }
+
+        int expectedCardinality = numToMerge * cardinalityPer;
+        CountThenEstimate merged = CountThenEstimate.mergeEstimators(ctes);
+        long mergedEstimate = merged.cardinality();
+        assertEquals(expectedCardinality, mergedEstimate);
+        assertFalse(merged.tipped);
+        
+        // Tipped test case
+        numToMerge = 10;
+        cardinalityPer = 100;
+
+        ctes = new CountThenEstimate[numToMerge];
+
+        for (int i = 0; i < numToMerge; i++)
+        {
+            ctes[i] = new CountThenEstimate(cardinalityPer + 1, AdaptiveCounting.Builder.obyCount(100000));
+            for (int j = 0; j < cardinalityPer; j++)
+            {
+                ctes[i].offer(Math.random());
+            }
+        }
+
+        expectedCardinality = numToMerge * cardinalityPer;
+        merged = CountThenEstimate.mergeEstimators(ctes);
+        mergedEstimate = merged.cardinality();
+        double error = Math.abs(mergedEstimate - expectedCardinality) / (double) expectedCardinality;
+        assertEquals(0.01, error, 0.01);
+        assertTrue(merged.tipped);
+
     }
 
     @Test