Remove shortest weighting from DefaultWeightingFactory class (graphhopper#2865)

* heading_penalty parameter should be picked and properly merged for custom weighting too

* remove shortest weighting

* keep ShortestWeighting in jar

* keep default constructor

* fix imports

* fix imports

* comments from review

* changelog

---------

Co-authored-by: easbar <[email protected]>

Author: karussell

CHANGELOG.md

@@ -1,5 +1,6 @@
 ### 8.0 [not yet released]
 
+- remove shortest weighting for public usage, use a high distance_influence instead, see #2865
 - removed duration:seconds as intermediate tag
 - /info endpoint does no longer return the vehicle used per profile and won't return encoded value of vehicles like car_average_speed
 - Country rules no longer contain maxspeed handling, enable a much better alternative via `max_speed_calculator.enabled: true`. On the client side use `max_speed_estimated` to determine if max_speed is from OSM or an estimation. See #2810

README.md

@@ -232,7 +232,7 @@ Here is a list of the more detailed features:
  * OpenStreetMap integration: stores and considers road type, speed limit, the surface, barriers, access restrictions, ferries, [conditional access restrictions](https://github.com/graphhopper/graphhopper/pull/621), ...
  * GraphHopper is fast. And with the so called "Contraction Hierarchies" it can be even faster (enabled by default).
  * Memory efficient data structures, algorithms and [the low and high level API](./docs/core/low-level-api.md) is tuned towards ease of use and efficiency
- * Multiple weightings (fastest/shortest/custom/...) and pre-built routing profiles: car, bike, racing bike, mountain bike, foot, hike, motorcycle, wheelchair, ...
+ * Pre-built routing profiles: car, bike, racing bike, mountain bike, foot, hike, motorcycle, wheelchair, ...
  * [Customization of these profiles](./docs/core/profiles.md#custom-profiles) are possible and e.g. get truck routing or support for cargo bikes and [many other changes](https://www.graphhopper.com/blog/2020/05/31/examples-for-customizable-routing/)
  * Provides a powerful [web API](./docs/web/api-doc.md) that exposes the data from OpenStreetMap and allows customizing the vehicle profiles per request. With JavaScript and Java clients.
  * Does [map matching](./map-matching)

config-example.yml

@@ -15,14 +15,12 @@ graphhopper:
   # In general a profile consists of the following
   # - name (required): a unique string identifier for the profile
   # - vehicle (required): refers to the `graph.vehicles` used for this profile
-  # - weighting (required): the weighting used for this profile like custom,fastest,shortest or short_fastest
+  # - weighting (required): the weighting used for this profile like custom
   # - turn_costs (true/false, default: false): whether or not turn restrictions should be applied for this profile.
   #
   # Depending on the above fields there are other properties that can be used, e.g.
   # - distance_factor: 0.1 (can be used to fine tune the time/distance trade-off of short_fastest weighting)
   # - u_turn_costs: 60 (time-penalty for doing a u-turn in seconds (only possible when `turn_costs: true`)).
-  #   Note that since the u-turn costs are given in seconds the weighting you use should also calculate the weight
-  #   in seconds, so for example it does not work with shortest weighting.
   # - custom_model_files: when you specified "weighting: custom" you need to set one or more json files which are searched in
   #   custom_models.directory or the working directory that defines the custom_model. If you want an empty model you can
   #   set "custom_model_files: []

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

@@ -248,8 +248,8 @@ public GraphHopper setStoreOnFlush(boolean storeOnFlush) {
      * <pre>
      * {@code
      *   hopper.setProfiles(
-     *     new Profile("my_car").setVehicle("car").setWeighting("shortest"),
-     *     new Profile("your_bike").setVehicle("bike").setWeighting("fastest")
+     *     new CustomProfile("my_car").setVehicle("car"),
+     *     new CustomProfile("your_bike").setVehicle("bike")
      *   );
      *   hopper.getCHPreparationHandler().setCHProfiles(
      *     new CHProfile("my_car"),

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

@@ -95,7 +95,7 @@ public Weighting createWeighting(Profile profile, PMap requestHints, boolean dis
             weighting = CustomModelParser.createWeighting(accessEnc, speedEnc,
                     priorityEnc, encodingManager, turnCostProvider, mergedCustomModel);
         } else if ("shortest".equalsIgnoreCase(weightingStr)) {
-            weighting = new ShortestWeighting(accessEnc, speedEnc, turnCostProvider);
+            throw new IllegalArgumentException("Instead of weighting=shortest use weighting=custom with a high distance_influence");
         } else if ("fastest".equalsIgnoreCase(weightingStr)) {
             if (!encodingManager.hasEncodedValue(RoadAccess.KEY))
                 throw new IllegalArgumentException("The fastest weighting requires road_access");

core/src/main/java/com/graphhopper/routing/weighting/AbstractWeighting.java

@@ -91,7 +91,7 @@ static boolean isValidName(String name) {
 
     @Override
     public String toString() {
-        return getName() + "|" + speedEnc.getName().split("$")[0];
+        return getName() + "|" + speedEnc.getName();
     }
 
 }

core/src/main/java/com/graphhopper/routing/weighting/ShortestWeighting.java

@@ -21,36 +21,61 @@
 import com.graphhopper.routing.ev.DecimalEncodedValue;
 import com.graphhopper.util.EdgeIteratorState;
 
-import static com.graphhopper.routing.weighting.TurnCostProvider.NO_TURN_COST_PROVIDER;
-
 /**
  * Calculates the shortest route - independent of a vehicle as the calculation is based on the
  * distance only.
- * <p>
  *
  * @author Peter Karich
  */
-public class ShortestWeighting extends AbstractWeighting {
+public class ShortestWeighting implements Weighting {
+
+    final BooleanEncodedValue accessEnc;
+    final DecimalEncodedValue speedEnc;
+    final TurnCostProvider tcProvider;
+
     public ShortestWeighting(BooleanEncodedValue accessEnc, DecimalEncodedValue speedEnc) {
-        this(accessEnc, speedEnc, NO_TURN_COST_PROVIDER);
+        this(accessEnc, speedEnc, TurnCostProvider.NO_TURN_COST_PROVIDER);
     }
 
-    public ShortestWeighting(BooleanEncodedValue accessEnc, DecimalEncodedValue speedEnc, TurnCostProvider turnCostProvider) {
-        super(accessEnc, speedEnc, turnCostProvider);
+    public ShortestWeighting(BooleanEncodedValue accessEnc, DecimalEncodedValue speedEnc, TurnCostProvider tcProvider) {
+        this.accessEnc = accessEnc;
+        this.speedEnc = speedEnc;
+        this.tcProvider = tcProvider;
     }
 
     @Override
-    public double getMinWeight(double currDistToGoal) {
-        return currDistToGoal;
+    public double getMinWeight(double distance) {
+        return distance;
     }
 
     @Override
     public double calcEdgeWeight(EdgeIteratorState edgeState, boolean reverse) {
-        if (edgeHasNoAccess(edgeState, reverse))
+        if (reverse ? !edgeState.getReverse(accessEnc) : !edgeState.get(accessEnc))
             return Double.POSITIVE_INFINITY;
         return edgeState.getDistance();
     }
 
+    @Override
+    public long calcEdgeMillis(EdgeIteratorState edgeState, boolean reverse) {
+        double speed = reverse ? edgeState.getReverse(speedEnc) : edgeState.get(speedEnc);
+        return Math.round(edgeState.getDistance() / speed * 3.6 * 1000);
+    }
+
+    @Override
+    public double calcTurnWeight(int inEdge, int viaNode, int outEdge) {
+        return tcProvider.calcTurnWeight(inEdge, viaNode, outEdge);
+    }
+
+    @Override
+    public long calcTurnMillis(int inEdge, int viaNode, int outEdge) {
+        return tcProvider.calcTurnMillis(inEdge, viaNode, outEdge);
+    }
+
+    @Override
+    public boolean hasTurnCosts() {
+        return tcProvider != TurnCostProvider.NO_TURN_COST_PROVIDER;
+    }
+
     @Override
     public String getName() {
         return "shortest";

core/src/main/java/com/graphhopper/routing/weighting/Weighting.java

@@ -20,8 +20,7 @@
 import com.graphhopper.util.EdgeIteratorState;
 
 /**
- * Specifies how the best route is calculated. E.g. the fastest or shortest route.
- * <p>
+ * Specifies how the best route is calculated.
  *
  * @author Peter Karich
  */