squash-12

CHANGELOG.md

@@ -8,10 +8,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 - Nothing yet
 
+## [2.1.4 - Unreleased]
+
+- Fixed tree corruption after remove() in QT2. [#40](https://github.com/tzaeschke/tinspin-indexes/issues/40)
+  - Fixed tree consistency (single-entry leaf after remove)
+  - Fixed tree consistency (nValues) -> verify
+  - Fixed bug in qt2.contains()
+- Fixed QT2 inconsistency after root resizing after insert(). [#42](https://github.com/tzaeschke/tinspin-indexes/issues/42)
+  Essentially, we enforce all radii and the center of the root to be a power of two.
+  This should immensely reduce and problems with precision errors. 
+
 ## [2.1.3] - 2023-11-19
 
 ### Fixed
-- Fixed QUadtreeKD2 kNN finding previously deleted entries [#37](https://github.com/tzaeschke/tinspin-indexes/issue/37)
+- Fixed QuadtreeKD2 kNN finding previously deleted entries [#37](https://github.com/tzaeschke/tinspin-indexes/issue/37)
 
 ## [2.1.2] - 2023-10-31
 

src/main/java/org/tinspin/index/Index.java

@@ -41,6 +41,7 @@ public interface Index {
 	/**
 	 * @return Collect and return some index statistics. Note that indexes are not required
 	 * to fill all fields. Also, individual indexes may use subclasses with additional fields.
+     * Many indexes also perform consistency checks while gathering stats.
 	 */
 	Stats getStats();
 

src/main/java/org/tinspin/index/Stats.java

@@ -27,6 +27,7 @@ public class Stats {
 	public int minLevel = Integer.MAX_VALUE;
 	public int maxLevel = -1;
 	public int maxDepth = 0;
+	public int maxValuesInNode = 0;
 	public double sumLevel;
 	public int maxNodeSize = -1;
 	public int nLeaf;
@@ -49,7 +50,9 @@ public String toString() {
 				";nNodes=" + nNodes +
 				";nLeaf=" + nLeaf + 
 				";nInner=" + nInner +
-				";minLevel=" + minLevel + 
+				";maxDepth=" + maxDepth +
+				";maxValues=" + maxValuesInNode +
+				";minLevel=" + minLevel +
 				";maxLevel=" + maxLevel + 
 				";avgLevel=" + (sumLevel/nEntries) +
 				";maxNodeSize=" + maxNodeSize;

src/main/java/org/tinspin/index/qthypercube/QNode.java

@@ -303,6 +303,7 @@ void checkNode(QStats s, QNode<T> parent, int depth) {
 			s.nLeaf++;
 			s.nEntries += values.size();
 			s.histoValues[values.size()]++;
+			s.maxValuesInNode = Math.max(s.maxValuesInNode, values.size());
 			for (int i = 0; i < values.size(); i++) {
 				PointEntry<T> e = values.get(i);
 				if (!QUtil.fitsIntoNode(e.point(), center, radius*QUtil.EPS_MUL)) {

src/main/java/org/tinspin/index/qthypercube/QRNode.java

@@ -350,6 +350,7 @@ void checkNode(QStats s, QRNode<T> parent, int depth) {
 			}
 		}
 		if (values != null) {
+			s.maxValuesInNode = Math.max(s.maxValuesInNode, values.size());
 			for (int i = 0; i < values.size(); i++) {
 				BoxEntry<T> e = values.get(i);
 				if (!QUtil.fitsIntoNode(e.min(), e.max(), center, radius*QUtil.EPS_MUL)) {

src/main/java/org/tinspin/index/qthypercube/QuadTreeKD.java

@@ -407,7 +407,7 @@ public QStats getStats() {
 	 * Statistics container class.
 	 */
 	public static class QStats extends Stats {
-		final int[] histoValues = new int[100];
+		final int[] histoValues = new int[1000];
 		final int[] histoSubs;
 
 		public QStats(int dims) {

src/main/java/org/tinspin/index/qthypercube2/QNode.java

@@ -26,7 +26,14 @@
 
 /**
  * Node class for the quadtree.
- * 
+ * <p>
+ * A node can be in one of two modes: "directory node" and "leaf node".
+ * Directory nodes have an 2^dim array of "subs", where ich entry can be one of: a point, a subnode, or null.
+ * Leaf nodes have an array of "values" which are all points.
+ * <p>
+ * A new subnode is inserted in "subs" if a slot in "subs" already contains a point.
+ *
+ *
  * @author ztilmann
  *
  * @param <T> Value type.
@@ -203,7 +210,11 @@ PointEntry<T> remove(QNode<T> parent, double[] key, int maxNodeSize, Predicate<P
 			int pos = calcSubPosition(key);
 			Object o = subs[pos];
 			if (o instanceof QNode) {
-				return ((QNode<T>)o).remove(this, key, maxNodeSize, pred);
+				PointEntry<T> removed = ((QNode<T>)o).remove(this, key, maxNodeSize, pred);
+				if (removed != null) {
+					checkAndMergeLeafNodesInParent(parent, maxNodeSize);
+				}
+				return removed;
 			} else if (o instanceof PointEntry) {
 				PointEntry<T> e = (PointEntry<T>) o;
 				if (removeSub(parent, key, pos, e, maxNodeSize, pred)) {
@@ -228,9 +239,7 @@ private boolean removeSub(
 			removeValue(pos);
 			//TODO provide threshold for re-insert
 			// i.e. do not always merge.
-			if (parent != null) {
-				parent.checkAndMergeLeafNodes(maxNodeSize);
-			}
+			checkAndMergeLeafNodesInParent(parent, maxNodeSize);
 			return true;
 		}
 		return false;
@@ -273,13 +282,11 @@ PointEntry<T> update(QNode<T> parent, double[] keyOld, double[] keyNew, int maxN
 					requiresReinsert[0] = false;
 				} else {
 					requiresReinsert[0] = true;
-					if (parent != null) {
-						parent.checkAndMergeLeafNodes(maxNodeSize);
-					}
+					checkAndMergeLeafNodesInParent(parent, maxNodeSize);
 				}
 				return qe;
 			}
-			throw new IllegalStateException();
+			return null;
 		}
 
 		for (int i = 0; i < nValues; i++) {
@@ -304,16 +311,37 @@ private void updateSub(double[] keyNew, PointEntry<T> e, QNode<T> parent, int ma
 			requiresReinsert[0] = true;
 			//TODO provide threshold for re-insert
 			//i.e. do not always merge.
-			if (parent != null) {
-				parent.checkAndMergeLeafNodes(maxNodeSize);
+			checkAndMergeLeafNodesInParent(parent, maxNodeSize);
+		}
+	}
+
+	private boolean checkMergeSingleLeaf(QNode<T> parent) {
+		// Merge single value into parent if  possible
+		if (!isLeaf() || parent == null || nValues > 1) {
+			return false;
+		}
+		for (int i = 0; i < parent.subs.length; i++) {
+			if (parent.subs[i] == this) {
+				parent.subs[i] = values[0];
+				parent.nValues++;
+				nValues = 0;
+				return true;
 			}
 		}
+		throw new IllegalStateException();
+	}
+
+	@SuppressWarnings("unchecked")
+	private void checkAndMergeLeafNodesInParent(QNode<T> parent, int maxNodeSize) {
+		if (!checkMergeSingleLeaf(parent) && parent != null) {
+			parent.checkAndMergeLeafNodes(maxNodeSize);
+		}
 	}
 
 	@SuppressWarnings("unchecked")
 	private void checkAndMergeLeafNodes(int maxNodeSize) {
 		//check: We start with including all local values: nValues
-		int nTotal = nValues;
+		int nTotal = 0;
 		for (int i = 0; i < subs.length; i++) {
 			Object e = subs[i];
 			if (e instanceof QNode) {
@@ -324,14 +352,16 @@ private void checkAndMergeLeafNodes(int maxNodeSize) {
 					return;
 				}
 				nTotal += sub.getValueCount();
-				if (nTotal > maxNodeSize) {
-					//too many children
-					return;
-				}
+			} else if (e instanceof PointEntry) {
+				nTotal++;
 			}
 		}
-
-		//okay, let's merge
+		if (nTotal > maxNodeSize) {
+			//too many children
+			return;
+		}
+
+		//okay, let's merge.
 		values = new PointEntry[nTotal];
 		nValues = 0;
 		for (int i = 0; i < subs.length; i++) {
@@ -366,7 +396,7 @@ PointEntry<T> getExact(double[] key, Predicate<PointEntry<T>> pred) {
 				return ((QNode<T>)sub).getExact(key, pred);
 			} else  if (sub != null) {
 				PointEntry<T> e = (PointEntry<T>) sub;
-				if (QUtil.isPointEqual(e.point(), key)) {
+				if (QUtil.isPointEqual(e.point(), key) && pred.test(e)) {
 					return e;
 				}
 			}
@@ -402,18 +432,23 @@ void checkNode(QStats s, QNode<T> parent, int depth) {
 
 		if (parent != null) {
 			if (!QUtil.isNodeEnclosed(center, radius, parent.center, parent.radius*QUtil.EPS_MUL)) {
+				System.out.println("Outer: " + parent.radius + " " + Arrays.toString(parent.center));
+				System.out.println("Child(" + depth + "): " + radius + " " + Arrays.toString(center));
 				for (int d = 0; d < center.length; d++) {
-//					if ((centerOuter[d]+radiusOuter) / (centerEnclosed[d]+radiusEnclosed) < 0.9999999 || 
-//							(centerOuter[d]-radiusOuter) / (centerEnclosed[d]-radiusEnclosed) > 1.0000001) {
-//						return false;
-//					}
-					System.out.println("Outer: " + parent.radius + " " + 
-						Arrays.toString(parent.center));
-					System.out.println("Child: " + radius + " " + Arrays.toString(center));
-					System.out.println((parent.center[d]+parent.radius) + " vs " + (center[d]+radius)); 
-					System.out.println("r=" + (parent.center[d]+parent.radius) / (center[d]+radius)); 
-					System.out.println((parent.center[d]-parent.radius) + " vs " + (center[d]-radius));
-					System.out.println("r=" + (parent.center[d]-parent.radius) / (center[d]-radius));
+					double parentMax = parent.center[d] + parent.radius;
+					double childMax = center[d] + radius;
+					double parentMin = parent.center[d] - parent.radius;
+					double childMin = center[d] - radius;
+					if (parentMax < childMax) {
+						System.out.println("DIM: " + d);
+						System.out.println("max: " + parentMax + " vs " + childMax);
+						System.out.println("  r: " + parentMax / childMax);
+					}
+					if (parentMin > childMin) {
+						System.out.println("DIM: " + d);
+						System.out.println("min: " + parentMin + " vs " + childMin);
+						System.out.println("  r: " + parentMin / childMin);
+					}
 				}
 				throw new IllegalStateException();
 			}
@@ -422,19 +457,25 @@ void checkNode(QStats s, QNode<T> parent, int depth) {
 			s.nLeaf++;
 			s.nEntries += nValues;
 			s.histoValues[nValues]++;
+			s.maxValuesInNode = Math.max(s.maxValuesInNode, nValues);
 			for (int i = 0; i < nValues; i++) {
 				PointEntry<T> e = values[i];
 				checkEntry(e);
 			}
 			if (subs != null) {
 				throw new IllegalStateException();
 			}
+			if (nValues < 2 && parent != null) {
+				// Leaf nodes (except the root) must contain at least two values.
+				throw new IllegalStateException();
+			}
 		} else {
 			s.nInner++;
 			if (subs.length != 1L<<s.dims) {
 				throw new IllegalStateException();
 			}
 			int nSubs = 0;
+			int nFoundValues = 0;
 			for (int i = 0; i < subs.length; i++) {
 				Object n = subs[i];
 				//TODO check pos
@@ -443,9 +484,14 @@ void checkNode(QStats s, QNode<T> parent, int depth) {
 					((QNode<T>)n).checkNode(s, this, depth+1);
 				} else if (n != null) {
 					s.nEntries++;
+					nFoundValues++;
 					checkEntry(n);
 				}
 			}
+			if (nValues != nFoundValues) {
+				throw new IllegalStateException();
+			}
+			s.histoValues[nFoundValues]++;
 			s.histo(nSubs);
 		}
 	}
@@ -457,10 +503,6 @@ private void checkEntry(Object o) {
 			System.out.println("Node: " + radius + " " + Arrays.toString(center));
 			System.out.println("Child: " + Arrays.toString(e.point()));
 			for (int d = 0; d < center.length; d++) {
-//				if ((centerOuter[d]+radiusOuter) / (centerEnclosed[d]+radiusEnclosed) < 0.9999999 || 
-//						(centerOuter[d]-radiusOuter) / (centerEnclosed[d]-radiusEnclosed) > 1.0000001) {
-//					return false;
-//				}
 				System.out.println("min/max for " + d);
 				System.out.println("min: " + (center[d]-radius) + " vs " + (e.point()[d]));
 				System.out.println("r=" + (center[d]-radius) / (e.point()[d]));

src/main/java/org/tinspin/index/qthypercube2/QuadTreeKD2.java

@@ -21,6 +21,7 @@
 import java.util.function.Predicate;
 
 import org.tinspin.index.*;
+import org.tinspin.index.util.MathTools;
 import org.tinspin.index.util.StringBuilderLn;
 
 /**
@@ -42,7 +43,7 @@
  * With version 1, there were many nodes with just one data entry because the parent directory 
  * node could not hold date entries. 
  * With version two a directory node (which contains a hypercube array) will only
- * create a subnode if a quadrant has to mhold more than one data entry.
+ * create a subnode if a quadrant has to hold more than one data entry.
  * 
  * 
  * @author ztilmann
@@ -108,7 +109,7 @@ public void insert(double[] key, T value) {
 		PointEntry<T> e = new PointEntry<>(key, value);
 		if (root == null) {
 			// We calculate a better radius when adding a second point.
-			root = new QNode<>(key.clone(), INITIAL_RADIUS);
+			root = new QNode<>(MathTools.floorPowerOfTwoCopy(key), INITIAL_RADIUS);
 		}
 		if (root.getRadius() == INITIAL_RADIUS) {
 			adjustRootSize(key);
@@ -127,16 +128,20 @@ private void adjustRootSize(double[] key) {
 			return;
 		}
 		if (root.getRadius() == INITIAL_RADIUS) {
-			double dist = PointDistance.L2.dist(key, root.getCenter());
-			if (dist > 0) {
-				root.adjustRadius(2 * dist);
+			double dMax = MathTools.maxDelta(key, root.getCenter());
+			for (int i = 0; i < root.getValueCount(); i++) {
+				dMax = Math.max(dMax, MathTools.maxDelta(root.getValues()[i].point(), root.getCenter()));
+			}
+			double radius = MathTools.ceilPowerOfTwo(dMax + QUtil.EPS_MUL);
+			if (radius > 0) {
+				root.adjustRadius(radius);
 			} else if (root.getValueCount() >= maxNodeSize - 1) {
-				// we just set an arbitrary radius here
+				// all entries have (approximately?) the same coordinates. We just set an arbitrary radius here.
 				root.adjustRadius(1000);
 			}
 		}
 	}
-	
+
 	/**
 	 * Check whether a given key exists.
 	 * @param key the key to check
@@ -378,6 +383,7 @@ private void toStringTree(StringBuilderLn sb, QNode<T> node,
 							  int depth, int posInParent) {
 		String prefix = ".".repeat(depth);
 		sb.append(prefix + posInParent + " d=" + depth);
+		sb.append(" nV=" + node.getValueCount());
 		sb.append(" " + Arrays.toString(node.getCenter()));
 		sb.appendLn("/" + node.getRadius());
 		prefix += " ";
@@ -418,7 +424,7 @@ public QStats getStats() {
 	 * Statistics container class.
 	 */
 	public static class QStats extends Stats {
-		final int[] histoValues = new int[100];
+		final int[] histoValues = new int[1000];
 		final int[] histoSubs;
 		static final int HISTO_MAX = (1 << 10) + 1;
 		public QStats(int dims) {

src/main/java/org/tinspin/index/qtplain/QNode.java

@@ -296,6 +296,7 @@ void checkNode(QStats s, QNode<T> parent, int depth) {
 			}
 		}
 		if (values != null) {
+			s.maxValuesInNode = Math.max(s.maxValuesInNode, values.size());
 			for (int i = 0; i < values.size(); i++) {
 				PointEntry<T> e = values.get(i);
 				if (!QUtil.fitsIntoNode(e.point(), center, radius*QUtil.EPS_MUL)) {

src/main/java/org/tinspin/index/qtplain/QRNode.java

@@ -389,6 +389,7 @@ void checkNode(QStats s, QRNode<T> parent, int depth) {
 			}
 		}
 		if (values != null) {
+			s.maxValuesInNode = Math.max(s.maxValuesInNode, values.size());
 			for (int i = 0; i < values.size(); i++) {
 				BoxEntry<T> e = values.get(i);
 				if (!QUtil.fitsIntoNode(e.min(), e.max(), center, radius*QUtil.EPS_MUL)) {