Add option to configure linear cost function for removeTransitWithHig…

…herCostThanBestOnWalkOnly filter
opentripplanner · Nov 9, 2023 · 2b510d4 · 2b510d4
1 parent 0f57947
commit 2b510d4
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 4 deletions.
diff --git a/...va/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java b/...va/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java
@@ -11,6 +11,7 @@
 import org.opentripplanner.ext.accessibilityscore.AccessibilityScoreFilter;
 import org.opentripplanner.ext.fares.FaresFilter;
 import org.opentripplanner.framework.lang.Sandbox;
+import org.opentripplanner.framework.model.Cost;
 import org.opentripplanner.model.plan.Itinerary;
 import org.opentripplanner.model.plan.SortOrder;
 import org.opentripplanner.routing.algorithm.filterchain.api.TransitGeneralizedCostFilterParams;
@@ -60,6 +61,7 @@ public class ItineraryListFilterChainBuilder {
   private int maxNumberOfItineraries = NOT_SET;
   private ListSection maxNumberOfItinerariesCrop = ListSection.TAIL;
   private CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly;
+  private CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly;
   private boolean removeWalkAllTheWayResults;
   private boolean sameFirstOrLastTripFilter;
   private TransitGeneralizedCostFilterParams transitGeneralizedCostFilterParams;
@@ -206,6 +208,27 @@ public ItineraryListFilterChainBuilder withRemoveTransitWithHigherCostThanBestOn
     return this;
   }
 
