Revert ignored highways for now

CHANGELOG.md

@@ -1,16 +1,12 @@
 ### 7.0 [not yet released]
 
-- there is a new, required 'import.osm.ignored_highways' configuration option that must be used to not increase the
-  graph size and decrease performance for motorized-only routing compared to previous versions, #2702
 - new osm_way_id encoded value, #2701
-- the parameters vehicle, weighting, edge_based and turn_costs are no longer supported, use the profile parameter
-  instead
+- the parameters vehicle, weighting, edge_based and turn_costs are no longer supported, use the profile parameter instead
 - removed motorroad to road_class conversion, #2329
 - removed YAML support for custom models on the server-side. Only allow JSON with // comments.
 - Bike2WeightTagParser was removed. Use the bike vehicle with a custom model, see custom_models/bike2.json
 - CurvatureWeighting was removed. Use a custom model with 'curvature' instead, see custom_models/curvature.json (#2665)
-- internal keys for EdgeKVStorage changed to contain the street_ prefix like the path details too. Similarly, the
-  extra_info in the instructions of the API response, see #2661
+- internal keys for EdgeKVStorage changed to contain the street_ prefix like the path details too. Similarly, the extra_info in the instructions of the API response, see #2661
 
 ### 6.0 [13 Sep 2022]
 

benchmark/benchmark.sh

@@ -53,7 +53,6 @@ measurement.ch.node=true \
 measurement.ch.edge=true \
 measurement.lm=false \
 measurement.vehicle=car \
-import.osm.ignored_highways=footway,cycleway,path,pedestrian,bridleway \
 measurement.turn_costs=true \
 graph.location=${GRAPH_DIR}measurement-small-gh \
 prepare.min_network_size=10000 \
@@ -82,7 +81,6 @@ measurement.lm=true \
 "measurement.lm.active_counts=[4,8,12]" \
 measurement.lm.edge_based=true \
 measurement.vehicle=car \
-import.osm.ignored_highways=footway,cycleway,path,pedestrian,bridleway \
 measurement.turn_costs=true \
 graph.location=${GRAPH_DIR}measurement-big-gh \
 prepare.min_network_size=10000 \
@@ -114,7 +112,6 @@ measurement.lm=true \
 "measurement.lm.active_counts=[8]" \
 measurement.lm.edge_based=false \
 measurement.vehicle=car \
-import.osm.ignored_highways=footway,cycleway,path,pedestrian,bridleway \
 measurement.turn_costs=true \
 graph.location=${GRAPH_DIR}measurement-big-little-custom-gh \
 prepare.min_network_size=10000 \
@@ -146,7 +143,6 @@ measurement.lm=true \
 "measurement.lm.active_counts=[8]" \
 measurement.lm.edge_based=false \
 measurement.vehicle=car \
-import.osm.ignored_highways=footway,cycleway,path,pedestrian,bridleway \
 measurement.turn_costs=true \
 graph.location=${GRAPH_DIR}measurement-big-very-custom-gh \
 prepare.min_network_size=10000 \
@@ -175,7 +171,6 @@ measurement.lm=true \
 "measurement.lm.active_counts=[4,8,12]" \
 measurement.lm.edge_based=false \
 measurement.vehicle=foot \
-import.osm.ignored_highways= \
 measurement.turn_costs=false \
 graph.location=${GRAPH_DIR}measurement-big-outdoor-gh \
 prepare.min_network_size=10000 \

config-example.yml

@@ -40,7 +40,6 @@ graphhopper:
   #   u_turn_costs: 60
 
   # - name: bike
-      # to use the bike vehicle make sure to not ignore cycleways etc., see import.osm.ignored_highways below
   #   vehicle: bike
   #   weighting: custom
   #   # the custom model in bike2.json is defined to avoid hills
@@ -169,14 +168,6 @@ graphhopper:
 
   #### Storage ####
 
-  # Excludes certain types of highways during the OSM import to speed up the process and reduce the size of the graph.
-  # A typical application is excluding 'footway','cycleway','path' and maybe 'pedestrian' and 'track' highways for
-  # motorized vehicles. This leads to a smaller and less dense graph, because there are fewer ways (obviously),
-  # but also because there are fewer crossings between highways (=junctions).
-  # Another typical example is excluding 'motorway', 'trunk' and maybe 'primary' highways for bicycle or pedestrian routing.
-  import.osm.ignored_highways: footway,cycleway,path,pedestrian,steps # typically useful for motorized-only routing
-  # import.osm.ignored_highways: motorway,trunk # typically useful for non-motorized routing
-
   # configure the memory access, use RAM_STORE for well equipped servers (default and recommended)
   graph.dataaccess.default_type: RAM_STORE
 

core/src/main/java/com/graphhopper/GraphHopper.java

@@ -568,18 +568,6 @@ public GraphHopper init(GraphHopperConfig ghConfig) {
         lmPreparationHandler.init(ghConfig);
 
         // osm import
-        // We do a few checks for import.osm.ignored_highways to prevent configuration errors when migrating from an older
-        // GH version.
-        if (!ghConfig.has("import.osm.ignored_highways"))
-            throw new IllegalArgumentException("Missing 'import.osm.ignored_highways'. Not using this parameter can decrease performance, see config-example.yml for more details");
-        String ignoredHighwaysString = ghConfig.getString("import.osm.ignored_highways", "");
-        if ((ignoredHighwaysString.contains("footway") || ignoredHighwaysString.contains("path")) && ghConfig.getProfiles().stream().map(Profile::getName).anyMatch(p -> p.contains("foot") || p.contains("hike") || p.contains("wheelchair")))
-            throw new IllegalArgumentException("You should not use import.osm.ignored_highways=footway or =path in conjunction with pedestrian profiles. This is probably an error in your configuration.");
-        if ((ignoredHighwaysString.contains("cycleway") || ignoredHighwaysString.contains("path")) && ghConfig.getProfiles().stream().map(Profile::getName).anyMatch(p -> p.contains("mtb") || p.contains("bike")))
-            throw new IllegalArgumentException("You should not use import.osm.ignored_highways=cycleway or =path in conjunction with bicycle profiles. This is probably an error in your configuration");
-
-        osmReaderConfig.setIgnoredHighways(Arrays.stream(ghConfig.getString("import.osm.ignored_highways", String.join(",", osmReaderConfig.getIgnoredHighways()))
-                .split(",")).map(String::trim).collect(Collectors.toList()));
         osmReaderConfig.setParseWayNames(ghConfig.getBool("datareader.instructions", osmReaderConfig.isParseWayNames()));
         osmReaderConfig.setPreferredLanguage(ghConfig.getString("datareader.preferred_language", osmReaderConfig.getPreferredLanguage()));
         osmReaderConfig.setMaxWayPointDistance(ghConfig.getDouble(Routing.INIT_WAY_POINT_MAX_DISTANCE, osmReaderConfig.getMaxWayPointDistance()));
@@ -620,9 +608,8 @@ protected EncodingManager buildEncodingManager(Map<String, String> vehiclesByNam
         return emBuilder.build();
     }
 
-    protected OSMParsers buildOSMParsers(Map<String, String> vehiclesByName, List<String> encodedValueStrings, List<String> ignoredHighways, String dateRangeParserString) {
+    protected OSMParsers buildOSMParsers(Map<String, String> vehiclesByName, List<String> encodedValueStrings, String dateRangeParserString) {
         OSMParsers osmParsers = new OSMParsers();
-        ignoredHighways.forEach(osmParsers::addIgnoredHighway);
         for (String s : encodedValueStrings) {
             TagParser tagParser = tagParserFactory.create(encodingManager, s);
             if (tagParser != null)
@@ -819,7 +806,7 @@ protected void process(boolean closeEarly) {
         Map<String, String> vehiclesByName = getVehiclesByName(vehiclesString, profilesByName.values());
         List<String> encodedValueStrings = getEncodedValueStrings(encodedValuesString);
         encodingManager = buildEncodingManager(vehiclesByName, encodedValueStrings, withUrbanDensity, profilesByName.values());
-        osmParsers = buildOSMParsers(vehiclesByName, encodedValueStrings, osmReaderConfig.getIgnoredHighways(), dateRangeParserString);
+        osmParsers = buildOSMParsers(vehiclesByName, encodedValueStrings, dateRangeParserString);
         baseGraph = new BaseGraph.Builder(getEncodingManager())
                 .setDir(directory)
                 .set3D(hasElevation())

core/src/main/java/com/graphhopper/reader/osm/WaySegmentParser.java

@@ -137,9 +137,9 @@ public Date getTimeStamp() {
     private class Pass1Handler implements ReaderElementHandler {
         private boolean handledWays;
         private boolean handledRelations;
-        private long wayCounter = 0;
+        private long wayCounter = 1;
         private long acceptedWays = 0;
-        private long relationsCounter = 0;
+        private long relationsCounter = -1;
 
         @Override
         public void handleWay(ReaderWay way) {
@@ -198,10 +198,10 @@ private class Pass2Handler implements ReaderElementHandler {
         private boolean handledNodes;
         private boolean handledWays;
         private boolean handledRelations;
-        private long nodeCounter = 0;
+        private long nodeCounter = -1;
         private long acceptedNodes = 0;
         private long ignoredSplitNodes = 0;
-        private long wayCounter = 0;
+        private long wayCounter = -1;
 
         @Override
         public void handleNode(ReaderNode node) {

core/src/main/java/com/graphhopper/routing/OSMReaderConfig.java

@@ -18,11 +18,7 @@
 
 package com.graphhopper.routing;
 
-import java.util.ArrayList;
-import java.util.List;
-
 public class OSMReaderConfig {
-    private List<String> ignoredHighways = new ArrayList<>();
     private boolean parseWayNames = true;
     private String preferredLanguage = "";
     private double maxWayPointDistance = 1;
@@ -32,24 +28,6 @@ public class OSMReaderConfig {
     private double longEdgeSamplingDistance = Double.MAX_VALUE;
     private int workerThreads = 2;
 
-    public List<String> getIgnoredHighways() {
-        return ignoredHighways;
-    }
-
-    /**
-     * Sets the values of the highway tag that shall be ignored when we read the OSM file. This can be used to speed up
-     * the import and reduce the size of the resulting routing graph. For example if one is only interested in routing
-     * for motorized vehicles the routing graph size can be reduced by excluding footways, cycleways, paths and/or
-     * tracks. This can be quite significant depending on your area. Not only are there fewer ways to be processed, but
-     * there are also fewer junctions, which means fewer nodes and edges. Another reason to exclude footways etc. for
-     * motorized vehicle routing could be preventing undesired u-turns (#1858). Similarly, one could exclude motorway,
-     * trunk or even primary highways for bicycle or pedestrian routing.
-     */
-    public OSMReaderConfig setIgnoredHighways(List<String> ignoredHighways) {
-        this.ignoredHighways = ignoredHighways;
-        return this;
-    }
-
     public String getPreferredLanguage() {
         return preferredLanguage;
     }

core/src/main/java/com/graphhopper/routing/util/OSMParsers.java

@@ -31,31 +31,24 @@
 import java.util.function.Function;
 
 public class OSMParsers {
-    private final List<String> ignoredHighways;
     private final List<TagParser> wayTagParsers;
     private final List<VehicleTagParser> vehicleTagParsers;
     private final List<RelationTagParser> relationTagParsers;
     private final List<RestrictionTagParser> restrictionTagParsers;
     private final EncodedValue.InitializerConfig relConfig = new EncodedValue.InitializerConfig();
 
     public OSMParsers() {
-        this(new ArrayList<>(), new ArrayList<>(), new ArrayList<>(), new ArrayList<>(), new ArrayList<>());
+        this(new ArrayList<>(), new ArrayList<>(), new ArrayList<>(), new ArrayList<>());
     }
 
-    public OSMParsers(List<String> ignoredHighways, List<TagParser> wayTagParsers, List<VehicleTagParser> vehicleTagParsers,
+    public OSMParsers(List<TagParser> wayTagParsers, List<VehicleTagParser> vehicleTagParsers,
                       List<RelationTagParser> relationTagParsers, List<RestrictionTagParser> restrictionTagParsers) {
-        this.ignoredHighways = ignoredHighways;
         this.wayTagParsers = wayTagParsers;
         this.vehicleTagParsers = vehicleTagParsers;
         this.relationTagParsers = relationTagParsers;
         this.restrictionTagParsers = restrictionTagParsers;
     }
 
-    public OSMParsers addIgnoredHighway(String highway) {
-        ignoredHighways.add(highway);
-        return this;
-    }
-
     public OSMParsers addWayTagParser(TagParser tagParser) {
         wayTagParsers.add(tagParser);
         return this;
@@ -80,20 +73,7 @@ public OSMParsers addRestrictionTagParser(RestrictionTagParser restrictionTagPar
     }
 
     public boolean acceptWay(ReaderWay way) {
-        String highway = way.getTag("highway");
-        if (highway != null)
-            return !ignoredHighways.contains(highway);
-        else if (way.getTag("route") != null)
-            // we accept all the ways with a 'route' tag and no 'highway' tag, because these are needed for e.g.
-            // route=ferry and there aren't many others:
-            // https://github.com/graphhopper/graphhopper/pull/2702/files#r1038093050
-            return true;
-        else if ("pier".equals(way.getTag("man_made")))
-            return true;
-        else if ("platform".equals(way.getTag("railway")))
-            return true;
-        else
-            return false;
+        return vehicleTagParsers.stream().anyMatch(v -> !v.getAccess(way).equals(WayAccess.CAN_SKIP));
     }
 
     public IntsRef handleRelationTags(ReaderRelation relation, IntsRef relFlags) {
@@ -120,10 +100,6 @@ public IntsRef createRelationFlags() {
         return new IntsRef(2);
     }
 
-    public List<String> getIgnoredHighways() {
-        return ignoredHighways;
-    }
-
     public List<VehicleTagParser> getVehicleTagParsers() {
         return vehicleTagParsers;
     }

core/src/test/java/com/graphhopper/GraphHopperTest.java

@@ -98,12 +98,12 @@ public void setup() {
 
     @ParameterizedTest
     @CsvSource({
-            DIJKSTRA + ",false,708",
-            ASTAR + ",false,363",
-            DIJKSTRA_BI + ",false,346",
-            ASTAR_BI + ",false,192",
-            ASTAR_BI + ",true,46",
-            DIJKSTRA_BI + ",true,51"
+            DIJKSTRA + ",false,511",
+            ASTAR + ",false,256",
+            DIJKSTRA_BI + ",false,228",
+            ASTAR_BI + ",false,142",
+            ASTAR_BI + ",true,41",
+            DIJKSTRA_BI + ",true,48"
     })
     public void testMonacoDifferentAlgorithms(String algo, boolean withCH, int expectedVisitedNodes) {
         final String vehicle = "car";
@@ -150,7 +150,7 @@ public void testMonacoWithInstructions() {
                 setAlgorithm(ASTAR).setProfile(profile));
 
         // identify the number of counts to compare with CH foot route
-        assertEquals(713, rsp.getHints().getLong("visited_nodes.sum", 0));
+        assertEquals(706, rsp.getHints().getLong("visited_nodes.sum", 0));
 
         ResponsePath res = rsp.getBest();
         assertEquals(3437.1, res.getDistance(), .1);
@@ -644,13 +644,9 @@ public void testNorthBayreuthBlockedEdges() {
 
         req.putHint(Routing.BLOCK_AREA, "49.984434,11.505212,49.985394,11.506333");
         rsp = hopper.route(req);
-        // we do not snap to Grüngraben (within the blocked area), but onto Lohweg and then we need to go to Hauptstraße
-        // and turn left onto Waldhüttenstraße. Note that if we exclude footway we get an entirely different path, because
-        // the start point snaps all the way to the East onto the end of Bergstraße (because Lohweg gets no longer split
-        // into several edges...)
-        assertEquals(11.506, rsp.getBest().getWaypoints().getLon(0), 0.001);
+        assertEquals(11.508, rsp.getBest().getWaypoints().getLon(0), 0.001);
         assertFalse(rsp.hasErrors(), rsp.getErrors().toString());
-        assertEquals(510, rsp.getBest().getDistance(), 10);
+        assertEquals(1185, rsp.getBest().getDistance(), 10);
 
         // first point is contained in block_area => error
         req = new GHRequest(49.979, 11.516, 49.986107, 11.507202).
@@ -1604,17 +1600,17 @@ public void testCrossQuery() {
         hopper.importOrLoad();
 
         // flex
-        testCrossQueryAssert(profile1, hopper, 528.3, 196, true);
-        testCrossQueryAssert(profile2, hopper, 635.8, 198, true);
-        testCrossQueryAssert(profile3, hopper, 815.2, 198, true);
+        testCrossQueryAssert(profile1, hopper, 528.3, 138, true);
+        testCrossQueryAssert(profile2, hopper, 635.8, 138, true);
+        testCrossQueryAssert(profile3, hopper, 815.2, 140, true);
 
         // LM (should be the same as flex, but with less visited nodes!)
-        testCrossQueryAssert(profile1, hopper, 528.3, 108, false);
-        testCrossQueryAssert(profile2, hopper, 635.8, 162, false);
-        // this is actually interesting: the number of visited nodes increases: cross-querying 'works',
-        // but can even perform *worse*, because the landmarks were not customized for the weighting in use.
-        // Creating a separate LM preparation for profile3 yields 108 (not shown)
-        testCrossQueryAssert(profile3, hopper, 815.2, 236, false);
+        testCrossQueryAssert(profile1, hopper, 528.3, 74, false);
+        testCrossQueryAssert(profile2, hopper, 635.8, 124, false);
+        // this is actually interesting: the number of visited nodes *increases* once again (while it strictly decreases
+        // with rising distance factor for flex): cross-querying 'works', but performs *worse*, because the landmarks
+        // were not customized for the weighting in use. Creating a separate LM preparation for profile3 yields 74
+        testCrossQueryAssert(profile3, hopper, 815.2, 162, false);
     }
 
     private void testCrossQueryAssert(String profile, GraphHopper hopper, double expectedWeight, int expectedVisitedNodes, boolean disableLM) {
@@ -2202,13 +2198,13 @@ public void testCHWithFiniteUTurnCosts() {
         GHRequest req = new GHRequest(p, q);
         req.setProfile("my_profile");
         // we force the start/target directions such that there are u-turns right after we start and right before
-        // we reach the target. at the start location we do a u-turn at the crossing with the *steps* ('ghost junction')
+        // we reach the target
         req.setCurbsides(Arrays.asList("right", "right"));
         GHResponse res = h.route(req);
         assertFalse(res.hasErrors(), "routing should not fail");
-        assertEquals(242.9, res.getBest().getRouteWeight(), 0.1);
-        assertEquals(1917, res.getBest().getDistance(), 1);
-        assertEquals(243000, res.getBest().getTime(), 1000);
+        assertEquals(266.8, res.getBest().getRouteWeight(), 0.1);
+        assertEquals(2116, res.getBest().getDistance(), 1);
+        assertEquals(266800, res.getBest().getTime(), 1000);
     }
 
     @Test
@@ -2675,7 +2671,7 @@ void testLoadingWithAnotherSpeedFactorWorks() {
                     .setProfiles(new Profile("car").setVehicle("car").setWeighting("fastest"))
                     .setGraphHopperLocation(GH_LOCATION);
             hopper.load();
-            assertEquals(2969, hopper.getBaseGraph().getNodes());
+            assertEquals(1942, hopper.getBaseGraph().getNodes());
         }
     }