fixes for issues 390,391 and test coverage (#392)

Author: sudiptoguha

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

@@ -55,6 +55,9 @@ public RangeVector(int dimensions) {
     public RangeVector(float[] values, float[] upper, float[] lower) {
         checkArgument(values.length > 0, " dimensions must be > 0");
         checkArgument(values.length == upper.length && upper.length == lower.length, "dimensions must be equal");
+        for (int i = 0; i < values.length; i++) {
+            checkArgument(upper[i] >= values[i] && values[i] >= lower[i], "incorrect semantics");
+        }
         this.values = Arrays.copyOf(values, values.length);
         this.upper = Arrays.copyOf(upper, upper.length);
         this.lower = Arrays.copyOf(lower, lower.length);

Java/core/src/main/java/com/amazon/randomcutforest/store/IndexIntervalManager.java

@@ -179,12 +179,4 @@ public void releaseIndex(int index) {
         lastInUse += 1;
     }
 
-    public int[] getFreeIndices() {
-        int[] answer = new int[2 * lastInUse];
-        for (int i = 0; i < 2 * lastInUse; i += 2) {
-            answer[i] = freeIndexesStart[i / 2];
-            answer[i + 1] = freeIndexesEnd[i / 2];
-        }
-        return answer;
-    }
 }

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

@@ -77,6 +77,8 @@ public boolean isInternal(int index) {
 
     public abstract int getRightIndex(int index);
 
+    public abstract int getParentIndex(int index);
+
     public abstract void setRoot(int index);
 
     protected abstract void decreaseMassOfInternalNode(int node);

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

@@ -15,6 +15,8 @@
 
 package com.amazon.randomcutforest.tree;
 
+import static com.amazon.randomcutforest.CommonUtils.checkArgument;
+
 import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Stack;
@@ -202,4 +204,9 @@ public int[] getRightIndex() {
         return Arrays.copyOf(rightIndex, rightIndex.length);
     }
 
+    public int getParentIndex(int index) {
+        checkArgument(parentIndex != null, "incorrect call");
+        return parentIndex[index];
+    }
+
 }

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

@@ -15,6 +15,7 @@
 
 package com.amazon.randomcutforest.tree;
 
+import static com.amazon.randomcutforest.CommonUtils.checkArgument;
 import static com.amazon.randomcutforest.CommonUtils.toCharArray;
 import static com.amazon.randomcutforest.CommonUtils.toIntArray;
 
@@ -136,6 +137,11 @@ public int getRightIndex(int index) {
         return rightIndex[index];
     }
 
+    public int getParentIndex(int index) {
+        checkArgument(parentIndex != null, "incorrect call");
+        return parentIndex[index];
+    }
+
     public void setRoot(int index) {
         if (!isLeaf(index) && parentIndex != null) {
             parentIndex[index] = (char) capacity;

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

@@ -141,6 +141,11 @@ public int getRightIndex(int index) {
         return rightIndex[index];
     }
 
+    public int getParentIndex(int index) {
+        checkArgument(parentIndex != null, "incorrect call");
+        return parentIndex[index];
+    }
+
     public void setRoot(int index) {
         if (!isLeaf(index) && parentIndex != null) {
             parentIndex[index] = (byte) capacity;

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

@@ -18,6 +18,7 @@
 import static com.amazon.randomcutforest.CommonUtils.checkArgument;
 import static com.amazon.randomcutforest.CommonUtils.checkNotNull;
 import static com.amazon.randomcutforest.CommonUtils.checkState;
+import static com.amazon.randomcutforest.tree.AbstractNodeStore.DEFAULT_STORE_PARENT;
 import static com.amazon.randomcutforest.tree.AbstractNodeStore.Null;
 import static java.lang.Math.max;
 
@@ -85,7 +86,8 @@ protected RandomCutTree(Builder<?> builder) {
         outputAfter = builder.outputAfter.orElse(max(1, numberOfLeaves / 4));
         dimension = (builder.dimension != 0) ? builder.dimension : pointStoreView.getDimensions();
         nodeStore = (builder.nodeStore != null) ? builder.nodeStore
-                : AbstractNodeStore.builder().capacity(numberOfLeaves - 1).dimension(dimension).build();
+                : AbstractNodeStore.builder().capacity(numberOfLeaves - 1).storeParent(builder.storeParent)
+                        .dimension(dimension).build();
         this.boundingBoxCacheFraction = builder.boundingBoxCacheFraction;
         this.storeSequenceIndexesEnabled = builder.storeSequenceIndexesEnabled;
         this.centerOfMassEnabled = builder.centerOfMassEnabled;
@@ -201,7 +203,7 @@ protected Cut randomCut(double factor, float[] point, BoundingBox box) {
         // debugging
         // this should be an anomaly - no pun intended.
 
-        Random rng = new Random((long) factor);
+        Random rng = new Random((long) (factor * Long.MAX_VALUE / 2));
         if (rng.nextDouble() < 0.5) {
             for (int i = 0; i < box.getDimensions(); i++) {
                 float minValue = (float) box.getMinValue(i);
@@ -357,7 +359,7 @@ protected void manageAncestorsAdd(Stack<int[]> path, float[] point) {
             }
             if (boundingBoxCacheFraction > 0.0) {
                 checkContainsAndRebuildBox(index, point, pointStoreView);
-                checkContainsAndAddPoint(index, point);
+                addPointInPlace(index, point);
             }
         }
     }
@@ -471,7 +473,7 @@ public boolean isLeaf(int index) {
     public boolean isInternal(int index) {
         // note that numberOfLeaves - 1 corresponds to an unspefied leaf in partial tree
         // 0 .. numberOfLeaves - 2 corresponds to internal nodes
-        return index < numberOfLeaves - 1;
+        return index < numberOfLeaves - 1 && index >= 0;
     }
 
     public int convertToLeaf(int pointIndex) {
@@ -571,9 +573,9 @@ void copyBoxToData(int idx, BoundingBox box) {
         rangeSumData[idx] = box.getRangeSum();
     }
 
-    boolean checkContainsAndAddPoint(int index, float[] point) {
+    void addPointInPlace(int index, float[] point) {
         int idx = translate(index);
-        if (idx != Integer.MAX_VALUE && rangeSumData[idx] != 0) {
+        if (idx != Integer.MAX_VALUE) {
             int base = 2 * idx * dimension;
             int mid = base + dimension;
             double rangeSum = 0;
@@ -586,11 +588,8 @@ boolean checkContainsAndAddPoint(int index, float[] point) {
             for (int i = 0; i < dimension; i++) {
                 rangeSum += boundingBoxData[mid + i] - boundingBoxData[base + i];
             }
-            boolean answer = (rangeSumData[idx] == rangeSum);
             rangeSumData[idx] = rangeSum;
-            return answer;
         }
-        return false;
     }
 
     public BoundingBox getBox(int index) {
@@ -639,7 +638,7 @@ boolean checkStrictlyContains(int index, float[] point) {
 
     boolean checkContainsAndRebuildBox(int index, float[] point, IPointStoreView<float[]> pointStoreView) {
         int idx = translate(index);
-        if (idx != Integer.MAX_VALUE && rangeSumData[idx] != 0) {
+        if (idx != Integer.MAX_VALUE) {
             if (!checkStrictlyContains(index, point)) {
                 BoundingBox mutatedBoundingBox = reconstructBox(index, pointStoreView);
                 copyBoxToData(idx, mutatedBoundingBox);
@@ -663,7 +662,7 @@ void addBox(int index, float[] point, BoundingBox box) {
             int idx = translate(index);
             if (idx != Integer.MAX_VALUE) { // always add irrespective of rangesum
                 copyBoxToData(idx, box);
-                checkContainsAndAddPoint(index, point);
+                addPointInPlace(index, point);
             }
         }
     }
@@ -1007,6 +1006,7 @@ public static class Builder<T extends Builder<T>> {
         protected IPointStoreView<float[]> pointStoreView;
         protected AbstractNodeStore nodeStore;
         protected int root = Null;
+        protected boolean storeParent = DEFAULT_STORE_PARENT;
 
         public T capacity(int capacity) {
             this.capacity = capacity;
@@ -1053,6 +1053,11 @@ public T setRoot(int root) {
             return (T) this;
         }
 
+        public T storeParent(boolean storeParent) {
+            this.storeParent = storeParent;
+            return (T) this;
+        }
+
         public T storeSequenceIndexesEnabled(boolean storeSequenceIndexesEnabled) {
             this.storeSequenceIndexesEnabled = storeSequenceIndexesEnabled;
             return (T) this;

Java/core/src/test/java/com/amazon/randomcutforest/returntypes/RangeVectorTest.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * or in the "license" file accompanying this file. This file 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 com.amazon.randomcutforest.returntypes;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class RangeVectorTest {
+
+    int dimensions;
+    private RangeVector vector;
+
+    @BeforeEach
+    public void setUp() {
+        dimensions = 3;
+        vector = new RangeVector(dimensions);
+    }
+
+    @Test
+    public void testNew() {
+        assertThrows(IllegalArgumentException.class, () -> new RangeVector(0));
+        assertThrows(IllegalArgumentException.class, () -> new RangeVector(new float[0]));
+        float[] expected = new float[dimensions];
+        assertArrayEquals(expected, vector.values);
+        assertArrayEquals(expected, vector.upper);
+        assertArrayEquals(expected, vector.lower);
+
+        float[] another = new float[0];
+        assertThrows(IllegalArgumentException.class, () -> new RangeVector(another, another, another));
+        assertThrows(IllegalArgumentException.class,
+                () -> new RangeVector(expected, expected, new float[dimensions + 1]));
+        assertThrows(IllegalArgumentException.class,
+                () -> new RangeVector(expected, new float[dimensions + 1], expected));
+        assertThrows(IllegalArgumentException.class,
+                () -> new RangeVector(new float[dimensions + 1], expected, expected));
+        assertDoesNotThrow(() -> new RangeVector(expected, expected, expected));
+
+        assertThrows(IllegalArgumentException.class,
+                () -> new RangeVector(expected, new float[] { -1f, 0f, 0f }, expected));
+        assertDoesNotThrow(() -> new RangeVector(expected, expected, new float[] { -1f, 0f, 0f }));
+
+        assertThrows(IllegalArgumentException.class,
+                () -> new RangeVector(expected, new float[] { 1f, 0f, 0f }, new float[] { 1f, 0f, 0f }));
+        assertDoesNotThrow(() -> new RangeVector(expected, new float[] { 1f, 0f, 0f }, new float[] { -1f, 0f, 0f }));
+    }
+
+    @Test
+    public void testScale() {
+        vector.upper[0] = 1.1f;
+        vector.upper[2] = 3.1f;
+        vector.upper[1] = 3.1f;
+        vector.lower[1] = -2.2f;
+
+        float z = 9.9f;
+        assertThrows(IllegalArgumentException.class, () -> vector.scale(0, -1.0f));
+        assertThrows(IllegalArgumentException.class, () -> vector.scale(-1, 1.0f));
+        assertThrows(IllegalArgumentException.class, () -> vector.scale(dimensions + 1, 1.0f));
+        vector.scale(0, z);
+
+        float[] expected = new float[] { 1.1f * 9.9f, 3.1f, 3.1f };
+        assertArrayEquals(expected, vector.upper, 1e-6f);
+
+        expected = new float[] { 0.0f, -2.2f, 0.0f };
+        assertArrayEquals(expected, vector.lower);
+
+        vector.scale(1, 2 * z);
+        assertArrayEquals(new float[] { 1.1f * 9.9f, 3.1f * 2 * z, 3.1f }, vector.upper, 1e-6f);
+        assertArrayEquals(new float[] { 0f, -2.2f * 2 * z, 0f }, vector.lower, 1e-6f);
+    }
+
+    @Test
+    public void testShift() {
+        vector.upper[0] = 1.1f;
+        vector.upper[2] = 3.1f;
+        vector.lower[1] = -2.2f;
+
+        float z = -9.9f;
+        assertThrows(IllegalArgumentException.class, () -> vector.shift(-1, z));
+        assertThrows(IllegalArgumentException.class, () -> vector.shift(dimensions + 1, z));
+        vector.shift(0, z);
+
+        float[] expected = new float[] { 1.1f - 9.9f, 0.0f, 3.1f };
+        assertArrayEquals(expected, vector.upper, 1e-6f);
+
+        expected = new float[] { z, -2.2f, 0.0f };
+        assertArrayEquals(expected, vector.lower);
+
+        assertArrayEquals(new float[] { z, 0, 0 }, vector.values, 1e-6f);
+    }
+
+}