+  /**
+   * The direct street search(walk) is not pruning the transit search, so in some
+   * cases we get "silly" transit itineraries that is marginally better on travel-duration compared
+   * with an on-street-all-the-way itinerary. Use this method to filter worse enough itineraries.
+   * <p>
+   * The filter removes all itineraries with a generalized-cost that is higher than the best
+   * on-street-all-the-way itinerary.
+   * <p>
+   * This filter only have an effect, if an on-street-all-the-way(WALK) itinerary
+   * exist.
+   *
+   * This have same effect as {@link ItineraryListFilterChainBuilder#withRemoveTransitWithHigherCostThanBestOnStreetOnly(CostLinearFunction)}
+   * but only for WALK itineraries
+   */
+  public ItineraryListFilterChainBuilder withRemoveTransitWithHigherCostThanBestOnWalkOnly(
+    CostLinearFunction value
+  ) {
+    this.removeTransitWithHigherCostThanBestOnStreetOnly = value;
+    return this;
+  }
+
   /**
    * A transit itinerary with higher generalized-cost than a walk-only itinerary is silly. This filter removes such
    * itineraries.
@@ -388,7 +411,15 @@ public ItineraryListFilterChain build() {
       }
 
       if (removeTransitIfWalkingIsBetter) {
-        filters.add(new DeletionFlaggingFilter(new RemoveTransitIfWalkingIsBetterFilter()));
+        filters.add(
+          new DeletionFlaggingFilter(
+            new RemoveTransitIfWalkingIsBetterFilter(
+              removeTransitWithHigherCostThanBestOnWalkOnly != null
+                ? removeTransitWithHigherCostThanBestOnWalkOnly
+                : CostLinearFunction.of(Cost.ZERO, 1.0)
+            )
+          )
+        );
       }
 
       if (removeWalkAllTheWayResults) {

diff --git a/...r/routing/algorithm/filterchain/deletionflagger/RemoveTransitIfWalkingIsBetterFilter.java b/...r/routing/algorithm/filterchain/deletionflagger/RemoveTransitIfWalkingIsBetterFilter.java
@@ -15,6 +15,12 @@
  */
 public class RemoveTransitIfWalkingIsBetterFilter implements ItineraryDeletionFlagger {
 
+  private final CostLinearFunction costLimitFunction;
+
+  public RemoveTransitIfWalkingIsBetterFilter(CostLinearFunction costLimitFunction) {
+    this.costLimitFunction = costLimitFunction;
+  }
+
   /**
    * Required for {@link org.opentripplanner.routing.algorithm.filterchain.ItineraryListFilterChain},
    * to know which filters removed
@@ -38,7 +44,7 @@ public List<Itinerary> flagForRemoval(List<Itinerary> itineraries) {
       return List.of();
     }
 
-    var limit = minWalkCost.getAsInt();
+    var limit = costLimitFunction.calculate(Cost.costOfSeconds(minWalkCost.getAsInt())).toSeconds();
 
     return itineraries
       .stream()

diff --git a/...n/java/org/opentripplanner/routing/algorithm/mapping/RouteRequestToFilterChainMapper.java b/...n/java/org/opentripplanner/routing/algorithm/mapping/RouteRequestToFilterChainMapper.java
@@ -70,6 +70,9 @@ public static ItineraryListFilterChain createFilterChain(
       .withRemoveTransitWithHigherCostThanBestOnStreetOnly(
         params.removeTransitWithHigherCostThanBestOnStreetOnly()
       )
+      .withRemoveTransitWithHigherCostThanBestOnWalkOnly(
+        params.removeTransitWithHigherCostThanBestOnWalkOnly()
+      )
       .withSameFirstOrLastTripFilter(params.filterItinerariesWithSameFirstOrLastTrip())
       .withAccessibilityScore(
         params.useAccessibilityScore() && request.wheelchair(),

diff --git a/...n/java/org/opentripplanner/routing/api/request/preference/ItineraryFilterPreferences.java b/...n/java/org/opentripplanner/routing/api/request/preference/ItineraryFilterPreferences.java
@@ -31,6 +31,7 @@ public final class ItineraryFilterPreferences {
   private final boolean removeItinerariesWithSameRoutesAndStops;
   private final TransitGeneralizedCostFilterParams transitGeneralizedCostLimit;
   private final CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly;
+  private final CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly;
   private final boolean removeTransitIfWalkingIsBetter;
 
   private ItineraryFilterPreferences() {
@@ -52,6 +53,8 @@ private ItineraryFilterPreferences() {
       );
     this.removeTransitWithHigherCostThanBestOnStreetOnly =
       CostLinearFunction.of(Duration.ofMinutes(1), 1.3);
+    this.removeTransitWithHigherCostThanBestOnWalkOnly =
+      CostLinearFunction.of(Duration.ofMinutes(0), 1.0);
     this.removeTransitIfWalkingIsBetter = false;
   }
 
@@ -73,6 +76,8 @@ private ItineraryFilterPreferences(Builder builder) {
     this.transitGeneralizedCostLimit = Objects.requireNonNull(builder.transitGeneralizedCostLimit);
     this.removeTransitWithHigherCostThanBestOnStreetOnly =
       Objects.requireNonNull(builder.removeTransitWithHigherCostThanBestOnStreetOnly);
+    this.removeTransitWithHigherCostThanBestOnWalkOnly =
+      Objects.requireNonNull(builder.removeTransitWithHigherCostThanBestOnWalkOnly);
     this.removeTransitIfWalkingIsBetter = builder.removeTransitIfWalkingIsBetter;
   }
 
@@ -136,6 +141,10 @@ public CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly() {
     return removeTransitWithHigherCostThanBestOnStreetOnly;
   }
 
+  public CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly() {
+    return removeTransitWithHigherCostThanBestOnWalkOnly;
+  }
+
   public boolean removeTransitIfWalkingIsBetter() {
     return removeTransitIfWalkingIsBetter;
   }
@@ -183,6 +192,11 @@ public String toString() {
         removeTransitWithHigherCostThanBestOnStreetOnly,
         DEFAULT.removeTransitWithHigherCostThanBestOnStreetOnly
       )
+      .addObj(
+        "removeTransitWithHigherCostThanBestOnWalkOnly",
+        removeTransitWithHigherCostThanBestOnWalkOnly,
+        DEFAULT.removeTransitWithHigherCostThanBestOnWalkOnly
+      )
       .addBoolIfTrue(
         "removeItinerariesWithSameRoutesAndStops",
         removeItinerariesWithSameRoutesAndStops
@@ -217,6 +231,10 @@ public boolean equals(Object o) {
         removeTransitWithHigherCostThanBestOnStreetOnly,
         that.removeTransitWithHigherCostThanBestOnStreetOnly
       ) &&
+      Objects.equals(
+        removeTransitWithHigherCostThanBestOnWalkOnly,
+        that.removeTransitWithHigherCostThanBestOnWalkOnly
+      ) &&
       Objects.equals(transitGeneralizedCostLimit, that.transitGeneralizedCostLimit)
     );
   }
@@ -237,6 +255,7 @@ public int hashCode() {
       removeItinerariesWithSameRoutesAndStops,
       transitGeneralizedCostLimit,
       removeTransitWithHigherCostThanBestOnStreetOnly,
+      removeTransitWithHigherCostThanBestOnWalkOnly,
       removeTransitIfWalkingIsBetter
     );
   }
@@ -257,6 +276,7 @@ public static class Builder {
     private boolean removeItinerariesWithSameRoutesAndStops;
     private TransitGeneralizedCostFilterParams transitGeneralizedCostLimit;
     private CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly;
+    private CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly;
     private boolean removeTransitIfWalkingIsBetter;
 
     public ItineraryFilterPreferences original() {
@@ -341,6 +361,14 @@ public Builder withRemoveTransitWithHigherCostThanBestOnStreetOnly(
       return this;
     }
 
+    public Builder withRemoveTransitWithHigherCostThanBestOnWalkOnly(
+      CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly
+    ) {
+      this.removeTransitWithHigherCostThanBestOnWalkOnly =
+        removeTransitWithHigherCostThanBestOnWalkOnly;
+      return this;
+    }
+
     public Builder withRemoveTransitIfWalkingIsBetter(boolean removeTransitIfWalkingIsBetter) {
       this.removeTransitIfWalkingIsBetter = removeTransitIfWalkingIsBetter;
       return this;

diff --git a/src/main/java/org/opentripplanner/standalone/config/routerequest/ItineraryFiltersConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerequest/ItineraryFiltersConfig.java
@@ -174,6 +174,28 @@ public static void mapItineraryFilterParams(
           )
           .asCostLinearFunction(dft.removeTransitWithHigherCostThanBestOnStreetOnly())
       )
+      .withRemoveTransitWithHigherCostThanBestOnStreetOnly(
+        c
+          .of("removeTransitWithHigherCostThanBestOnWalkOnly")
+          .since(V2_4)
+          .summary(
+            "Limit function for generalized-cost computed from street-only itineraries applied to transit itineraries."
+          )
+          .description(
+            """
+The max-limit is applied to itineraries with transit *legs*, and only itineraries
+without transit legs are considered when calculating the minimum cost. The smallest
+generalized-cost value is used as input to the function. The function is used to calculate a
+*max-limit*. The max-limit is then used to filter *transit* itineraries by
+*generalized-cost*. Itineraries with a cost higher than the max-limit are dropped from the result
+set. Walking is handled with a different logic: if a transit itinerary has higher cost than
+a plain walk itinerary, it will be removed even if the cost limit function would keep it.
+This function works in the same way as removeTransitWithHigherCostThanBestOnStreetOnly but only for WALK legs.
+Default value is 0 + 1.0x.
+"""
+          )
+          .asCostLinearFunction(dft.removeTransitWithHigherCostThanBestOnWalkOnly())
+      )
       .withBikeRentalDistanceRatio(
         c
           .of("bikeRentalDistanceRatio")

diff --git a/...ner/routing/algorithm/filterchain/deletionflagger/RemoveTransitIfWalkingIsBetterTest.java b/...ner/routing/algorithm/filterchain/deletionflagger/RemoveTransitIfWalkingIsBetterTest.java
@@ -7,8 +7,10 @@
 import java.time.Duration;
 import java.util.List;
 import org.junit.jupiter.api.Test;
+import org.opentripplanner.framework.model.Cost;
 import org.opentripplanner.model.plan.Itinerary;
 import org.opentripplanner.model.plan.PlanTestConstants;
+import org.opentripplanner.routing.api.request.framework.CostLinearFunction;
 
 public class RemoveTransitIfWalkingIsBetterTest implements PlanTestConstants {
 
@@ -21,7 +23,7 @@ public void filterAwayNothingIfNoWalking() {
     // When:
     List<Itinerary> result = DeletionFlaggerTestHelper.process(
       List.of(i1, i2),
-      new RemoveTransitIfWalkingIsBetterFilter()
+      new RemoveTransitIfWalkingIsBetterFilter(CostLinearFunction.of(Cost.ZERO, 1.0))
     );
 
     // Then:
@@ -44,9 +46,10 @@ public void filterAwayTransitWithLongerWalk() {
 
     List<Itinerary> result = DeletionFlaggerTestHelper.process(
       List.of(i1, i2, bicycle, walk),
-      new RemoveTransitIfWalkingIsBetterFilter()
+      new RemoveTransitIfWalkingIsBetterFilter(CostLinearFunction.of(Cost.ZERO, 1.0))
     );
 
     assertEquals(toStr(List.of(i2, bicycle, walk)), toStr(result));
   }
+  // TODO: 2023-11-09 Write tests for cases other than 0 + 1.0x
 }