Improvement of landmark weight precision, fixes graphhopper#1080 (graphhopper#1376)

* Changed how the weights are stored

I changed the way the landmark weights are stored from having two shorts (one for the forward and one for the backward value) to having 18 bits for the backward value and 14 bits for the difference between theese two.

* Added some tests

* Update LandmarkStorage.java

* Update LandmarkStorageTest.java

* Updated LandmarkStorage.java

* dos2unix

* make compiling again

* revert changes from ammagamma

* avoid warnings in tests and do not limit factor to 0.1

* ensure correct version is used for landmark storage

* fix warning

Author: baumboi

core/src/main/java/com/graphhopper/routing/lm/LandmarkStorage.java

@@ -61,8 +61,6 @@ public class LandmarkStorage implements Storable<LandmarkStorage> {
     // one node has an associated landmark information ('one landmark row'): the forward and backward weight
     private long LM_ROW_LENGTH;
     private int landmarks;
-    private final int FROM_OFFSET;
-    private final int TO_OFFSET;
     private final DataAccess landmarkWeightDA;
     /* every subnetwork has its own landmark mapping but the count of landmarks is always the same */
     private final List<int[]> landmarkIDs;
@@ -74,15 +72,11 @@ public class LandmarkStorage implements Storable<LandmarkStorage> {
     private Weighting lmSelectionWeighting;
     private final TraversalMode traversalMode;
     private boolean initialized;
-    private int minimumNodes = 500_000;
+    private int minimumNodes;
     private final SubnetworkStorage subnetworkStorage;
     private List<LandmarkSuggestion> landmarkSuggestions = Collections.emptyList();
     private SpatialRuleLookup ruleLookup;
     private boolean logDetails = false;
-    /**
-     * 'to' and 'from' fit into 32 bit => 16 bit for each of them => 65536
-     */
-    static final long PRECISION = 1 << 16;
 
     public LandmarkStorage(GraphHopperStorage graph, Directory dir, final Weighting weighting, int landmarks) {
         this.graph = graph;
@@ -117,12 +111,14 @@ public String toString() {
         this.landmarks = landmarks;
         // one short per landmark and two directions => 2*2 byte
         this.LM_ROW_LENGTH = landmarks * 4;
-        this.FROM_OFFSET = 0;
-        this.TO_OFFSET = 2;
         this.landmarkIDs = new ArrayList<>();
         this.subnetworkStorage = new SubnetworkStorage(dir, "landmarks_" + name);
     }
 
+    public int getVersion() {
+        return 1;
+    }
+
     /**
      * Specify the maximum possible value for your used area. With this maximum weight value you can influence the storage
      * precision for your weights that help A* finding its way to the goal. The same value is used for all subnetworks.
@@ -214,8 +210,8 @@ public void createLandmarks() {
         this.landmarkWeightDA.create(2000);
         this.landmarkWeightDA.ensureCapacity(maxBytes);
 
-        for (long pointer = 0; pointer < maxBytes; pointer += 2) {
-            landmarkWeightDA.setShort(pointer, (short) SHORT_INFINITY);
+        for (long pointer = 0; pointer < maxBytes; pointer += 4) {
+            landmarkWeightDA.setInt(pointer, (DELTA_INF << FROM_WEIGHT_BITS) | FROM_WEIGHT_INF);
         }
 
         String additionalInfo = "";
@@ -253,7 +249,7 @@ public void createLandmarks() {
         if (ruleLookup != null && ruleLookup.size() > 0) {
             StopWatch sw = new StopWatch().start();
             blockedEdges = findBorderEdgeIds(ruleLookup);
-            tarjanFilter = new BlockedEdgesFilter(encoder, false, true, blockedEdges);
+            tarjanFilter = new BlockedEdgesFilter(encoder, true, false, blockedEdges);
             if (logDetails)
                 LOGGER.info("Made " + blockedEdges.size() + " edges inaccessible. Calculated country cut in " + sw.stop().getSeconds() + "s, " + Helper.getMemInfo());
         }
@@ -308,12 +304,14 @@ public void createLandmarks() {
             }
         }
 
-        landmarkWeightDA.setHeader(0 * 4, graph.getNodes());
+        // make backward incompatible to force rebuilt (pre 0.11 releases had nodes count at 0)
+        landmarkWeightDA.setHeader(0 * 4, getVersion());
         landmarkWeightDA.setHeader(1 * 4, landmarks);
         landmarkWeightDA.setHeader(2 * 4, subnetworkCount);
         if (factor * DOUBLE_MLTPL > Integer.MAX_VALUE)
             throw new UnsupportedOperationException("landmark weight factor cannot be bigger than Integer.MAX_VALUE " + factor * DOUBLE_MLTPL);
         landmarkWeightDA.setHeader(3 * 4, (int) Math.round(factor * DOUBLE_MLTPL));
+        landmarkWeightDA.setHeader(4 * 4, graph.getNodes());
 
         // serialize fast byte[] into DataAccess
         subnetworkStorage.create(graph.getNodes());
@@ -407,9 +405,9 @@ private boolean createLandmarksForSubnetwork(final int startNode, final byte[] s
             int lmNodeId = tmpLandmarkNodeIds[lmIdx];
             LandmarkExplorer explorer = new LandmarkExplorer(graph, this, weighting, traversalMode, true);
             explorer.setStartNode(lmNodeId);
-            explorer.setFilter(blockedEdges, false, true);
+            explorer.setFilter(blockedEdges, true, false);
             explorer.runAlgo();
-            explorer.initLandmarkWeights(lmIdx, lmNodeId, LM_ROW_LENGTH, FROM_OFFSET);
+            explorer.initLandmarkWeights(lmIdx, lmNodeId, LM_ROW_LENGTH);
 
             // set subnetwork id to all explored nodes, but do this only for the first landmark
             if (lmIdx == 0) {
@@ -419,9 +417,9 @@ private boolean createLandmarksForSubnetwork(final int startNode, final byte[] s
 
             explorer = new LandmarkExplorer(graph, this, weighting, traversalMode, false);
             explorer.setStartNode(lmNodeId);
-            explorer.setFilter(blockedEdges, true, false);
+            explorer.setFilter(blockedEdges, false, true);
             explorer.runAlgo();
-            explorer.initLandmarkWeights(lmIdx, lmNodeId, LM_ROW_LENGTH, TO_OFFSET);
+            explorer.initLandmarkWeights(lmIdx, lmNodeId, LM_ROW_LENGTH);
 
             if (lmIdx == 0) {
                 if (explorer.setSubnetworks(subnetworks, subnetworkId))
@@ -479,58 +477,102 @@ protected IntHashSet findBorderEdgeIds(SpatialRuleLookup ruleLookup) {
      * a node ID but the internal index of the landmark array.
      */
     int getFromWeight(int landmarkIndex, int node) {
-        int res = (int) landmarkWeightDA.getShort((long) node * LM_ROW_LENGTH + landmarkIndex * 4 + FROM_OFFSET)
-                & 0x0000FFFF;
-        assert res >= 0 : "Negative to weight " + res + ", landmark index:" + landmarkIndex + ", node:" + node;
-        if (res == SHORT_INFINITY)
-            // TODO can happen if endstanding oneway
-            // we should set a 'from' value to SHORT_MAX if the 'to' value was already set to find real bugs
-            // and what to return? Integer.MAX_VALUE i.e. convert to Double.pos_infinity upstream?
-            return SHORT_MAX;
+        //only the right bits of this integer store the backward value
+        int res = landmarkWeightDA.getInt((long) node * LM_ROW_LENGTH + landmarkIndex * 4) & FROM_WEIGHT_INF;
+
+        if (res == FROM_WEIGHT_INF)
+            return Integer.MAX_VALUE;
         // throw new IllegalStateException("Do not call getFromWeight for wrong landmark[" + landmarkIndex + "]=" + landmarkIDs[landmarkIndex] + " and node " + node);
-        // TODO if(res == MAX) fallback to beeline approximation!?
 
+        assert res >= 0 : "Negative backward weight " + res + ", landmark index:" + landmarkIndex + ", node:" + node;
         return res;
     }
 
     /**
      * @return the weight from the specified node to the landmark (specified *as index*)
      */
     int getToWeight(int landmarkIndex, int node) {
-        int res = (int) landmarkWeightDA.getShort((long) node * LM_ROW_LENGTH + landmarkIndex * 4 + TO_OFFSET)
-                & 0x0000FFFF;
-        assert res >= 0 : "Negative to weight " + res + ", landmark index:" + landmarkIndex + ", node:" + node;
-        if (res == SHORT_INFINITY)
-            return SHORT_MAX;
-//            throw new IllegalStateException("Do not call getToWeight for wrong landmark[" + landmarkIndex + "]=" + landmarkIDs[landmarkIndex] + " and node " + node);
+        int res = landmarkWeightDA.getInt((long) node * LM_ROW_LENGTH + landmarkIndex * 4);
+
+        //the left bits of "res" store the difference between forward and backward value
+        int delta = res >> FROM_WEIGHT_BITS;
 
+        if (delta == DELTA_INF)
+            return Integer.MAX_VALUE;
+        // throw new IllegalStateException("Do not call getToWeight for wrong landmark[" + landmarkIndex + "]=" + landmarkIDs[landmarkIndex] + " and node " + node);
+
+        //the right bits of "res" store the backward value
+        int from = res & FROM_WEIGHT_INF;
+
+        if (from == FROM_WEIGHT_INF) {
+            from = DELTA_INF + 1;
+        }
+
+        //to get the forward value you have to add the backward to the delta value
+        res = from + delta;
+
+        assert res >= 0 : "Negative forward weight " + res + ", landmark index:" + landmarkIndex + ", node:" + node;
         return res;
     }
 
-    // Short.MAX_VALUE = 2^15-1 but we have unsigned short so we need 2^16-1
-    private static final int SHORT_INFINITY = Short.MAX_VALUE * 2 + 1;
-    // We have large values that do not fit into a short, use a specific maximum value
-    private static final int SHORT_MAX = SHORT_INFINITY - 1;
+    // 'to' and 'from' fit into 32 bit => 16 bit for each of them => 65536
+    static final long PRECISION = 1 << 16;
+    /* This value sets the amount of bits used to store the backward weight.
+    The rest of overall 32 bits stores the difference between forward and backward weight*/
+    private static final int FROM_WEIGHT_BITS = 18;
+    // The backward weight is unsigned --> 2^x - 1
+    private static final int FROM_WEIGHT_INF = (int) Math.pow(2, FROM_WEIGHT_BITS) - 1;
+    // This value will be used if the backward weight is too large
+    private static final int FROM_WEIGHT_MAX = FROM_WEIGHT_INF - 1;
+    /* The difference between forward and backward weight is signed
+    --> 2^(31-x) - 1 instead of 2^(32-x) - 1*/
+    private static final int DELTA_INF = (int) Math.pow(2, 31 - FROM_WEIGHT_BITS) - 1;
+    // This value will be used if the difference between these weights is too large and forward > backward
+    private static final int DELTA_MAX = DELTA_INF - 1;
+    // This value will be used if the difference between these weights is too large and forward < backward
+    private static final int DELTA_MIN = -DELTA_INF - 1;
 
     /**
-     * @return false if the value capacity was reached and instead of the real value the SHORT_MAX was stored.
+     * @return false if the value capacity was reached and instead of the real value the MAX was stored.
      */
-    final boolean setWeight(long pointer, double value) {
+    final boolean setWeight(int lmIdx, int nodeId, long rowSize, double value, boolean from) {
         double tmpVal = value / factor;
         if (tmpVal > Integer.MAX_VALUE)
-            throw new UnsupportedOperationException("Cannot store infinity explicitely, pointer=" + pointer + ", value: " + value);
+            throw new UnsupportedOperationException("Cannot store infinity explicitly, landmark: " + lmIdx + ", node: " + nodeId + ", value: " + value);
 
-        if (tmpVal >= SHORT_MAX) {
-            landmarkWeightDA.setShort(pointer, (short) SHORT_MAX);
-            return false;
+        if (from) {
+            if (tmpVal >= FROM_WEIGHT_MAX) {
+                landmarkWeightDA.setInt(nodeId * rowSize + lmIdx * 4, (DELTA_INF << FROM_WEIGHT_BITS) | FROM_WEIGHT_MAX);
+                return false;
+            } else {
+                landmarkWeightDA.setInt(nodeId * rowSize + lmIdx * 4, (DELTA_INF << FROM_WEIGHT_BITS) | (int) tmpVal);
+                return true;
+            }
         } else {
-            landmarkWeightDA.setShort(pointer, (short) tmpVal);
-            return true;
+            int fromWeight = getFromWeight(lmIdx, nodeId);
+            int delta;
+            if (fromWeight == Integer.MAX_VALUE) {
+                fromWeight = FROM_WEIGHT_INF;
+                delta = (int) tmpVal - DELTA_INF + 1;
+            } else {
+                delta = (int) tmpVal - fromWeight;
+            }
+
+            if (delta >= DELTA_MAX) {
+                landmarkWeightDA.setInt(nodeId * rowSize + lmIdx * 4, (DELTA_MAX << FROM_WEIGHT_BITS) | fromWeight);
+                return false;
+            } else if (delta <= DELTA_MIN) {
+                landmarkWeightDA.setInt(nodeId * rowSize + lmIdx * 4, (DELTA_MIN << FROM_WEIGHT_BITS) | fromWeight);
+                return false;
+            } else {
+                landmarkWeightDA.setInt(nodeId * rowSize + lmIdx * 4, (delta << FROM_WEIGHT_BITS) | fromWeight);
+                return true;
+            }
         }
     }
 
     boolean isInfinity(long pointer) {
-        return ((int) landmarkWeightDA.getShort(pointer) & 0x0000FFFF) == SHORT_INFINITY;
+        return (landmarkWeightDA.getInt(pointer) & FROM_WEIGHT_INF) == FROM_WEIGHT_INF;
     }
 
     int calcWeight(EdgeIteratorState edge, boolean reverse) {
@@ -661,9 +703,13 @@ public boolean loadExisting() {
             if (!subnetworkStorage.loadExisting())
                 throw new IllegalStateException("landmark weights loaded but not the subnetworks!?");
 
-            int nodes = landmarkWeightDA.getHeader(0 * 4);
+            int version = landmarkWeightDA.getHeader(0 * 4);
+            if (version != getVersion())
+                throw new IllegalArgumentException("Cannot load landmark data due to incompatible version. Storage used version: " + version + ", expected: " + getVersion());
+            int nodes = landmarkWeightDA.getHeader(4 * 4);
             if (nodes != graph.getNodes())
                 throw new IllegalArgumentException("Cannot load landmark data as written for different graph storage with " + nodes + " nodes, not " + graph.getNodes());
+
             landmarks = landmarkWeightDA.getHeader(1 * 4);
             int subnetworks = landmarkWeightDA.getHeader(2 * 4);
             factor = landmarkWeightDA.getHeader(3 * 4) / DOUBLE_MLTPL;
@@ -728,7 +774,7 @@ public LandmarkExplorer(Graph g, LandmarkStorage lms, Weighting weighting, Trave
             super(g, weighting, tMode);
             this.lms = lms;
             this.from = from;
-            // set one of the bi directions as already finished            
+            // set one of the bi directions as already finished
             if (from)
                 finishedTo = true;
             else
@@ -737,8 +783,15 @@ public LandmarkExplorer(Graph g, LandmarkStorage lms, Weighting weighting, Trave
             setUpdateBestPath(false);
         }
 
-        public void setFilter(IntHashSet set, boolean bwd, boolean fwd) {
-            EdgeFilter ef = new BlockedEdgesFilter(flagEncoder, bwd, fwd, set);
+        public void setStartNode(int startNode) {
+            if (from)
+                initFrom(startNode, 0);
+            else
+                initTo(startNode, 0);
+        }
+
+        void setFilter(IntHashSet set, boolean fwd, boolean bwd) {
+            EdgeFilter ef = new BlockedEdgesFilter(flagEncoder, fwd, bwd, set);
             outEdgeExplorer = graph.createEdgeExplorer(ef);
             inEdgeExplorer = graph.createEdgeExplorer(ef);
         }
@@ -755,13 +808,6 @@ public int getLastNode() {
             return lastNode;
         }
 
-        public void setStartNode(int startNode) {
-            if (from)
-                initFrom(startNode, 0);
-            else
-                initTo(startNode, 0);
-        }
-
         public void runAlgo() {
             super.runAlgo();
         }
@@ -777,7 +823,7 @@ public boolean finished() {
             }
         }
 
-        public boolean setSubnetworks(final byte[] subnetworks, final int subnetworkId) {
+        boolean setSubnetworks(final byte[] subnetworks, final int subnetworkId) {
             if (subnetworkId > 127)
                 throw new IllegalStateException("Too many subnetworks " + subnetworkId);
 
@@ -805,15 +851,15 @@ public boolean apply(int nodeId, SPTEntry value) {
             return failed.get();
         }
 
-        public void initLandmarkWeights(final int lmIdx, int lmNodeId, final long rowSize, final int offset) {
+        public void initLandmarkWeights(final int lmIdx, int lmNodeId, final long rowSize) {
             IntObjectMap<SPTEntry> map = from ? bestWeightMapFrom : bestWeightMapTo;
             final AtomicInteger maxedout = new AtomicInteger(0);
             final Map.Entry<Double, Double> finalMaxWeight = new MapEntry<>(0d, 0d);
 
             map.forEach(new IntObjectProcedure<SPTEntry>() {
                 @Override
                 public void apply(int nodeId, SPTEntry b) {
-                    if (!lms.setWeight(nodeId * rowSize + lmIdx * 4 + offset, b.weight)) {
+                    if (!lms.setWeight(lmIdx, nodeId, rowSize, b.weight, from)) {
                         maxedout.incrementAndGet();
                         finalMaxWeight.setValue(Math.max(b.weight, finalMaxWeight.getValue()));
                     }
@@ -822,8 +868,8 @@ public void apply(int nodeId, SPTEntry b) {
 
             if ((double) maxedout.get() / map.size() > 0.1) {
                 LOGGER.warn("landmark " + lmIdx + " (" + nodeAccess.getLatitude(lmNodeId) + "," + nodeAccess.getLongitude(lmNodeId) + "): " +
-                        "too many weights were maxed out (" + maxedout.get() + "/" + map.size() + "). Use a bigger factor than " + lms.factor
-                        + ". For example use the following in the config.yml: weighting=" + weighting.getName() + "|maximum=" + finalMaxWeight.getValue() * 1.2);
+                        "too many " + (from ? "backward" : "delta") + " weights were maxed out (" + maxedout.get() + "/" + map.size() + "). Factor is too small " + lms.factor
+                        + ". To fix this increase maximum in config.yml: prepare.lm.weighting: " + weighting.getName() + "|maximum=" + finalMaxWeight.getValue() * 1.2);
             }
         }
     }
@@ -838,7 +884,7 @@ public int compare(Map.Entry<Integer, Integer> o1, Map.Entry<Integer, Integer> o
         }
     };
 
-    static GHPoint createPoint(Graph graph, int nodeId) {
+    private static GHPoint createPoint(Graph graph, int nodeId) {
         return new GHPoint(graph.getNodeAccess().getLatitude(nodeId), graph.getNodeAccess().getLongitude(nodeId));
     }
 
@@ -862,7 +908,7 @@ private static class BlockedEdgesFilter implements EdgeFilter {
         private final boolean fwd;
         private final boolean bwd;
 
-        public BlockedEdgesFilter(FlagEncoder encoder, boolean bwd, boolean fwd, IntHashSet blockedEdges) {
+        public BlockedEdgesFilter(FlagEncoder encoder, boolean fwd, boolean bwd, IntHashSet blockedEdges) {
             this.encoder = encoder;
             this.bwd = bwd;
             this.fwd = fwd;
@@ -888,4 +934,4 @@ public String toString() {
             return encoder.toString() + ", bwd:" + bwd + ", fwd:" + fwd;
         }
     }
-}
+}