Keep at least one result for min-transfers and each transit-group in itinerary-group-filter

src/main/java/org/opentripplanner/framework/application/OTPFeature.java

@@ -90,6 +90,14 @@ public enum OTPFeature {
   FlexRouting(false, true, "Enable FLEX routing."),
   GoogleCloudStorage(false, true, "Enable Google Cloud Storage integration."),
   LegacyRestApi(true, true, "Enable legacy REST API. This API will be removed in the future."),
+  MultiCriteriaGroupMaxFilter(
+    false,
+    false,
+    "Keep the best itinerary with respect to each criteria used in the transit-routing search. " +
+    "For example the itinerary with the lowest cost, fewest transfers, and each unique transit-group " +
+    "(transit-group-priority) is kept, even if the max-limit is exceeded. This is turned off by default " +
+    "for now, until this feature is well tested."
+  ),
   RealtimeResolver(
     false,
     true,

src/main/java/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java

@@ -12,6 +12,7 @@
 import java.util.function.Function;
 import javax.annotation.Nullable;
 import org.opentripplanner.ext.accessibilityscore.DecorateWithAccessibilityScore;
+import org.opentripplanner.framework.application.OTPFeature;
 import org.opentripplanner.framework.collection.ListSection;
 import org.opentripplanner.framework.lang.Sandbox;
 import org.opentripplanner.model.plan.Itinerary;
@@ -27,6 +28,8 @@
 import org.opentripplanner.routing.algorithm.filterchain.filters.system.NumItinerariesFilter;
 import org.opentripplanner.routing.algorithm.filterchain.filters.system.OutsideSearchWindowFilter;
 import org.opentripplanner.routing.algorithm.filterchain.filters.system.PagingFilter;
+import org.opentripplanner.routing.algorithm.filterchain.filters.system.SingeCriteriaComparator;
+import org.opentripplanner.routing.algorithm.filterchain.filters.system.mcmax.McMaxLimitFilter;
 import org.opentripplanner.routing.algorithm.filterchain.filters.transit.DecorateTransitAlert;
 import org.opentripplanner.routing.algorithm.filterchain.filters.transit.KeepItinerariesWithFewestTransfers;
 import org.opentripplanner.routing.algorithm.filterchain.filters.transit.RemoveItinerariesWithShortStreetLeg;
@@ -64,7 +67,6 @@ public class ItineraryListFilterChainBuilder {
   private static final int NOT_SET = -1;
   private final SortOrder sortOrder;
   private final List<GroupBySimilarity> groupBySimilarity = new ArrayList<>();
-
   private ItineraryFilterDebugProfile debug = ItineraryFilterDebugProfile.OFF;
   private int maxNumberOfItineraries = NOT_SET;
   private ListSection maxNumberOfItinerariesCropSection = ListSection.TAIL;
@@ -86,6 +88,7 @@ public class ItineraryListFilterChainBuilder {
   private double minBikeParkingDistance;
   private boolean removeTransitIfWalkingIsBetter = true;
   private ItinerarySortKey itineraryPageCut;
+  private boolean transitGroupPriorityUsed = false;
 
   /**
    * Sandbox filters which decorate the itineraries with extra information.
@@ -292,6 +295,15 @@ public ItineraryListFilterChainBuilder withPagingDeduplicationFilter(
     return this;
   }
 
+  /**
+   * Adjust filters to include multi-criteria parameter c2 and treat it as the
+   * transit-group.
+   */
+  public ItineraryListFilterChainBuilder withTransitGroupPriority() {
+    this.transitGroupPriorityUsed = true;
+    return this;
+  }
+
   /**
    * If set, walk-all-the-way itineraries are removed. This happens AFTER e.g. the group-by and
    * remove-transit-with-higher-cost-than-best-on-street-only filter. This make sure that poor
@@ -531,7 +543,7 @@ private ItineraryListFilter buildGroupBySameRoutesAndStopsFilter() {
       GroupBySameRoutesAndStops::new,
       List.of(
         new SortingFilter(SortOrderComparator.comparator(sortOrder)),
-        new RemoveFilter(new MaxLimit(GroupBySameRoutesAndStops.TAG, 1))
+        new RemoveFilter(createMaxLimitFilter(GroupBySameRoutesAndStops.TAG, 1))
       )
     );
   }
@@ -574,7 +586,7 @@ private List<ItineraryListFilter> buildGroupByTripIdAndDistanceFilters() {
             GroupByAllSameStations::new,
             List.of(
               new SortingFilter(generalizedCostComparator()),
-              new RemoveFilter(new MaxLimit(innerGroupName, 1))
+              new RemoveFilter(createMaxLimitFilter(innerGroupName, 1))
             )
           )
         );
@@ -587,7 +599,7 @@ private List<ItineraryListFilter> buildGroupByTripIdAndDistanceFilters() {
       }
 
       addSort(nested, generalizedCostComparator());
-      addRemoveFilter(nested, new MaxLimit(tag, group.maxNumOfItinerariesPerGroup));
+      addRemoveFilter(nested, createMaxLimitFilter(tag, group.maxNumOfItinerariesPerGroup));
 
       nested.add(new KeepItinerariesWithFewestTransfers(sysTags));
 
@@ -620,4 +632,20 @@ private static void addDecorateFilter(
   ) {
     filters.add(new DecorateFilter(decorator));
   }
+
+  private RemoveItineraryFlagger createMaxLimitFilter(String filterName, int maxLimit) {
+    if (OTPFeature.MultiCriteriaGroupMaxFilter.isOn()) {
+      List<SingeCriteriaComparator> comparators = new ArrayList<>();
+      comparators.add(SingeCriteriaComparator.compareGeneralizedCost());
+      comparators.add(SingeCriteriaComparator.compareNumTransfers());
+      if (transitGroupPriorityUsed) {
+        comparators.add(SingeCriteriaComparator.compareTransitGroupsPriority());
+      }
+      return new McMaxLimitFilter(filterName, maxLimit, comparators);
+    }
+    // Default is to just use a "hard" max limit
+    else {
+      return new MaxLimit(filterName, maxLimit);
+    }
+  }
 }

src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/SingeCriteriaComparator.java

@@ -0,0 +1,68 @@
+package org.opentripplanner.routing.algorithm.filterchain.filters.system;
+
+import java.util.Comparator;
+import java.util.function.ToIntFunction;
+import org.opentripplanner.model.plan.Itinerary;
+import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.grouppriority.TransitGroupPriority32n;
+
+/**
+ * Comparator used to compare a SINGE criteria for dominance. The difference between this and the
+ * {@link org.opentripplanner.raptor.util.paretoset.ParetoComparator} is that:
+ * <ol>
+ *   <li>This applies to one criteria, not multiple.</li>
+ *   <li>This interface apply to itineraries; It is not generic.</li>
+ * </ol>
+ * A set of instances of this interface can be used to create a pareto-set. See
+ * {@link org.opentripplanner.raptor.util.paretoset.ParetoSet} and
+ * {@link org.opentripplanner.raptor.util.paretoset.ParetoComparator}.
+ * <p/>
+ * This interface extends {@link Comparator} so elements can be sorted as well. Not all criteria
+ * can be sorted, if so the {@link #strictOrder()} should return false (this is the default).
+ */
+@FunctionalInterface
+public interface SingeCriteriaComparator {
+  /**
+   * The left criteria dominates the right criteria. Note! The right criteria my dominate
+   * the left criteria if there is no {@link #strictOrder()}. If left and right are equals, then
+   * there is no dominance.
+   */
+  boolean leftDominanceExist(Itinerary left, Itinerary right);
+
+  /**
+   * Return true if the criteria can be deterministically sorted.
+   */
+  default boolean strictOrder() {
+    return false;
+  }
+
+  static SingeCriteriaComparator compareNumTransfers() {
+    return compareLessThan(Itinerary::getNumberOfTransfers);
+  }
+
+  static SingeCriteriaComparator compareGeneralizedCost() {
+    return compareLessThan(Itinerary::getGeneralizedCost);
+  }
+
+  @SuppressWarnings("OptionalGetWithoutIsPresent")
+  static SingeCriteriaComparator compareTransitGroupsPriority() {
+    return (left, right) ->
+      TransitGroupPriority32n.dominate(
+        left.getGeneralizedCost2().get(),
+        right.getGeneralizedCost2().get()
+      );
+  }
+
+  static SingeCriteriaComparator compareLessThan(final ToIntFunction<Itinerary> op) {
+    return new SingeCriteriaComparator() {
+      @Override
+      public boolean leftDominanceExist(Itinerary left, Itinerary right) {
+        return op.applyAsInt(left) < op.applyAsInt(right);
+      }
+
+      @Override
+      public boolean strictOrder() {
+        return true;
+      }
+    };
+  }
+}

src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/Group.java

@@ -0,0 +1,55 @@
+package org.opentripplanner.routing.algorithm.filterchain.filters.system.mcmax;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * The purpose of a group is to maintain a list of items, all optimal for a single
+ * criteria/comparator. After the group is created, then the criteria is no longer needed, so we do
+ * not keep a reference to the original criteria.
+ */
+class Group implements Iterable<Item> {
+
+  private final List<Item> items = new ArrayList<>();
+
+  public Group(Item firstItem) {
+    add(firstItem);
+  }
+
+  Item first() {
+    return items.getFirst();
+  }
+
+  boolean isEmpty() {
+    return items.isEmpty();
+  }
+
+  boolean isSingleItemGroup() {
+    return items.size() == 1;
+  }
+
+  void add(Item item) {
+    item.incGroupCount();
+    items.add(item);
+  }
+
+  void removeAllItems() {
+    items.forEach(Item::decGroupCount);
+    items.clear();
+  }
+
+  void addNewDominantItem(Item item) {
+    removeAllItems();
+    add(item);
+  }
+
+  boolean contains(Item item) {
+    return this.items.contains(item);
+  }
+
+  @Override
+  public Iterator<Item> iterator() {
+    return items.iterator();
+  }
+}

src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/Item.java

@@ -0,0 +1,47 @@
+package org.opentripplanner.routing.algorithm.filterchain.filters.system.mcmax;
+
+import org.opentripplanner.model.plan.Itinerary;
+
+/**
+ * An item is a decorated itinerary. The extra information added is the index in the input list
+ * (sort order) and a groupCount. The sort order is used to break ties, while the group-count is
+ * used to select the itinerary witch exist in the highest number of groups. The group dynamically
+ * updates the group-count; The count is incremented when an item is added to a group, and
+ * decremented when the group is removed from the State.
+ */
+class Item {
+
+  private final Itinerary item;
+  private final int index;
+  private int groupCount = 0;
+
+  Item(Itinerary item, int index) {
+    this.item = item;
+    this.index = index;
+  }
+
+  /**
+   * An item is better than another if the groupCount is higher, and in case of a tie, if the sort
+   * index is lower.
+   */
+  public boolean betterThan(Item o) {
+    return groupCount != o.groupCount ? groupCount > o.groupCount : index < o.index;
+  }
+
+  Itinerary item() {
+    return item;
+  }
+
+  void incGroupCount() {
+    ++this.groupCount;
+  }
+
+  void decGroupCount() {
+    --this.groupCount;
+  }
+
+  @Override
+  public String toString() {
+    return "Item #%d {count:%d, %s}".formatted(index, groupCount, item.toStr());
+  }
+}

src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/McMaxLimitFilter.java

@@ -0,0 +1,105 @@
+package org.opentripplanner.routing.algorithm.filterchain.filters.system.mcmax;
+
+import java.util.List;
+import java.util.function.Predicate;
+import org.opentripplanner.model.plan.Itinerary;
+import org.opentripplanner.routing.algorithm.filterchain.filters.system.SingeCriteriaComparator;
+import org.opentripplanner.routing.algorithm.filterchain.framework.spi.RemoveItineraryFlagger;
+
+/**
+ * This filter is used to reduce a set of itineraries down to the specified limit, if possible.
+ * The filter is guaranteed to keep at least the given {@code minNumItineraries} and/or the best
+ * itinerary for each criterion. The criterion is defined using the list of {@code comparators}.
+ * <p>
+ * The main usage of this filter is to combine it with a transit grouping filter and for each group
+ * make sure there is at least {@code minNumItineraries} and that the best itinerary with respect
+ * to each criterion is kept. So, if the grouping is based on time and riding common trips, then
+ * this filter will use the reminding criterion (transfers, generalized-cost,
+ * [transit-group-priority]) to filter the grouped set of itineraries. DO NOT INCLUDE CRITERIA
+ * USED TO GROUP THE ITINERARIES, ONLY THE REMINDING CRITERION USED IN THE RAPTOR SEARCH.
+ * <p>
+ * <b>IMPLEMENTATION DETAILS</b>
+ * <p>
+ * This is not a trivial problem. In most cases, the best itinerary for a given criteria is unique,
+ * but there might be ties - same number of transfers, same cost, and/or different priority groups.
+ * In case of a tie, we will look if an itinerary is "best-in-group" for more than one criterion,
+ * if so we pick the one witch is best in the highest number of groups. Again, if there is a tie
+ * (best in the same number of groups), then we fall back to the given itinerary sorting order.
+ * <p>
+ * This filter will use the order of the input itineraries to break ties. So, make sure to call the
+ * appropriate sort function before this filter is invoked.
+ * <p>
+ * Note! For a criteria like num-of-transfers or generalized-cost, there is only one set of "best"
+ * itineraries, and usually there are only one or a few itineraries. In case there is more than one,
+ * picking just one is fine. But, for transit-group-priority there might be more than one optimal
+ * set of itineraries. For each set, we need to pick one itinerary for the final result. Each of
+ * these sets may or may not have more than one itinerary. If you group by agency, then there will
+ * be at least one itinerary for each agency present in the result (simplified, an itinerary may
+ * consist of legs with different agencies). The transit-group-priority pareto-function used by
+ * Raptor is reused, so we do not need to worry about the logic here.
+ * <p>
+ * Let's discuss an example (this example also exists as a unit-test case):
+ * <pre>
+ *   minNumItineraries = 4
+ *   comparators = [ generalized-cost, min-num-transfers, transit-group-priority ]
+ *   itineraries: [
+ *    #0 : [ 1000, 2, (a) ]
+ *    #1 : [ 1000, 3, (a,b) ]
+ *    #2 : [ 1000, 3, (b) ]
+ *    #3 : [ 1200, 1, (a,b) ]
+ *    #4 : [ 1200, 1, (a) ]
+ *    #5 : [ 1300, 2, (c) ]
+ *    #6 : [ 1300, 3, (c) ]
+ *   ]
+ * </pre>
+ * The best itineraries by generalized-cost are (#0, #1, #2). The best itineraries by
+ * min-num-transfers are (#3, #4). The best itineraries by transit-group-priority are
+ * (a:(#0, #4), b:(#2), c:(#5, #6)).
+ * <p>
+ * So we need to pick one from each group (#0, #1, #2), (#3, #4), (#0, #4), (#2), and (#5, #6).
+ * Since #2 is a single, we pick it first. Itinerary #2 is also one of the best
+ * generalized-cost itineraries - so we are done with generalized-cost itineraries as well. The two
+ * groups left are (#3, #4), (#0, #4), and (#5, #6). #4 exists in 2 groups, so we pick it next. Now
+ * we are left with (#5, #6). To break the tie, we look at the sort-order. We pick
+ * itinerary #5. Result: #2, #4, and #5.
+ * <p>
+ * The `minNumItineraries` limit is not met, so we need to pick another itinerary, we use the
+ * sort-order again and add itinerary #0. The result returned is: [#0, #2, #4, #5]
+ */
+public class McMaxLimitFilter implements RemoveItineraryFlagger {
+
+  private final String name;
+  private final int minNumItineraries;
+  private final List<SingeCriteriaComparator> comparators;
+
+  public McMaxLimitFilter(
+    String name,
+    int minNumItineraries,
+    List<SingeCriteriaComparator> comparators
+  ) {
+    this.name = name;
+    this.minNumItineraries = minNumItineraries;
+    this.comparators = comparators;
+  }
+
+  @Override
+  public String name() {
+    return name;
+  }
+
+  @Override
+  public List<Itinerary> flagForRemoval(List<Itinerary> itineraries) {
+    if (itineraries.size() <= minNumItineraries) {
+      return List.of();
+    }
+    var state = new State(itineraries, comparators);
+    state.findAllSingleItemGroupsAndAddTheItemToTheResult();
+    state.findTheBestItemsUntilAllGroupsAreRepresentedInTheResult();
+    state.fillUpTheResultWithMinimumNumberOfItineraries(minNumItineraries);
+
+    // We now have the itineraries we want, but we must invert this and return the
+    // list of itineraries to drop - keeping the original order
+    var ok = state.getResult();
+    return itineraries.stream().filter(Predicate.not(ok::contains)).toList();
+  }
+}