improving precision (#393)

* improving precision

* refactor plus more tests

* 100 percent coverage for tree and boundingbox

* tests and coverage

* 100 per cent coverage for summarization

* 100 percent coverage for ErrorHandler

* refactor RCFCaster

Author: sudiptoguha

Java/core/src/main/java/com/amazon/randomcutforest/imputation/ConditionalSampleSummarizer.java

@@ -179,7 +179,7 @@ public SampleSummary summarize(List<ConditionalTreeSample> alist, boolean addTyp
         }
         int maxAllowed = min(queryPoint.length * MAX_NUMBER_OF_TYPICAL_PER_DIMENSION, MAX_NUMBER_OF_TYPICAL_ELEMENTS);
         maxAllowed = min(maxAllowed, num);
-        SampleSummary projectedSummary = Summarizer.summarize(typicalPoints, maxAllowed, num, false);
+        SampleSummary projectedSummary = Summarizer.l2summarize(typicalPoints, maxAllowed, num, false, 72);
 
         float[][] pointList = new float[projectedSummary.summaryPoints.length][];
         float[] likelihood = new float[projectedSummary.summaryPoints.length];

Java/core/src/main/java/com/amazon/randomcutforest/returntypes/RangeVector.java

@@ -92,7 +92,7 @@ public void shift(int i, float shift) {
 
     public void scale(int i, float weight) {
         checkArgument(i >= 0 && i < values.length, "incorrect index");
-        checkArgument(weight > 0, " negative weight not permitted");
+        checkArgument(weight >= 0, " negative weight not permitted");
         values[i] = values[i] * weight;
         // managing precision
         upper[i] = max(upper[i] * weight, values[i]);

Java/core/src/main/java/com/amazon/randomcutforest/summarization/Center.java

@@ -83,10 +83,6 @@ public double getWeight() {
         return weight;
     }
 
-    public boolean captureBeforeReset(float[] point, BiFunction<float[], float[], Double> distance) {
-        return previousWeight * distance.apply(point, representative) < 3 * previousSumOFRadius;
-    }
-
     // a standard reassignment using the median values and NOT the mean; the mean is
     // unlikely to
     // provide robust convergence
@@ -114,6 +110,9 @@ public double recompute(Function<Integer, float[]> getPoint, boolean approx,
                     break;
                 }
             }
+            if (position == assignedPoints.size()) {
+                position--;
+            }
             representative[index] = getPoint.apply(assignedPoints.get(position).index)[index];
         }
         for (int j = 0; j < assignedPoints.size(); j++) {
@@ -163,7 +162,9 @@ public void absorb(ICluster<float[]> other, BiFunction<float[], float[], Double>
     }
 
     public double distance(float[] point, BiFunction<float[], float[], Double> distance) {
-        return distance.apply(point, representative);
+        double t = distance.apply(point, representative);
+        checkArgument(t >= 0, "distance cannot be negative");
+        return t;
     }
 
     @Override

Java/core/src/main/java/com/amazon/randomcutforest/summarization/GenericMultiCenter.java

@@ -72,10 +72,6 @@ public class GenericMultiCenter<R> implements ICluster<R> {
         this.shrinkage = shrinkage;
     }
 
-    public static <R> GenericMultiCenter<R> initialize(R coordinate, float weight) {
-        return new GenericMultiCenter<>(coordinate, weight, DEFAULT_SHRINKAGE, DEFAULT_NUMBER_OF_REPRESENTATIVES);
-    }
-
     public static <R> GenericMultiCenter<R> initialize(R coordinate, float weight, double shrinkage,
             int numberOfRepresentatives) {
         checkArgument(shrinkage >= 0 && shrinkage <= 1.0, " parameter has to be in [0,1]");
@@ -130,10 +126,6 @@ public double getWeight() {
         return weight;
     }
 
-    public boolean captureBeforeReset(R point, BiFunction<R, R, Double> distanceFunction) {
-        return previousWeight * distance(point, distanceFunction) < 3 * previousSumOFRadius;
-    }
-
     // reassignment may not be meaningful for generic types, without additional
     // information
     public double recompute(Function<Integer, R> getPoint, boolean flag, BiFunction<R, R, Double> distanceFunction) {
@@ -191,7 +183,7 @@ public void absorb(ICluster<R> other, BiFunction<R, R, Double> distance) {
             savedRepresentatives.remove(farthestIndex);
         }
 
-        // absorb the remainder into existing represen tatives
+        // absorb the remainder into existing representatives
         for (Weighted<R> representative : savedRepresentatives) {
             double dist = distance.apply(representative.index, this.representatives.get(0).index);
             checkArgument(dist >= 0, "distance cannot be negative");

Java/core/src/main/java/com/amazon/randomcutforest/summarization/ICluster.java

@@ -48,16 +48,13 @@ public interface ICluster<R> {
     // weight computation
     double getWeight();
 
-    // is a point well expressed by the cluster? To be used in the future.
-    boolean captureBeforeReset(R point, BiFunction<R, R, Double> distance);
-
     // merge another cluster of same type
     void absorb(ICluster<R> other, BiFunction<R, R, Double> distance);
 
-    // distance of apoint from a cluster
+    // distance of apoint from a cluster, has to be non-negative
     double distance(R point, BiFunction<R, R, Double> distance);
 
-    // distance of another cluster from this cluster
+    // distance of another cluster from this cluster, has to be non negative
     double distance(ICluster<R> other, BiFunction<R, R, Double> distance);
 
     // all potential representativess of a cluster these are typically chosen to be

Java/core/src/main/java/com/amazon/randomcutforest/summarization/MultiCenter.java

@@ -33,10 +33,6 @@ public class MultiCenter extends GenericMultiCenter<float[]> {
         this.assignedPoints = new ArrayList<>();
     }
 
-    public static MultiCenter initialize(float[] coordinate, float weight) {
-        return new MultiCenter(coordinate, weight, DEFAULT_SHRINKAGE, DEFAULT_NUMBER_OF_REPRESENTATIVES);
-    }
-
     public static MultiCenter initialize(float[] coordinate, float weight, double shrinkage,
             int numberOfRepresentatives) {
         checkArgument(shrinkage >= 0 && shrinkage <= 1.0, " parameter has to be in [0,1]");
@@ -70,9 +66,9 @@ public double recompute(Function<Integer, float[]> getPoint, boolean force,
         previousSumOFRadius = sumOfRadius;
         sumOfRadius = 0;
         for (int j = 0; j < assignedPoints.size(); j++) {
+            // distance will check for -negative internally
             double addTerm = distance(getPoint.apply(assignedPoints.get(j).index), distanceFunction)
                     * assignedPoints.get(j).weight;
-            checkArgument(addTerm >= 0, "distances or weights cannot be negative");
             sumOfRadius += addTerm;
         }
         return (previousSumOFRadius - sumOfRadius);

Java/core/src/main/java/com/amazon/randomcutforest/summarization/Summarizer.java

@@ -105,8 +105,8 @@ public static <R> void assignAndRecompute(List<Weighted<Integer>> sampledPoints,
                 double minDist = Double.MAX_VALUE;
                 int minDistNbr = -1;
                 for (int i = 0; i < clusters.size(); i++) {
+                    // will check for negative distances
                     dist[i] = clusters.get(i).distance(getPoint.apply(point.index), distance);
-                    checkArgument(dist[i] >= 0, "distance cannot be negative");
                     if (minDist > dist[i]) {
                         minDist = dist[i];
                         minDistNbr = i;
@@ -201,10 +201,18 @@ public static <R> List<ICluster<R>> iterativeClustering(int maxAllowed, int init
             boolean phase2GlobalReassign, double overlapParameter, List<ICluster<R>> previousClustering) {
 
         checkArgument(refs.size() > 0, "empty list, nothing to do");
-        checkArgument(maxAllowed >= stopAt && stopAt > 0, "incorrect bounds on number of clusters");
+        checkArgument(stopAt > 0, "has to stop at 1 cluster");
+        checkArgument(stopAt <= maxAllowed, "cannot stop before achieving the limit");
 
         Random rng = new Random(seed);
-        double sampledSum = refs.stream().map(e -> (double) e.weight).reduce(Double::sum).get();
+        double sampledSum = refs.stream().map(e -> {
+            checkArgument(!Double.isNaN(e.weight), " weights have to be non-NaN");
+            checkArgument(Double.isFinite(e.weight), " weights have to be finite");
+            checkArgument(e.weight >= 0.0, () -> "negative weights are not meaningful" + e.weight);
+            return (double) e.weight;
+        }).reduce(0.0, Double::sum);
+        checkArgument(sampledSum > 0, " total weight has to be positive");
+
         ArrayList<ICluster<R>> centers = new ArrayList<>();
         if (refs.size() < 10 * (initial + 5)) {
             for (Weighted<Integer> point : refs) {
@@ -294,6 +302,8 @@ public static <R> List<ICluster<R>> iterativeClustering(int maxAllowed, int init
                 }
                 centers.sort(Comparator.comparingDouble(ICluster::getWeight));
                 while (centers.get(0).getWeight() == 0.0) {
+                    // this line is reachable via zeroTest() in
+                    // SampleSummaryTest
                     centers.remove(0);
                 }
                 if (inital < 1.2 * maxAllowed + 1) {
@@ -345,14 +355,14 @@ public static <R> List<ICluster<R>> summarize(List<Weighted<R>> points, int maxA
             List<ICluster<R>> previousClustering) {
         checkArgument(maxAllowed < 100, "are you sure you want more elements in the summary?");
         checkArgument(maxAllowed <= initial, "initial parameter should be at least maximum allowed in final result");
-        checkArgument(stopAt > 0 && stopAt <= maxAllowed, "lower bound set incorrectly");
 
         double totalWeight = points.stream().map(e -> {
-            checkArgument(e.weight >= 0.0, "negative weights are not meaningful");
+            checkArgument(!Double.isNaN(e.weight), " weights have to be non-NaN");
+            checkArgument(Double.isFinite(e.weight), " weights have to be finite");
+            checkArgument(e.weight >= 0.0, () -> "negative weights are not meaningful" + e.weight);
             return (double) e.weight;
         }).reduce(0.0, Double::sum);
-        checkArgument(!Double.isNaN(totalWeight) && Double.isFinite(totalWeight),
-                " weights have to finite and non-NaN");
+        checkArgument(totalWeight > 0, " total weight has to be positive");
         Random rng = new Random(seed);
         // the following list is explicity copied and sorted for potential efficiency
         List<Weighted<R>> sampledPoints = createSample(points, rng.nextLong(), 5 * LENGTH_BOUND, 0.005, 1.0);
@@ -363,8 +373,6 @@ public static <R> List<ICluster<R>> summarize(List<Weighted<R>> points, int maxA
         }
 
         Function<Integer, R> getPoint = (i) -> sampledPoints.get(i).index;
-        checkArgument(sampledPoints.size() > 0, "empty list, nothing to do");
-        double sampledSum = sampledPoints.stream().map(e -> (double) e.weight).reduce(Double::sum).get();
 
         return iterativeClustering(maxAllowed, initial, stopAt, refs, getPoint, distance, clusterInitializer,
                 rng.nextLong(), parallelEnabled, phase2GlobalReassign, overlapParameter, previousClustering);
@@ -403,11 +411,13 @@ public static SampleSummary summarize(List<Weighted<float[]>> points, int maxAll
         checkArgument(maxAllowed <= initial, "initial parameter should be at least maximum allowed in final result");
 
         double totalWeight = points.stream().map(e -> {
-            checkArgument(e.weight >= 0.0, "negative weights are not meaningful");
+            checkArgument(!Double.isNaN(e.weight), " weights have to be non-NaN");
+            checkArgument(Double.isFinite(e.weight), " weights have to be finite");
+            checkArgument(e.weight >= 0.0, () -> "negative weights are not meaningful" + e.weight);
             return (double) e.weight;
         }).reduce(0.0, Double::sum);
-        checkArgument(!Double.isNaN(totalWeight) && Double.isFinite(totalWeight),
-                " weights have to finite and non-NaN");
+        checkArgument(totalWeight > 0, " total weight has to be positive");
+
         Random rng = new Random(seed);
         // the following list is explicity copied and sorted for potential efficiency
         List<Weighted<float[]>> sampledPoints = createSample(points, rng.nextLong(), 5 * LENGTH_BOUND, 0.005, 1.0);
@@ -458,24 +468,24 @@ public static SampleSummary summarize(float[][] points, int maxAllowed, int init
      * @param maxAllowed      maximum number of groups/clusters
      * @param initial         a parameter controlling the initialization
      * @param reassignPerStep if reassignment is to be performed each step
+     * @param seed            random seed
      * @return a summarization
      */
-    public static SampleSummary summarize(List<Weighted<float[]>> points, int maxAllowed, int initial,
-            boolean reassignPerStep) {
-        return summarize(points, maxAllowed, initial, reassignPerStep, Summarizer::L2distance, new Random().nextLong(),
-                false);
+    public static SampleSummary l2summarize(List<Weighted<float[]>> points, int maxAllowed, int initial,
+            boolean reassignPerStep, long seed) {
+        return summarize(points, maxAllowed, initial, reassignPerStep, Summarizer::L2distance, seed, false);
     }
 
     /**
      * Same as above, with the most common use cases filled in
      *
      * @param points     points in float[][], each of weight 1.0
      * @param maxAllowed maximum number of clusters one is interested in
+     * @param seed       random seed
      * @return a summarization
      */
-    public static SampleSummary summarize(float[][] points, int maxAllowed) {
-        return summarize(points, maxAllowed, 4 * maxAllowed, false, Summarizer::L2distance, new Random().nextLong(),
-                false);
+    public static SampleSummary l2summarize(float[][] points, int maxAllowed, long seed) {
+        return summarize(points, maxAllowed, 4 * maxAllowed, false, Summarizer::L2distance, seed, false);
     }
 
     /**
@@ -529,9 +539,9 @@ public static <R> List<ICluster<R>> multiSummarize(R[] points, int maxAllowed, i
                 clusterInitializer, seed, parallelEnabled, null);
     }
 
-    // same as above, with defaults
+    // same as above, with multicenter instead of generic
     public static List<ICluster<float[]>> multiSummarize(float[][] points, int maxAllowed, double shrinkage,
-            int numberOfRepresentatives) {
+            int numberOfRepresentatives, long seed) {
 
         ArrayList<Weighted<float[]>> weighted = new ArrayList<>();
         for (float[] point : points) {
@@ -540,7 +550,7 @@ public static List<ICluster<float[]>> multiSummarize(float[][] points, int maxAl
         BiFunction<float[], Float, ICluster<float[]>> clusterInitializer = (a, b) -> MultiCenter.initialize(a, b,
                 shrinkage, numberOfRepresentatives);
         return summarize(weighted, maxAllowed, 4 * maxAllowed, 1, true, DEFAULT_SEPARATION_RATIO_FOR_MERGE,
-                Summarizer::L2distance, clusterInitializer, new Random().nextLong(), true, null);
+                Summarizer::L2distance, clusterInitializer, seed, true, null);
     }
 
 }

Java/core/src/main/java/com/amazon/randomcutforest/tree/BoundingBox.java

@@ -91,18 +91,6 @@ public BoundingBox getMergedBox(IBoundingBoxView otherBox) {
         return new BoundingBox(minValuesMerged, maxValuesMerged, sum);
     }
 
-    public void replaceBox(float[] point) {
-        System.arraycopy(point, 0, minValues, 0, point.length);
-        System.arraycopy(point, 0, maxValues, 0, point.length);
-        rangeSum = 0;
-    }
-
-    public void copyFrom(BoundingBox otherBox) {
-        System.arraycopy(otherBox.minValues, 0, minValues, 0, otherBox.minValues.length);
-        System.arraycopy(otherBox.maxValues, 0, maxValues, 0, otherBox.maxValues.length);
-        rangeSum = otherBox.rangeSum;
-    }
-
     public double probabilityOfCut(float[] point) {
         double range = 0;
         for (int i = 0; i < point.length; i++) {