Skip to content

Commit

Permalink
Bugfix for base boundaries and fix for stackoverflow
Browse files Browse the repository at this point in the history
When dist happens to be on exact base boundaries, the wrong decision was being made. For example when the distance between a and b is exactly 1, it will not correctly insert the second node. Leading to wrong size() computation and wrong neighbor retrieval.

And, nodes toString, equals and hashCode yield StackOverflowError because they used parent and children.
  • Loading branch information
Daniel Tischner committed Nov 4, 2019
1 parent acd252e commit d0eecc5
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 20 deletions.
10 changes: 5 additions & 5 deletions de.zabuza.closy/src/de/zabuza/closy/internal/CoverTree.java
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ public Collection<E> getKNearestNeighbors(final E point, final int k) {
// Create a set of nearest neighbor candidates
final double greatestMinDist = Objects.requireNonNull(minDistances.peek());
for (final Node<E> nextCandidate : nextCandidates) {
if (nextCandidate.getDistance() < greatestMinDist + Math.pow(base, level)) {
if (nextCandidate.getDistance() <= greatestMinDist + Math.pow(base, level)) {
candidates.add(nextCandidate);
}
}
Expand Down Expand Up @@ -292,7 +292,7 @@ public Optional<E> getNearestNeighbor(final E point) {

// Create a set of nearest neighbor candidates
for (final Node<E> nextCandidate : nextCandidates) {
if (nextCandidate.getDistance() < minDist + Math.pow(base, level)) {
if (nextCandidate.getDistance() <= minDist + Math.pow(base, level)) {
candidates.add(nextCandidate);
}
}
Expand Down Expand Up @@ -338,7 +338,7 @@ public Collection<E> getNeighborhood(final E point, final double range) {

// Create a set of nearest neighbor candidates
for (final Node<E> nextCandidate : nextCandidates) {
if (nextCandidate.getDistance() < range + Math.pow(base, level)) {
if (nextCandidate.getDistance() <= range + Math.pow(base, level)) {
candidates.add(nextCandidate);
}
}
Expand Down Expand Up @@ -399,7 +399,7 @@ public boolean add(final E element) {
child.setDistance(node.getDistance());
}

if (child.getDistance() < Math.pow(base, level)) {
if (child.getDistance() <= Math.pow(base, level)) {
candidates.add(child);
parentFound = false;
}
Expand All @@ -414,7 +414,7 @@ public boolean add(final E element) {

// Select one node of the cover-set as the parent of the node
for (final Node<E> node : coverset) {
if (node.getDistance() < Math.pow(base, level)) {
if (node.getDistance() <= Math.pow(base, level)) {
parent = node;
parentLevel = level;
break;
Expand Down
29 changes: 14 additions & 15 deletions de.zabuza.closy/src/de/zabuza/closy/internal/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ final class Node<E> implements Comparable<Node<E>> {
children = new ArrayList<>();
}

/**
* Compares nodes ascending based on their distance.
*/
@Override
public int compareTo(final Node<E> o) {
//noinspection CompareToUsesNonFinalVariable
return Double.compare(distance, o.getDistance());
}

@Override
public boolean equals(final Object obj) {
if (this == obj) {
Expand All @@ -52,29 +61,19 @@ public boolean equals(final Object obj) {
return false;
}
final Node<?> node = (Node<?>) obj;
//noinspection ChainedMethodCall
return Double.compare(node.getDistance(), getDistance()) == 0 && getChildren().equals(node.getChildren())
&& Objects.equals(getElement(), node.getElement()) && Objects.equals(getParent(), node.getParent());
}

/**
* Compares nodes ascending based on their distance.
*/
@Override
public int compareTo(final Node<E> o) {
//noinspection CompareToUsesNonFinalVariable
return Double.compare(distance, o.getDistance());
return Double.compare(node.getDistance(), getDistance()) == 0 && Objects.equals(getElement(),
node.getElement());
}

@Override
public int hashCode() {
return Objects.hash(getChildren(), getDistance(), getElement(), getParent());
return Objects.hash(getDistance(), getElement());
}

@Override
public String toString() {
return "Node{" + "children=" + children + ", distance=" + distance + ", element=" + element + ", parent="
+ parent + '}';
return "Node{" + "children size=" + children.size() + ", distance=" + distance + ", element=" + element
+ ", has parent=" + (parent != null) + '}';
}

/**
Expand Down

0 comments on commit d0eecc5

Please sign in to comment.