Merge remote-tracking branch 'upstream/dev-2.x' into upstream-merge-2…

…024-02-05

docs/Changelog.md

@@ -88,6 +88,7 @@ based on merged pull requests. Search GitHub issues and pull requests for smalle
 - Separate walk time from non-transit time [#5648](https://github.com/opentripplanner/OpenTripPlanner/pull/5648)
 - Remove "fare" [#5645](https://github.com/opentripplanner/OpenTripPlanner/pull/5645)
 - Refactor GroupStopBuilder addLocation method [#5651](https://github.com/opentripplanner/OpenTripPlanner/pull/5651)
+- Remove `VehicleToStopHeuristics` [#5381](https://github.com/opentripplanner/OpenTripPlanner/pull/5381)
 [](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE)
 
 ## 2.4.0 (2023-09-13)

docs/Configuration.md

@@ -250,7 +250,6 @@ Here is a list of all features which can be toggled on/off and their default val
 | `SandboxAPIParkAndRideApi`                 | Enable park-and-ride endpoint.                                                                                                                                                                                                                                                                                   |                    |    ✓️   |
 | `SandboxAPITravelTime`                     | Enable the isochrone/travel time surface API.                                                                                                                                                                                                                                                                    |                    |    ✓️   |
 | `TransferAnalyzer`                         | Analyze transfers during graph build.                                                                                                                                                                                                                                                                            |                    |    ✓️   |
-| `VehicleToStopHeuristics`                  | Enable improved heuristic for park-and-ride queries.                                                                                                                                                                                                                                                             |                    |    ✓️   |
 
 <!-- OTP-FEATURE-TABLE END -->
 

docs/RouteRequest.md

@@ -78,8 +78,8 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe
 |       [safety](#rd_bicycle_triangle_safety)                                                                  |        `double`        | Relative importance of safety (range 0-1).                                                                                                     | *Optional* | `0.0`            |  2.0  |
 |       time                                                                                                   |        `double`        | Relative importance of duration of travel (range 0-1).                                                                                         | *Optional* | `0.0`            |  2.0  |
 |    walk                                                                                                      |        `object`        | Preferences for walking a vehicle.                                                                                                             | *Optional* |                  |  2.5  |
-|       [hopCost](#rd_bicycle_walk_hopCost)                                                                    |        `integer`       | The cost of hopping on or off a vehicle.                                                                                                       | *Optional* | `0`              |  2.0  |
-|       [hopTime](#rd_bicycle_walk_hopTime)                                                                    |       `duration`       | The time it takes the user to hop on or off a vehicle.                                                                                         | *Optional* | `"PT0S"`         |  2.0  |
+|       [mountDismountCost](#rd_bicycle_walk_mountDismountCost)                                                |        `integer`       | The cost of hopping on or off a vehicle.                                                                                                       | *Optional* | `0`              |  2.0  |
+|       [mountDismountTime](#rd_bicycle_walk_mountDismountTime)                                                |       `duration`       | The time it takes the user to hop on or off a vehicle.                                                                                         | *Optional* | `"PT0S"`         |  2.0  |
 |       reluctance                                                                                             |        `double`        | A multiplier for how bad walking with a vehicle is, compared to being in transit for equal lengths of time.                                    | *Optional* | `5.0`            |  2.1  |
 |       speed                                                                                                  |        `double`        | The user's vehicle walking speed in meters/second. Defaults to approximately 3 MPH.                                                            | *Optional* | `1.33`           |  2.1  |
 |       stairsReluctance                                                                                       |        `double`        | How bad is it to walk the vehicle up/down a flight of stairs compared to taking a detour.                                                      | *Optional* | `10.0`           |  2.3  |
@@ -406,7 +406,12 @@ since the search-window is increased with the same amount as the maximum penalty
 the access legs used. In other cases where the access(CAR) is faster than transit the
 performance will be better.
 
-The default is no penalty, if not configured.
+The default values are
+
+- `car-to-park` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
+- `car-rental` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
+- `car-hailing` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
+- `flexible` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
 
 Example: `"car-to-park" : { "timePenalty": "10m + 1.5t", "costFactor": 2.5 }`
 
@@ -525,7 +530,7 @@ This factor can also include other concerns such as convenience and general cycl
 preferences by taking into account road surface etc.
 
 
-<h3 id="rd_bicycle_walk_hopCost">hopCost</h3>
+<h3 id="rd_bicycle_walk_mountDismountCost">mountDismountCost</h3>
 
 **Since version:** `2.0` ∙ **Type:** `integer` ∙ **Cardinality:** `Optional` ∙ **Default value:** `0`   
 **Path:** /routingDefaults/bicycle/walk 
@@ -536,7 +541,7 @@ There are different parameters for the cost of renting or parking a vehicle and
 not meant for controlling the cost of those events.
 
 
-<h3 id="rd_bicycle_walk_hopTime">hopTime</h3>
+<h3 id="rd_bicycle_walk_mountDismountTime">mountDismountTime</h3>
 
 **Since version:** `2.0` ∙ **Type:** `duration` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"PT0S"`   
 **Path:** /routingDefaults/bicycle/walk 

docs/SandboxExtension.md

@@ -23,7 +23,6 @@ nearby stops generated by routing via OSM data.
 - [Park and Ride API](sandbox/ParkAndRideApi.md) - Park and Ride API
 - [Data Overlay](sandbox/DataOverlay.md) - StreetEdge grid data populating affecting the route planning
 - [Vehicle Parking](sandbox/VehicleParking.md) - Vehicle Parking updaters
-- [Vehicle-to-stop heuristics](sandbox/VehicleToStopHeuristics.md) - Speeding up Park+Ride, Bike+Ride and Bike+Transit searches
 - [Travel Time (Isochrone & Surface) API](sandbox/TravelTime.md) - Travel Time API
 - [IBI accessibility score](sandbox/IBIAccessibilityScore.md) - IBI accessibility score
 - [Fares](sandbox/Fares.md) - Fare calculation

docs/examples/entur/otp-config.json

@@ -6,7 +6,6 @@
     "OptimizeTransfers": true,
     "TransferConstraints": true,
     "ParallelRouting": false,
-    "ReportApi" : true,
-    "VehicleToStopHeuristics": true
+    "ReportApi" : true
   }
 }

mkdocs.yml

@@ -106,7 +106,6 @@ nav:
     - Park and Ride API: 'sandbox/ParkAndRideApi.md'
     - Data Overlay: 'sandbox/DataOverlay.md'
     - Vehicle Parking Updaters: 'sandbox/VehicleParking.md'
-    - Vehicle-to-stop Heuristics: 'sandbox/VehicleToStopHeuristics.md'
     - Geocoder API: 'sandbox/GeocoderAPI.md'
     - Travel Time Isochrones: 'sandbox/TravelTime.md'
     - IBI Accessibility Score: 'sandbox/IBIAccessibilityScore.md'

pom.xml

@@ -56,18 +56,18 @@
 
 
     <properties>
-        <otp.serialization.version.id>143</otp.serialization.version.id>
+        <otp.serialization.version.id>144</otp.serialization.version.id>
         <!-- Lib versions - keep list sorted on property name -->
         <geotools.version>30.2</geotools.version>
         <google.dagger.version>2.50</google.dagger.version>
         <jackson.version>2.16.1</jackson.version>
         <jersey.version>3.1.5</jersey.version>
-        <junit.version>5.10.1</junit.version>
-        <micrometer.version>1.12.1</micrometer.version>
+        <junit.version>5.10.2</junit.version>
+        <micrometer.version>1.12.2</micrometer.version>
         <netcdf4.version>5.5.3</netcdf4.version>
         <logback.version>1.4.14</logback.version>
         <lucene.version>9.9.1</lucene.version>
-        <slf4j.version>2.0.11</slf4j.version>
+        <slf4j.version>2.0.12</slf4j.version>
         <netex-java-model.version>2.0.15</netex-java-model.version>
         <siri-java-model.version>1.26</siri-java-model.version>
         <jaxb-runtime.version>4.0.4</jaxb-runtime.version>
@@ -552,7 +552,7 @@
                 <!-- This make sure all google libraries are using compatible versions. -->
                 <groupId>com.google.cloud</groupId>
                 <artifactId>libraries-bom</artifactId>
-                <version>26.27.0</version>
+                <version>26.31.0</version>
                 <type>pom</type>
                 <scope>import</scope>
             </dependency>
@@ -720,7 +720,7 @@
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-core</artifactId>
-            <version>5.9.0</version>
+            <version>5.10.0</version>
             <scope>test</scope>
         </dependency>
         <dependency>

src/ext-test/java/org/opentripplanner/ext/flex/FlexIntegrationTest.java

@@ -32,6 +32,7 @@
 import org.opentripplanner.model.plan.Itinerary;
 import org.opentripplanner.routing.api.RoutingService;
 import org.opentripplanner.routing.api.request.RouteRequest;
+import org.opentripplanner.routing.api.request.framework.TimeAndCostPenalty;
 import org.opentripplanner.routing.api.request.request.filter.AllowAllTransitFilter;
 import org.opentripplanner.routing.graph.Graph;
 import org.opentripplanner.transit.service.TransitModel;
@@ -224,6 +225,11 @@ private static List<Itinerary> getItineraries(
     request.setTo(to);
     request.setNumItineraries(10);
     request.setSearchWindow(Duration.ofHours(2));
+    request.withPreferences(p ->
+      p.withStreet(s ->
+        s.withAccessEgress(ae -> ae.withPenalty(Map.of(FLEXIBLE, TimeAndCostPenalty.ZERO)))
+      )
+    );
 
     var modes = request.journey().modes().copyOf();
     modes.withEgressMode(FLEXIBLE);

src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java

@@ -37,8 +37,8 @@ void changeNames() {
 
     filter.decorate(itinerary);
 
-    var updatedLeg = itinerary.getLegs().get(0);
-    assertEquals(STOP_D.getName(), updatedLeg.getFrom().name);
+    var updatedLeg = itinerary.getLegs().getFirst();
+    assertEquals(STOP_C.getName(), updatedLeg.getFrom().name);
     assertEquals(STOP_D.getName(), updatedLeg.getTo().name);
   }
 }

src/ext/java/org/opentripplanner/ext/parkAndRideApi/ParkAndRideResource.java

@@ -42,7 +42,7 @@ public ParkAndRideResource(
     //           - serverContext.graphFinder(). This needs at least a comment!
     //           - This can be replaced with a search done with the StopModel
     //           - if we have a radius search there.
-    this.graphFinder = new DirectGraphFinder(serverContext.transitService()::findRegularStop);
+    this.graphFinder = new DirectGraphFinder(serverContext.transitService()::findRegularStops);
   }
 
   /** Envelopes are in latitude, longitude format */

src/ext/java/org/opentripplanner/ext/restapi/resources/IndexAPI.java

@@ -200,7 +200,7 @@ public List<ApiStopShort> getStopsInRadius(
 
       radius = Math.min(radius, MAX_STOP_SEARCH_RADIUS);
 
-      return new DirectGraphFinder(serverContext.transitService()::findRegularStop)
+      return new DirectGraphFinder(serverContext.transitService()::findRegularStops)
         .findClosestStops(new Coordinate(lon, lat), radius)
         .stream()
         .map(it -> StopMapper.mapToApiShort(it.stop, it.distance))
@@ -221,7 +221,7 @@ public List<ApiStopShort> getStopsInRadius(
         new Coordinate(maxLon, maxLat)
       );
 
-      var stops = transitService().findRegularStop(envelope);
+      var stops = transitService().findRegularStops(envelope);
       return stops
         .stream()
         .filter(stop -> envelope.contains(stop.getCoordinate().asJtsCoordinate()))

src/ext/java/org/opentripplanner/ext/restapi/resources/RequestToPreferencesMapper.java

@@ -89,8 +89,8 @@ private void mapBike() {
       bike.withWalking(walk -> {
         setIfNotNull(req.bikeWalkingSpeed, walk::withSpeed);
         setIfNotNull(req.bikeWalkingReluctance, walk::withReluctance);
-        setIfNotNull(req.bikeSwitchTime, walk::withHopTime);
-        setIfNotNull(req.bikeSwitchCost, walk::withHopCost);
+        setIfNotNull(req.bikeSwitchTime, walk::withMountDismountTime);
+        setIfNotNull(req.bikeSwitchCost, walk::withMountDismountCost);
       });
     });
   }

src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java

@@ -7,9 +7,9 @@
 import org.opentripplanner.routing.algorithm.filterchain.framework.spi.ItineraryDecorator;
 
 /**
- * A decorating filter that checks if a transit leg contains any primary stops and if it does,
- * then replaces it with the secondary, agency-specific stop name. This is so that the in-vehicle
- * display matches what OTP returns as a board/alight stop name.
+ * A decorating filter that checks if a transit leg contains any consolidated stops and if it does,
+ * then replaces it with the appropriate, agency-specific stop name. This is so that the physical
+ * signage and in-vehicle display matches what OTP returns as a board/alight stop name.
  */
 public class DecorateConsolidatedStopNames implements ItineraryDecorator {
 
@@ -21,31 +21,45 @@ public DecorateConsolidatedStopNames(StopConsolidationService service) {
 
   @Override
   public void decorate(Itinerary itinerary) {
-    replacePrimaryNamesWithSecondary(itinerary);
+    replaceConsolidatedStops(itinerary);
   }
 
   /**
-   * If the itinerary has a from/to that is the primary stop of a {@link org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup}
-   * then we replace its name with the secondary name of the agency that is
-   * operating the route, so that the name in the result matches the name in the in-vehicle
-   * display.
+   * If the itinerary has a "from" stop that is the secondary stop of a
+   * {@link org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup}
+   * then we replace its name with the primary name of the agency that is
+   * operating the route, so that the name in the result matches the physical signage on the stop.
+   * <p>
+   * If the leg has a "to" stop that is a primary stop, then we don't want to show the stop that's on
+   * the signage but what is shown _inside_ the vehicle. That's why we use the agency-specific (aka
+   * secondary) stop.
+   * <p>
+   * This follows the somewhat idiosyncratic logic of the consolidated stops feature.
    */
-  private void replacePrimaryNamesWithSecondary(Itinerary i) {
+  private void replaceConsolidatedStops(Itinerary i) {
     i.transformTransitLegs(leg -> {
       if (leg instanceof ScheduledTransitLeg stl && needsToRenameStops(stl)) {
         var agency = leg.getAgency();
-        return new ConsolidatedStopLeg(
-          stl,
-          service.agencySpecificName(stl.getFrom().stop, agency),
-          service.agencySpecificName(stl.getTo().stop, agency)
-        );
+        // to show the name on the stop signage we use the primary stop's name
+        var from = service.primaryStop(stl.getFrom().stop.getId());
+        // to show the name that's on the display inside the vehicle we use the agency-specific name
+        var to = service.agencySpecificStop(stl.getTo().stop, agency);
+        return new ConsolidatedStopLeg(stl, from, to);
       } else {
         return leg;
       }
     });
   }
 
+  /**
+   * Figures out if the from/to stops are part of a consolidated stop group and therefore
+   * some stops need to be replaced.
+   * <p>
+   * Please consult the Javadoc of {@link DecorateConsolidatedStopNames#replaceConsolidatedStops(Itinerary)}
+   * for details of this idiosyncratic business logic and in particular why the logic is not the same
+   * for the from/to stops.
+   */
   private boolean needsToRenameStops(ScheduledTransitLeg stl) {
-    return (service.isPrimaryStop(stl.getFrom().stop) || service.isPrimaryStop(stl.getTo().stop));
+    return (service.isSecondaryStop(stl.getFrom().stop) || service.isPrimaryStop(stl.getTo().stop));
   }
 }