Skip to content

Commit

Permalink
Re-enables and fixes tests. All running now, many of which did not ru…
Browse files Browse the repository at this point in the history 
…n with aggressive search.

Signed-off-by: ammagamma <contactammagamma@gmail.com>
  • Loading branch information
@easbar
easbar committed Apr 21, 2018
1 parent 9585a51 commit e19a2eb
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 38 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/com/graphhopper/routing/ch/EdgeBasedNodeContractor.java
View file
Expand Up @@ -39,7 +39,7 @@
public class EdgeBasedNodeContractor extends AbstractNodeContractor { public class EdgeBasedNodeContractor extends AbstractNodeContractor {
// todo: modify code such that logging does not alter performance // todo: modify code such that logging does not alter performance
private static final Logger LOGGER = LoggerFactory.getLogger(EdgeBasedNodeContractor.class); private static final Logger LOGGER = LoggerFactory.getLogger(EdgeBasedNodeContractor.class);
// public static SearchType searchType = SearchType.AGGRESSIVE; // public static SearchType searchType = SearchType.AGGRESSIVE;
public static SearchType searchType = SearchType.SMART; public static SearchType searchType = SearchType.SMART;
public static boolean arrayBasedWitnessPathFinder = true; public static boolean arrayBasedWitnessPathFinder = true;
public static float edgeDifferenceWeight = 1; public static float edgeDifferenceWeight = 1;
Expand Down
27 changes: 11 additions & 16 deletions core/src/main/java/com/graphhopper/routing/ch/SmartWitnessPathFinder.java
View file
Expand Up @@ -91,12 +91,7 @@ public SmartWitnessSearchEntry runSearch(int toNode, int targetEdge) {
if (entry == null) { if (entry == null) {
continue; continue;
} }
double totalWeight = entry.weight + calcTurnWeight(incEdge, toNode, targetEdge); updateBestPath(toNode, targetEdge, entry);
if (totalWeight < bestWeight) {
bestWeight = totalWeight;
resIncEdge = incEdge;
resOnOrigPath = entry.onOrigPath;
}
} }


// run dijkstra to find the optimal path // run dijkstra to find the optimal path
Expand Down Expand Up @@ -161,7 +156,7 @@ public SmartWitnessSearchEntry runSearch(int toNode, int targetEdge) {
} }
entries.indexInsert(index, key, newEntry); entries.indexInsert(index, key, newEntry);
priorityQueue.add(newEntry); priorityQueue.add(newEntry);
updateBestPath(toNode, targetEdge, weight, newEntry); updateBestPath(toNode, targetEdge, newEntry);
} else { } else {
SmartWitnessSearchEntry existingEntry = entries.indexGet(index); SmartWitnessSearchEntry existingEntry = entries.indexGet(index);
if (weight < existingEntry.weight) { if (weight < existingEntry.weight) {
Expand All @@ -182,7 +177,7 @@ public SmartWitnessSearchEntry runSearch(int toNode, int targetEdge) {
} }
existingEntry.viaCenter = viaCenter; existingEntry.viaCenter = viaCenter;
priorityQueue.add(existingEntry); priorityQueue.add(existingEntry);
updateBestPath(toNode, targetEdge, weight, existingEntry); updateBestPath(toNode, targetEdge, existingEntry);
} }
} }
} }
Expand All @@ -199,16 +194,16 @@ public SmartWitnessSearchEntry runSearch(int toNode, int targetEdge) {
} }
} }


