Skip to content

Commit

Permalink
Landmark bugs (#1745)
Browse files Browse the repository at this point in the history
* Fix first test with andis fix

* Fixes landmark issue; fixes #1687

* Fixes test so that it tests the relevant thing

* Omg, the setter takes the scaled value, but the getter returns the unscaled value

* Add the test

* for approximation use query node

* fixed auto refactoring
  • Loading branch information
michaz authored and karussell committed Oct 8, 2019
1 parent 0faf680 commit e1d5452
Show file tree
Hide file tree
Showing 4 changed files with 248 additions and 26 deletions.
27 changes: 12 additions & 15 deletions core/src/main/java/com/graphhopper/routing/lm/LMApproximator.java
Expand Up @@ -33,12 +33,12 @@
*/
public class LMApproximator implements WeightApproximator {
private static class VirtEntry {
private int node;
private int towerNode;
private int weight;

@Override
public String toString() {
return node + ", " + weight;
return towerNode + ", " + weight;
}
}

Expand All @@ -49,7 +49,7 @@ public String toString() {
private int[] activeFromIntWeights;
private int[] activeToIntWeights;
private double epsilon = 1;
private int to = -1;
private int toTowerNode = -1;
// do activate landmark recalculation
private boolean doALMRecalc = true;
private final double factor;
Expand Down Expand Up @@ -92,13 +92,13 @@ public LMApproximator(Graph graph, int maxBaseNodes, LandmarkStorage lms, int ac
VirtEntry virtEntry = new VirtEntry();
if (weight < Integer.MAX_VALUE && (reverseWeight >= Integer.MAX_VALUE || weight < reverseWeight)) {
virtEntry.weight = weight;
virtEntry.node = reverse ? edge.getBaseNode() : edge.getAdjNode();
virtEntry.towerNode = reverse ? edge.getBaseNode() : edge.getAdjNode();
} else {
virtEntry.weight = reverseWeight;
if (reverseWeight >= Integer.MAX_VALUE)
throw new IllegalStateException("At least one direction of edge (" + edge + ") should be accessible but wasn't!");

virtEntry.node = reverse ? edge.getAdjNode() : edge.getBaseNode();
virtEntry.towerNode = reverse ? edge.getAdjNode() : edge.getBaseNode();
}

virtNodeMap.put(idxVirtNode, virtEntry);
Expand All @@ -119,31 +119,31 @@ public double approximate(final int queryNode) {
if (!doALMRecalc && fallback || lms.isEmpty())
return fallBackApproximation.approximate(queryNode);

int node = queryNode;
int towerNode = queryNode;
int virtEdgeWeightInt = 0;
if (queryNode >= maxBaseNodes) {
// handle virtual node
VirtEntry virtEntry = virtNodeMap.get(queryNode);
node = virtEntry.node;
towerNode = virtEntry.towerNode;
virtEdgeWeightInt = virtEntry.weight;
}

if (node == to)
if (towerNode == toTowerNode)
return 0;

// select better active landmarks, LATER: use 'success' statistics about last active landmark
// we have to update the priority queues and the maps if done in the middle of the search http://cstheory.stackexchange.com/q/36355/13229
if (doALMRecalc) {
doALMRecalc = false;
boolean res = lms.initActiveLandmarks(node, to, activeLandmarks, activeFromIntWeights, activeToIntWeights, reverse);
boolean res = lms.initActiveLandmarks(towerNode, toTowerNode, activeLandmarks, activeFromIntWeights, activeToIntWeights, reverse);
if (!res) {
// note: fallback==true means forever true!
fallback = true;
return fallBackApproximation.approximate(queryNode);
}
}

int maxWeightInt = getMaxWeight(node, virtEdgeWeightInt, activeLandmarks, activeFromIntWeights, activeToIntWeights);
int maxWeightInt = getMaxWeight(towerNode, virtEdgeWeightInt, activeLandmarks, activeFromIntWeights, activeToIntWeights);
if (maxWeightInt < 0) {
// allow negative weight for now until we have more precise approximation (including query graph)
return 0;
Expand Down Expand Up @@ -194,13 +194,10 @@ int getMaxWeight(int node, int virtEdgeWeightInt, int[] activeLandmarks, int[] a
return maxWeightInt;
}

final int getNode(int node) {
return node >= maxBaseNodes ? virtNodeMap.get(node).node : node;
}

@Override
public void setTo(int to) {
this.to = getNode(to);
this.fallBackApproximation.setTo(to);
this.toTowerNode = to >= maxBaseNodes ? virtNodeMap.get(to).towerNode : to;
}

@Override
Expand Down
Expand Up @@ -503,6 +503,11 @@ int getToWeight(int landmarkIndex, int node) {
return Integer.MAX_VALUE;
// throw new IllegalStateException("Do not call getToWeight for wrong landmark[" + landmarkIndex + "]=" + landmarkIDs[landmarkIndex] + " and node " + node);

// If delta is 'maxed out' (minned out, really), we can only return 0, since we can't give a better
// under-approximation of the weight, since it can be arbitrarily smaller than 'from'.
if (delta == DELTA_MIN)
return 0;

//the right bits of "res" store the backward value
int from = res & FROM_WEIGHT_INF;

Expand Down
Expand Up @@ -100,18 +100,14 @@ public void testSetGetWeight() {
lms.setWeight(0, 0, 16, 999999, true);
assertEquals((int) Math.pow(2, 18) - 2, lms.getFromWeight(0, 0));

/* FROM_WEIGHT_BITS = 18 --> remaining bits: 32-18 = 14
The delta value is signed and will therefore go from -2^(14-1) to 2^(14-1).
Now 2^13-1 is used for infinity, 2^13-2 as maximum and -2^13 as minimum.
If the difference between forward and backward weight is too large it will use
the maximum (if positive) or the minimum (if negative) instead of 0 or infinity
The delta will then be added to the backward weight*/
lms.setWeight(0, 0, 16, 999999, false);
assertEquals((int) (Math.pow(2, 18) - 2 + Math.pow(2, 13) - 2), lms.getToWeight(0, 0));
// {backward weight} {delta weight}
// The important property is that the weight that is returned
// is _not bigger_ than the weight that was set.
assertTrue(lms.getToWeight(0, 0) * lms.getFactor() <= 999999);
lms.setWeight(0, 0, 16, 1, false);
assertEquals((int) (Math.pow(2, 18) - 2 + -Math.pow(2, 13)), lms.getToWeight(0, 0));
// {backward weight} {delta weight}
// This is an underrun of the 'delta' field -- but still, the important property is that the weight that is returned
// is _not bigger_ than the weight that was set.
assertTrue(lms.getToWeight(0, 0) * lms.getFactor() <= 1);

da.setInt(0, Integer.MAX_VALUE);
assertTrue(lms.isInfinity(0));
Expand Down Expand Up @@ -317,6 +313,5 @@ public void testDeltaWarning() {
assertEquals(9, storage.getLandmarks(1)[1]);

assertEquals((int) Math.pow(2, 13) - 2, storage.getToWeight(0, 9) - storage.getFromWeight(0, 9));
assertEquals((int) -Math.pow(2, 13), storage.getToWeight(1, 12) - storage.getFromWeight(1, 12));
}
}
@@ -0,0 +1,225 @@
package com.graphhopper.routing.weighting;

import com.graphhopper.RepeatRule;
import com.graphhopper.routing.*;
import com.graphhopper.routing.ch.PrepareContractionHierarchies;
import com.graphhopper.routing.lm.PrepareLandmarks;
import com.graphhopper.routing.profiles.DecimalEncodedValue;
import com.graphhopper.routing.util.CarFlagEncoder;
import com.graphhopper.routing.util.EncodingManager;
import com.graphhopper.routing.util.TraversalMode;
import com.graphhopper.storage.*;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.util.*;

import static com.graphhopper.util.Parameters.Algorithms.ASTAR_BI;
import static com.graphhopper.util.Parameters.Algorithms.DIJKSTRA_BI;
import static org.junit.Assert.fail;

/**
* This test compares the different bidirectional routing algorithms with {@link DijkstraBidirectionRef}
* // todo: no real need of emphasizing bidirectional here ?
*
* @author easbar
*/
@RunWith(Parameterized.class)
public class BidirectionalRoutingTest {
private final Algo algo;
private final boolean prepareCH;
private final boolean prepareLM;
private Directory dir;
private GraphHopperStorage graph;
private CHGraph chGraph;
private CarFlagEncoder encoder;
private Weighting weighting;
private EncodingManager encodingManager;
private PrepareContractionHierarchies pch;
private PrepareLandmarks lm;

@Rule
public RepeatRule repeatRule = new RepeatRule();

@Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> params() {
// todonow: run node & edge-based ?
return Arrays.asList(new Object[][]{
{Algo.ASTAR, false, false},
{Algo.CH_ASTAR, true, false},
{Algo.CH_DIJKSTRA, true, false},
{Algo.LM, false, true}
});
}

private enum Algo {
ASTAR,
CH_ASTAR,
CH_DIJKSTRA,
LM
}

public BidirectionalRoutingTest(Algo algo, boolean prepareCH, boolean prepareLM) {
this.algo = algo;
this.prepareCH = prepareCH;
this.prepareLM = prepareLM;
}

@Before
public void init() {
dir = new RAMDirectory();
// todonow: make this work with speed_both_directions=true!
encoder = new CarFlagEncoder(5, 5, 0);
encodingManager = EncodingManager.create(encoder);
weighting = new FastestWeighting(encoder);
graph = createGraph();
chGraph = graph.getCHGraph();
}

private void preProcessGraph() {
graph.freeze();
if (prepareCH) {
pch = new PrepareContractionHierarchies(chGraph);
pch.doWork();
}
if (prepareLM) {
lm = new PrepareLandmarks(dir, graph, weighting, 16, 8);
lm.setMaximumWeight(1000);
lm.doWork();
}
}

private AbstractBidirAlgo createAlgo() {
return createAlgo(prepareCH ? chGraph : graph);
}

private AbstractBidirAlgo createAlgo(Graph graph) {
switch (algo) {
case ASTAR:
return new AStarBidirection(graph, weighting, TraversalMode.NODE_BASED);
case CH_DIJKSTRA:
return (AbstractBidirAlgo) pch.createAlgo(graph, AlgorithmOptions.start().weighting(weighting).algorithm(DIJKSTRA_BI).build());
case CH_ASTAR:
return (AbstractBidirAlgo) pch.createAlgo(graph, AlgorithmOptions.start().weighting(weighting).algorithm(ASTAR_BI).build());
case LM:
AStarBidirection astarbi = new AStarBidirection(graph, weighting, TraversalMode.NODE_BASED);
return (AbstractBidirAlgo) lm.getDecoratedAlgorithm(graph, astarbi, AlgorithmOptions.start().build());
default:
throw new IllegalArgumentException("unknown algo " + algo);
}
}

@Test
public void lm_problem_to_node_of_fallback_approximator() {
// this test would fail because when the distance is approximated for the start node 0 the LMApproximator
// uses the fall back approximator for which the to node is never set. This in turn means that the to coordinates
// are zero and a way too large approximation is returned. Eventually the best path is not updated correctly
// because the spt entry of the fwd search already has a way too large weight.

// ---<---
// | |
// | 4 |
// |/ \ 0
// 1 | |
// \ | |
// 3 |
// 2 --<----
DecimalEncodedValue speedEnc = encoder.getAverageSpeedEnc();
NodeAccess na = graph.getNodeAccess();
na.setNode(0, 49.405150, 9.709054);
na.setNode(1, 49.403705, 9.700517);
na.setNode(2, 49.400112, 9.700209);
na.setNode(3, 49.403009, 9.708364);
na.setNode(4, 49.409021, 9.703622);
// 30s
graph.edge(4, 3, 1000, true).set(speedEnc, 120);
graph.edge(0, 2, 1000, false).set(speedEnc, 120);
// 360s
graph.edge(1, 3, 1000, true).set(speedEnc, 10);
// 80s
graph.edge(0, 1, 1000, false).set(speedEnc, 45);
graph.edge(1, 4, 1000, true).set(speedEnc, 45);
preProcessGraph();

int source = 0;
int target = 3;

Path refPath = new DijkstraBidirectionRef(graph, weighting, TraversalMode.NODE_BASED)
.calcPath(source, target);
Path path = createAlgo()
.calcPath(0, 3);
comparePaths(refPath, path, source, target);
}

@Test
public void lm_issue2() {
// This would fail because an underrun of 'delta' would not be treated correctly, and the remaining
// weight would be over-approximated

// ---
// / \
// 0 - 1 - 5 - 6 - 9 - 4 - 0
// \ /
// ->-
NodeAccess na = graph.getNodeAccess();
DecimalEncodedValue speedEnc = encoder.getAverageSpeedEnc();
na.setNode(0, 49.406987, 9.709767);
na.setNode(1, 49.403612, 9.702953);
na.setNode(2, 49.409755, 9.706517);
na.setNode(3, 49.409021, 9.708649);
na.setNode(4, 49.400674, 9.700906);
na.setNode(5, 49.408735, 9.709486);
na.setNode(6, 49.406402, 9.700937);
na.setNode(7, 49.406965, 9.702660);
na.setNode(8, 49.405227, 9.702863);
na.setNode(9, 49.409411, 9.709085);
graph.edge(0, 1, 623.197000, true).set(speedEnc, 112);
graph.edge(5, 1, 741.414000, true).set(speedEnc, 13);
graph.edge(9, 4, 1140.835000, true).set(speedEnc, 35);
graph.edge(5, 6, 670.689000, true).set(speedEnc, 18);
graph.edge(5, 9, 80.731000, false).set(speedEnc, 88);
graph.edge(0, 9, 273.948000, true).set(speedEnc, 82);
graph.edge(4, 0, 956.552000, true).set(speedEnc, 60);
preProcessGraph();
int source = 5;
int target = 4;
Path refPath = new DijkstraBidirectionRef(graph, weighting, TraversalMode.NODE_BASED)
.calcPath(source, target);
Path path = createAlgo()
.calcPath(source, target);
comparePaths(refPath, path, source, target);
}


private List<String> comparePaths(Path refPath, Path path, int source, int target) {
List<String> strictViolations = new ArrayList<>();
double refWeight = refPath.getWeight();
double weight = path.getWeight();
if (Math.abs(refWeight - weight) > 1.e-2) {
System.out.println("expected: " + refPath.calcNodes());
System.out.println("given: " + path.calcNodes());
fail("wrong weight: " + source + "->" + target + ", expected: " + refWeight + ", given: " + weight);
}
if (Math.abs(path.getDistance() - refPath.getDistance()) > 1.e-1) {
strictViolations.add("wrong distance " + source + "->" + target + ", expected: " + refPath.getDistance() + ", given: " + path.getDistance());
}
if (Math.abs(path.getTime() - refPath.getTime()) > 50) {
strictViolations.add("wrong time " + source + "->" + target + ", expected: " + refPath.getTime() + ", given: " + path.getTime());
}
if (!refPath.calcNodes().equals(path.calcNodes())) {
strictViolations.add("wrong nodes " + source + "->" + target + "\nexpected: " + refPath.calcNodes() + "\ngiven: " + path.calcNodes());
}
return strictViolations;
}

private GraphHopperStorage createGraph() {
GraphHopperStorage gh = new GraphHopperStorage(Collections.singletonList(weighting), dir, encodingManager,
false, new GraphExtension.NoOpExtension());
gh.create(1000);
return gh;
}

}

0 comments on commit e1d5452

Please sign in to comment.