Skip to content

Commit

Permalink
Fix/37 qt2 kNN returning deleted values. (#38)
Browse files Browse the repository at this point in the history
  • Loading branch information
tzaeschke authored Nov 19, 2023
1 parent e00cecc commit 3e768af
Show file tree
Hide file tree
Showing 10 changed files with 1,893 additions and 3 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

- Nothing yet

## [2.1.3] - 2023-11-19

- Fixed QUadtreeKD2 kNN finding previously deleted entries [#37](https://github.com/tzaeschke/tinspin-indexes/issue/37)

## [2.1.2] - 2023-10-31

### Fixed
Expand Down Expand Up @@ -151,7 +155,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Added CHANGELOG - [2015-10-11]
- Pushed to v1.3.3-SNAPSHOT - [2015-10-11]

[Unreleased]: https://github.com/tzaeschke/tinspin-indexes/compare/tinspin-indexes-2.1.2...HEAD
[Unreleased]: https://github.com/tzaeschke/tinspin-indexes/compare/tinspin-indexes-2.1.3...HEAD
[2.1.3]: https://github.com/tzaeschke/tinspin-indexes/compare/tinspin-indexes-2.1.2...tinspin-indexes-2.1.3
[2.1.2]: https://github.com/tzaeschke/tinspin-indexes/compare/tinspin-indexes-2.1.1...tinspin-indexes-2.1.2
[2.1.1]: https://github.com/tzaeschke/tinspin-indexes/compare/tinspin-indexes-2.1.0...tinspin-indexes-2.1.1
[2.1.0]: https://github.com/tzaeschke/tinspin-indexes/compare/tinspin-indexes-2.0.1...tinspin-indexes-2.1.0
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/tinspin/index/array/RectArray.java
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ private ArrayList<BoxEntryKnn<T>> knnQuery(double[] center, int k) {
for (int i = 0; i < phc.length/2; i++) {
double[] min = phc[i*2];
double[] max = phc[i*2+1];
if (min == null) {
continue; // Entry has been removed
}
double dist = distREdge(center, min, max);
if (ret.size() < k) {
ret.add(new BoxEntryKnn<>(min, max, values[i].value(), dist));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ private void findNextElement() {
}

if (node.isLeaf()) {
for (PointEntry<T> entry : node.getValues()) {
processEntry(entry);
for (int i = 0; i < node.getValueCount(); i++) {
processEntry(node.getValues()[i]);
}
} else {
for (Object o : node.getEntries()) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/tinspin/index/qthypercube2/QNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ private void removeValue(int pos) {
if (pos < --nValues) {
System.arraycopy(getValues(), pos+1, getValues(), pos, nValues-pos);
}
getValues()[nValues] = null;
} else {
nValues--;
subs[pos] = null;
Expand Down
61 changes: 61 additions & 0 deletions src/test/java/org/tinspin/index/test/BoxMapTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.tinspin.index.BoxMap;
import org.tinspin.index.Index;

import java.util.*;

Expand Down Expand Up @@ -265,6 +266,66 @@ public void testRemove() {
assertEquals(0, tree.size());
}

@Test
public void testIssueKnnRemove() {
if (candidate == IDX.COVER) {
return;
}
Random r = new Random(0);
int dim = 3;
int n = 1000;
int nDelete = n/2;
ArrayList<Entry> data = createInt(0, n, 3);
BoxMap<Entry> tree = createTree(data.size(), dim);

Collections.shuffle(data, r);

for (Entry e : data) {
tree.insert(e.p1, e.p2, e);
}

// remove 1st half
for (int i = 0; i < nDelete; ++i) {
Entry e = data.get(i);
assertNotNull(tree.remove(e.p1, e.p2));
assertNull(tree.remove(e.p1, e.p2));
assertFalse(containsExact(tree, e.p1, e.p2, e.id));
assertFalse(tree.contains(e.p1, e.p2));
}

// check contains() & kNN
for (int i = 0; i < nDelete; ++i) {
Entry e = data.get(i);
assertFalse(containsExact(tree, e.p1, e.p2, e.id));
assertFalse(tree.contains(e.p1, e.p2));
Index.BoxEntryKnn<Entry> eKnn = tree.query1nn(e.p1);
assertTrue(tree.contains(eKnn.min(), eKnn.max()));
}

// Issue #37: Entries remained referenced in arrays after removal.
// Moreover, the entry count was ignored by kNN, so it looked at the whole
// array instead of just the valid entries.
Index.BoxIteratorKnn<Entry> itKnn = tree.queryKnn(data.get(0).p1, n);
Set<Integer> kNNResult = new HashSet<>();
while (itKnn.hasNext()) {
Index.BoxEntryKnn<Entry> eKnn = itKnn.next();
kNNResult.add(eKnn.value().id);
}
for (int i = 0; i < n; i++) {
Entry e = data.get(i);
if (i < nDelete) {
assertFalse(containsExact(tree, e.p1, e.p2, e.id));
assertFalse(tree.contains(e.p1, e.p2));
assertFalse(kNNResult.contains(e.id));
} else {
assertTrue(containsExact(tree, e.p1, e.p2, e.id));
assertTrue(tree.contains(e.p1, e.p2));
assertTrue(kNNResult.contains(e.id));
}
}
assertEquals(n - nDelete, kNNResult.size());
}

private <T> BoxMap<T> createTree(int size, int dims) {
switch (candidate) {
case ARRAY:
Expand Down
61 changes: 61 additions & 0 deletions src/test/java/org/tinspin/index/test/BoxMultimapTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.tinspin.index.BoxMultimap;
import org.tinspin.index.Index;
import org.tinspin.index.util.MutableInt;

import java.util.*;
Expand Down Expand Up @@ -326,6 +327,66 @@ public void testRemoveIf() {
assertEquals(0, tree.size());
}

@Test
public void testIssueKnnRemove() {
if (candidate == IDX.COVER) {
return;
}
Random r = new Random(0);
int dim = 3;
int n = 1000;
int nDelete = n/2;
ArrayList<Entry> data = createInt(0, n, 3);
BoxMultimap<Entry> tree = createTree(data.size(), dim);

Collections.shuffle(data, r);

for (Entry e : data) {
tree.insert(e.p1, e.p2, e);
}

// remove 1st half
for (int i = 0; i < nDelete; ++i) {
Entry e = data.get(i);
assertTrue(tree.remove(e.p1, e.p2, e));
assertFalse(tree.remove(e.p1, e.p2, e));
assertFalse(containsExact(tree, e.p1, e.p2, e.id));
assertFalse(tree.contains(e.p1, e.p2, e));
}

// check contains() & kNN
for (int i = 0; i < nDelete; ++i) {
Entry e = data.get(i);
assertFalse(containsExact(tree, e.p1, e.p2, e.id));
assertFalse(tree.contains(e.p1, e.p2, e));
Index.BoxEntryKnn<Entry> eKnn = tree.query1nn(e.p1);
assertTrue(tree.contains(eKnn.min(), eKnn.max(), eKnn.value()));
}

// Issue #37: Entries remained referenced in arrays after removal.
// Moreover, the entry count was ignored by kNN, so it looked at the whole
// array instead of just the valid entries.
Index.BoxIteratorKnn<Entry> itKnn = tree.queryKnn(data.get(0).p1, n);
Set<Integer> kNNResult = new HashSet<>();
while (itKnn.hasNext()) {
Index.BoxEntryKnn<Entry> eKnn = itKnn.next();
kNNResult.add(eKnn.value().id);
}
for (int i = 0; i < n; i++) {
Entry e = data.get(i);
if (i < nDelete) {
assertFalse(containsExact(tree, e.p1, e.p2, e.id));
assertFalse(tree.contains(e.p1, e.p2, e));
assertFalse(kNNResult.contains(e.id));
} else {
assertTrue(containsExact(tree, e.p1, e.p2, e.id));
assertTrue(tree.contains(e.p1, e.p2, e));
assertTrue(kNNResult.contains(e.id));
}
}
assertEquals(n - nDelete, kNNResult.size());
}

private <T> BoxMultimap<T> createTree(int size, int dims) {
switch (candidate) {
case ARRAY:
Expand Down
145 changes: 145 additions & 0 deletions src/test/java/org/tinspin/index/test/Issue0037Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/*
* Copyright 2009-2017 Tilmann Zaeschke. All rights reserved.
*
* This file is part of TinSpin.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License 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 org.tinspin.index.test;

import org.junit.Test;
import org.tinspin.index.PointMap;
import org.tinspin.index.Stats;
import org.tinspin.index.qthypercube.QuadTreeKD;
import org.tinspin.index.qthypercube2.QuadTreeKD2;
import org.tinspin.index.qtplain.QuadTreeKD0;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Random;

import static org.junit.Assert.assertTrue;

public class Issue0037Test {

@Test
public void testQT() throws IOException {
QuadTreeKD<Integer> tree = QuadTreeKD.create(2);
testTree(tree);
}

@Test
public void testQT0() throws IOException {
QuadTreeKD0<Integer> tree = QuadTreeKD0.create(2);
testTree(tree);
}

@Test
public void testQT2() throws IOException {
QuadTreeKD2<Integer> tree = QuadTreeKD2.create(2);
testTree(tree);
}

/**
* p=20.7747859954834, -335.053844928741
* knn=[13.7747859954834, -335.053844928741]
* 1476
* true
* p=20.7747859954834, -335.053844928741
* knn=[24.7677965164185, -335.507710456848]
* 1476
* false
*
*
* p=20.7747859954834, -335.053844928741
* knn=[13.7747859954834, -335.053844928741]
* 1476
* true
*/
private void testTree(PointMap<Integer> tree) throws IOException {
File file = new File("src/test/resources/issue0037.txt");

BufferedReader reader = new BufferedReader(new FileReader(file));
int n = Integer.parseInt(reader.readLine());
for (int i = 0; i < n; i++) {
String[] sp = reader.readLine().split(" ");
double d1 = Double.parseDouble(sp[0]), d2 = Double.parseDouble(sp[1]);

tree.insert(new double[]{d1, d2}, i);
}
int k = Integer.parseInt(reader.readLine());
for (int i = 0; i < k; i++) {
String[] sp = reader.readLine().split(" ");
double d1 = Double.parseDouble(sp[0]), d2 = Double.parseDouble(sp[1]);
System.out.println("Remove: " + d1 + ", " + d2);
tree.remove(new double[]{d1, d2});
}
Stats stats = tree.getStats();
System.out.println("Stats: n=" + stats.nEntries);
{
String[] sp = reader.readLine().split(" ");
double d1 = Double.parseDouble(sp[0]), d2 = Double.parseDouble(sp[1]);
System.out.println("p=" + d1 + ", " + d2);
boolean has13_335 = tree.contains(new double[]{13.7747859954834, -335.053844928741});
System.out.println("contains: 13.7747859954834, -335.053844928741 ? " + has13_335);
var node = tree.query1nn(new double[]{d1, d2});
System.out.println("knn=" + Arrays.toString(node.point()));
boolean has = tree.contains(node.point());
System.out.println(n);

System.out.println(has); // Should print true, but prints false with QuadTreeKD2
assertTrue(has);
}
}

private void testTree2(PointMap<Integer> tree) throws IOException {
Random R = new Random(0);
int n = 100;
int k = n/2;

ArrayList<double[]> array = new ArrayList<>();
for (int i = 0; i < n; i++) {
double d1 = R.nextDouble(), d2 = R.nextDouble();
double[] p = new double[]{d1, d2};
array.add(p);
tree.insert(p, i);
}
for (int i = 0; i < k; i++) {
double d1 = R.nextDouble(), d2 = R.nextDouble();
tree.remove(new double[]{d1, d2});
}
// {
// String[] sp = reader.readLine().split(" ");
// double d1 = Double.parseDouble(sp[0]), d2 = Double.parseDouble(sp[1]);
// var node = tree.query1nn(new double[]{d1, d2});
// boolean has = tree.contains(node.point());
// System.out.println(n);
//
// System.out.println(has); // Should print true, but prints false with QuadTreeKD2
// assertTrue(has);
// }
for (int i = 0; i < n; i++) {
var node = tree.query1nn(array.get(i));
boolean has = tree.contains(node.point());
System.out.println(n);

System.out.println(has); // Should print true, but prints false with QuadTreeKD2
assertTrue(has);
}
}
}
Loading

0 comments on commit 3e768af

Please sign in to comment.