private void updateBestPath(int toNode, int targetEdge, double weight, SmartWitnessSearchEntry newEntry) { private void updateBestPath(int toNode, int targetEdge, SmartWitnessSearchEntry entry) {
// when we hit the target node we update the best path // when we hit the target node we update the best path
if (newEntry.adjNode == toNode) { if (entry.adjNode == toNode) {
double turnWeight = calcTurnWeight(newEntry.incEdge, toNode, targetEdge); double totalWeight = entry.weight + calcTurnWeight(entry.incEdge, toNode, targetEdge);
// when in doubt prefer a witness path over an original path // when in doubt prefer a witness path over an original path
double tolerance = newEntry.onOrigPath ? 0 : 1.e-6; double tolerance = entry.onOrigPath ? 0 : 1.e-6;
if (weight + turnWeight - tolerance < bestWeight) { if (totalWeight - tolerance < bestWeight) {
bestWeight = weight + turnWeight; bestWeight = totalWeight;
resIncEdge = newEntry.incEdge; resIncEdge = entry.incEdge;
resOnOrigPath = newEntry.onOrigPath; resOnOrigPath = entry.onOrigPath;
} }
} }
} }
Expand Down
41 changes: 20 additions & 21 deletions core/src/test/java/com/graphhopper/routing/ch/EdgeBasedNodeContractorTest.java
View file
Expand Up @@ -2,18 +2,25 @@


import com.graphhopper.Repeat; import com.graphhopper.Repeat;
import com.graphhopper.RepeatRule; import com.graphhopper.RepeatRule;
import com.graphhopper.routing.util.*; import com.graphhopper.routing.util.AllCHEdgesIterator;
import com.graphhopper.routing.util.CarFlagEncoder;
import com.graphhopper.routing.util.EncodingManager;
import com.graphhopper.routing.util.TraversalMode;
import com.graphhopper.routing.weighting.ShortestWeighting; import com.graphhopper.routing.weighting.ShortestWeighting;
import com.graphhopper.routing.weighting.TurnWeighting; import com.graphhopper.routing.weighting.TurnWeighting;
import com.graphhopper.routing.weighting.Weighting; import com.graphhopper.routing.weighting.Weighting;
import com.graphhopper.storage.*; import com.graphhopper.storage.*;
import com.graphhopper.util.*; import com.graphhopper.util.EdgeIterator;
import com.graphhopper.util.EdgeIteratorState;
import com.graphhopper.util.GHUtility;
import org.junit.Before; import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;


import java.util.*; import java.util.Arrays;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;


import static org.junit.Assert.*; import static org.junit.Assert.*;


Expand Down Expand Up @@ -332,14 +339,7 @@ public void testContractNode_duplicateOutgoingEdges_sameWeight() {
graph.freeze(); graph.freeze();
setMaxLevelOnAllNodes(); setMaxLevelOnAllNodes();
contractNodes(2); contractNodes(2);
// todo: we could prevent one of the two shortcuts, because once the first one is inserted it is a witness checkNumShortcuts(1);
// for the other one, but right now this does not work because we do not re-run the witness search after
// inserting the first shortcut. a better solution would probably be a clean-up after parsing the osm data where
// duplicate edges get removed. However, there should not be too many such duplicates
checkShortcuts(
createShortcut(1, 3, 1, 2, 1, 2, 2),
createShortcut(1, 3, 1, 3, 1, 3, 2)
);
} }


@Test @Test
Expand Down Expand Up @@ -761,7 +761,7 @@ public void testContractNode_witnessPathsAreFound() {
} }


