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(false, 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/model/plan/grouppriority/TransitGroupPriorityItineraryDecorator.java

@@ -40,7 +40,7 @@ public void decorate(Itinerary itinerary) {
       for (Leg leg : itinerary.getLegs()) {
         if (leg.getTrip() != null) {
           int newGroupId = priorityGroupConfigurator.lookupTransitGroupPriorityId(leg.getTrip());
-          c2 = transitGroupCalculator.mergeGroupIds(c2, newGroupId);
+          c2 = transitGroupCalculator.mergeInGroupId(c2, newGroupId);
         }
       }
       itinerary.setGeneralizedCost2(c2);

src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitGroupPriorityCalculator.java

@@ -11,7 +11,7 @@ public interface RaptorTransitGroupPriorityCalculator {
    * @param boardingGroupId the transit group id to add to the given set.
    * @return the new computed set of groupIds
    */
-  int mergeGroupIds(int currentGroupIds, int boardingGroupId);
+  int mergeInGroupId(int currentGroupIds, int boardingGroupId);
 
   /**
    * This is the dominance function to use for comparing transit-groups.

src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitGroupPriorityRideFactory.java

@@ -55,6 +55,6 @@ public void prepareForTransitWith(RaptorTripPattern pattern) {
    * Currently transit-group-priority is the only usage of c2
    */
   private int calculateC2(int c2) {
-    return transitGroupPriorityCalculator.mergeGroupIds(c2, currentPatternGroupPriority);
+    return transitGroupPriorityCalculator.mergeInGroupId(c2, currentPatternGroupPriority);
   }
 }

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.SingleCriteriaComparator;
+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<SingleCriteriaComparator> comparators = new ArrayList<>();
+      comparators.add(SingleCriteriaComparator.compareGeneralizedCost());
+      comparators.add(SingleCriteriaComparator.compareNumTransfers());
+      if (transitGroupPriorityUsed) {
+        comparators.add(SingleCriteriaComparator.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/SingleCriteriaComparator.java

@@ -0,0 +1,69 @@
+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.transit.model.network.grouppriority.DefaultTransitGroupPriorityCalculator;
+
+/**
+ * Comparator used to compare a SINGLE 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 applies 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 SingleCriteriaComparator {
+  DefaultTransitGroupPriorityCalculator GROUP_PRIORITY_CALCULATOR = new DefaultTransitGroupPriorityCalculator();
+
+  /**
+   * The left criteria dominates the right criteria. Note! The right criteria may 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 SingleCriteriaComparator compareNumTransfers() {
+    return compareLessThan(Itinerary::getNumberOfTransfers);
+  }
+
+  static SingleCriteriaComparator compareGeneralizedCost() {
+    return compareLessThan(Itinerary::getGeneralizedCost);
+  }
+
+  @SuppressWarnings("OptionalGetWithoutIsPresent")
+  static SingleCriteriaComparator compareTransitGroupsPriority() {
+    return (left, right) ->
+      GROUP_PRIORITY_CALCULATOR
+        .dominanceFunction()
+        .leftDominateRight(left.getGeneralizedCost2().get(), right.getGeneralizedCost2().get());
+  }
+
+  static SingleCriteriaComparator compareLessThan(final ToIntFunction<Itinerary> op) {
+    return new SingleCriteriaComparator() {
+      @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());
+  }
+}