@Test @Test
@Ignore("not sure how to fix this yet") // @Ignore("not sure how to fix this yet")
@Repeat(times = 10) @Repeat(times = 10)
public void testContractNode_noUnnecessaryShortcut_witnessPathOfEqualWeight() { public void testContractNode_noUnnecessaryShortcut_witnessPathOfEqualWeight() {
// this test runs repeatedly because it might pass/fail by chance (because path lengths are equal) // this test runs repeatedly because it might pass/fail by chance (because path lengths are equal)
Expand Down Expand Up @@ -919,7 +919,7 @@ public void testNodeContraction_directWitness() {
} }


@Test @Test
@Ignore("does not work for aggressive search") // @Ignore("does not work for aggressive search")
public void testNodeContraction_witnessBetterBecauseOfTurnCostAtTargetNode() { public void testNodeContraction_witnessBetterBecauseOfTurnCostAtTargetNode() {
// when we contract node 2 we should not stop searching for witnesses when edge 2->3 is settled, because then we miss // when we contract node 2 we should not stop searching for witnesses when edge 2->3 is settled, because then we miss
// the witness path via 5 that is found later, but still has less weight because of the turn costs at node 3 // the witness path via 5 that is found later, but still has less weight because of the turn costs at node 3
Expand Down Expand Up @@ -990,7 +990,7 @@ public void testNodeContraction_letShortcutsWitnessEachOther_twoOut() {
} }


@Test @Test
@Ignore("does not work for aggressive search") // @Ignore("does not work for aggressive search")
public void testNodeContraction_parallelEdges_onlyOneLoopShortcutNeeded() { public void testNodeContraction_parallelEdges_onlyOneLoopShortcutNeeded() {
// 0 -- 1 -- 2 // 0 -- 1 -- 2
// \--/ // \--/
Expand All @@ -1009,7 +1009,7 @@ public void testNodeContraction_parallelEdges_onlyOneLoopShortcutNeeded() {
} }


@Test @Test
@Ignore("does not work for aggressive search") // @Ignore("does not work for aggressive search")
public void testNodeContraction_duplicateEdge_severalLoops() { public void testNodeContraction_duplicateEdge_severalLoops() {
// 5 -- 4 -- 3 -- 1 // 5 -- 4 -- 3 -- 1
// |\ | // |\ |
Expand Down Expand Up @@ -1051,7 +1051,7 @@ public void testNodeContraction_duplicateEdge_severalLoops() {
} }


@Test @Test
@Ignore("does not work for aggressive search") // @Ignore("does not work for aggressive search")
public void testNodeContraction_tripleConnection() { public void testNodeContraction_tripleConnection() {
graph.edge(0, 1, 1.0, true); graph.edge(0, 1, 1.0, true);
graph.edge(0, 1, 2.0, true); graph.edge(0, 1, 2.0, true);
Expand All @@ -1067,7 +1067,6 @@ public void testNodeContraction_tripleConnection() {
} }


@Test @Test
@Ignore("does not work for aggressive search")
public void testNodeContraction_fromAndToNodesEqual() { public void testNodeContraction_fromAndToNodesEqual() {
// 0 -> 1 -> 3 // 0 -> 1 -> 3
// / \ // / \
Expand All @@ -1077,7 +1076,7 @@ public void testNodeContraction_fromAndToNodesEqual() {
graph.edge(0, 1, 1, false); graph.edge(0, 1, 1, false);
graph.edge(1, 2, 1, false); graph.edge(1, 2, 1, false);
graph.edge(2, 1, 1, false); graph.edge(2, 1, 1, false);
graph.edge(1, 3, 1, true); graph.edge(1, 3, 1, false);
graph.freeze(); graph.freeze();
setMaxLevelOnAllNodes(); setMaxLevelOnAllNodes();
contractNodes(2); contractNodes(2);
Expand Down Expand Up @@ -1111,7 +1110,7 @@ public void testNodeContraction_node_in_loop() {
} }


@Test @Test
@Ignore("does not work for aggressive search") // @Ignore("does not work for aggressive search")
public void testNodeContraction_turnRestrictionAndLoop() { public void testNodeContraction_turnRestrictionAndLoop() {
// /\ /<-3 // /\ /<-3
// 0 1--2 // 0 1--2
Expand Down Expand Up @@ -1154,7 +1153,7 @@ public void testNodeContraction_randomGraph_checkStatistics() {
GHUtility.printGraphForUnitTest(graph, encoder); GHUtility.printGraphForUnitTest(graph, encoder);
graph.freeze(); graph.freeze();
setMaxLevelOnAllNodes(); setMaxLevelOnAllNodes();

EdgeBasedNodeContractor nodeContractor = createNodeContractor(); EdgeBasedNodeContractor nodeContractor = createNodeContractor();
nodeContractor.contractNode(0); nodeContractor.contractNode(0);
int numEdgesPolled = nodeContractor.getNumPolledEdges(); int numEdgesPolled = nodeContractor.getNumPolledEdges();
Expand Down

0 comments on commit e19a2eb

Please sign in to comment.