diff --git a/docs/Configuration.md b/docs/Configuration.md index a4e8b5100a4..7e48dab5344 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -219,36 +219,37 @@ Here is a list of all features which can be toggled on/off and their default val -| Feature | Description | Enabled by default | Sandbox | -|--------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------------------:|:-------:| -| `APIBikeRental` | Enable the bike rental endpoint. | ✓️ | | -| `APIServerInfo` | Enable the server info endpoint. | ✓️ | | -| `APIUpdaterStatus` | Enable endpoint for graph updaters status. | ✓️ | | -| `ConsiderPatternsForDirectTransfers` | Enable limiting transfers so that there is only a single transfer to each pattern. | ✓️ | | -| `DebugUi` | Enable the debug GraphQL client and web UI and located at the root of the web server as well as the debug map tiles it uses. Be aware that the map tiles are not a stable API and can change without notice. Use the [vector tiles feature if](sandbox/MapboxVectorTilesApi.md) you want a stable map tiles API. | ✓️ | | -| `FloatingBike` | Enable floating bike routing. | ✓️ | | -| `GtfsGraphQlApi` | Enable the [GTFS GraphQL API](apis/GTFS-GraphQL-API.md). | ✓️ | | -| `GtfsGraphQlApiRentalStationFuzzyMatching` | Does vehicleRentalStation query also allow ids that are not feed scoped. | | | -| `MinimumTransferTimeIsDefinitive` | If the minimum transfer time is a lower bound (default) or the definitive time for the transfer. Set this to `true` if you want to set a transfer time lower than what OTP derives from OSM data. | | | -| `OptimizeTransfers` | OTP will inspect all itineraries found and optimize where (which stops) the transfer will happen. Waiting time, priority and guaranteed transfers are taken into account. | ✓️ | | -| `ParallelRouting` | Enable performing parts of the trip planning in parallel. | | | -| `TransferConstraints` | Enforce transfers to happen according to the _transfers.txt_ (GTFS) and Interchanges (NeTEx). Turning this _off_ will increase the routing performance a little. | ✓️ | | -| `TransmodelGraphQlApi` | Enable the [Transmodel (NeTEx) GraphQL API](apis/TransmodelApi.md). | ✓️ | ✓️ | -| `ActuatorAPI` | Endpoint for actuators (service health status). | | ✓️ | -| `AsyncGraphQLFetchers` | Whether the @async annotation in the GraphQL schema should lead to the fetch being executed asynchronously. This allows batch or alias queries to run in parallel at the cost of consuming extra threads. | | | -| `Co2Emissions` | Enable the emissions sandbox module. | | ✓️ | -| `DataOverlay` | Enable usage of data overlay when calculating costs for the street network. | | ✓️ | -| `FaresV2` | Enable import of GTFS-Fares v2 data. | | ✓️ | -| `FlexRouting` | Enable FLEX routing. | | ✓️ | -| `GoogleCloudStorage` | Enable Google Cloud Storage integration. | | ✓️ | -| `LegacyRestApi` | Enable legacy REST API. This API will be removed in the future. | | ✓️ | -| `RealtimeResolver` | When routing with ignoreRealtimeUpdates=true, add an extra step which populates results with real-time data | | ✓️ | -| `ReportApi` | Enable the report API. | | ✓️ | -| `RestAPIPassInDefaultConfigAsJson` | Enable a default RouteRequest to be passed in as JSON on the REST API - FOR DEBUGGING ONLY! | | | -| `SandboxAPIGeocoder` | Enable the Geocoder API. | | ✓️ | -| `SandboxAPIMapboxVectorTilesApi` | Enable Mapbox vector tiles API. | | ✓️ | -| `SandboxAPIParkAndRideApi` | Enable park-and-ride endpoint. | | ✓️ | -| `TransferAnalyzer` | Analyze transfers during graph build. | | ✓️ | +| Feature | Description | Enabled by default | Sandbox | +|--------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------------------:|:-------:| +| `APIBikeRental` | Enable the bike rental endpoint. | ✓️ | | +| `APIServerInfo` | Enable the server info endpoint. | ✓️ | | +| `APIUpdaterStatus` | Enable endpoint for graph updaters status. | ✓️ | | +| `ConsiderPatternsForDirectTransfers` | Enable limiting transfers so that there is only a single transfer to each pattern. | ✓️ | | +| `DebugUi` | Enable the debug GraphQL client and web UI and located at the root of the web server as well as the debug map tiles it uses. Be aware that the map tiles are not a stable API and can change without notice. Use the [vector tiles feature if](sandbox/MapboxVectorTilesApi.md) you want a stable map tiles API. | ✓️ | | +| `FloatingBike` | Enable floating bike routing. | ✓️ | | +| `GtfsGraphQlApi` | Enable the [GTFS GraphQL API](apis/GTFS-GraphQL-API.md). | ✓️ | | +| `GtfsGraphQlApiRentalStationFuzzyMatching` | Does vehicleRentalStation query also allow ids that are not feed scoped. | | | +| `MinimumTransferTimeIsDefinitive` | If the minimum transfer time is a lower bound (default) or the definitive time for the transfer. Set this to `true` if you want to set a transfer time lower than what OTP derives from OSM data. | | | +| `OptimizeTransfers` | OTP will inspect all itineraries found and optimize where (which stops) the transfer will happen. Waiting time, priority and guaranteed transfers are taken into account. | ✓️ | | +| `ParallelRouting` | Enable performing parts of the trip planning in parallel. | | | +| `TransferConstraints` | Enforce transfers to happen according to the _transfers.txt_ (GTFS) and Interchanges (NeTEx). Turning this _off_ will increase the routing performance a little. | ✓️ | | +| `TransmodelGraphQlApi` | Enable the [Transmodel (NeTEx) GraphQL API](apis/TransmodelApi.md). | ✓️ | ✓️ | +| `ActuatorAPI` | Endpoint for actuators (service health status). | | ✓️ | +| `AsyncGraphQLFetchers` | Whether the @async annotation in the GraphQL schema should lead to the fetch being executed asynchronously. This allows batch or alias queries to run in parallel at the cost of consuming extra threads. | | | +| `Co2Emissions` | Enable the emissions sandbox module. | | ✓️ | +| `DataOverlay` | Enable usage of data overlay when calculating costs for the street network. | | ✓️ | +| `FaresV2` | Enable import of GTFS-Fares v2 data. | | ✓️ | +| `FlexRouting` | Enable FLEX routing. | | ✓️ | +| `GoogleCloudStorage` | Enable Google Cloud Storage integration. | | ✓️ | +| `LegacyRestApi` | Enable legacy REST API. This API will be removed in the future. | | ✓️ | +| `MultiCriteriaGroupMaxFilter` | 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` | When routing with ignoreRealtimeUpdates=true, add an extra step which populates results with real-time data | | ✓️ | +| `ReportApi` | Enable the report API. | | ✓️ | +| `RestAPIPassInDefaultConfigAsJson` | Enable a default RouteRequest to be passed in as JSON on the REST API - FOR DEBUGGING ONLY! | | | +| `SandboxAPIGeocoder` | Enable the Geocoder API. | | ✓️ | +| `SandboxAPIMapboxVectorTilesApi` | Enable Mapbox vector tiles API. | | ✓️ | +| `SandboxAPIParkAndRideApi` | Enable park-and-ride endpoint. | | ✓️ | +| `TransferAnalyzer` | Analyze transfers during graph build. | | ✓️ | diff --git a/src/main/java/org/opentripplanner/framework/application/OTPFeature.java b/src/main/java/org/opentripplanner/framework/application/OTPFeature.java index 8ee5d100b94..749772fae12 100644 --- a/src/main/java/org/opentripplanner/framework/application/OTPFeature.java +++ b/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, diff --git a/src/main/java/org/opentripplanner/model/plan/grouppriority/TransitGroupPriorityItineraryDecorator.java b/src/main/java/org/opentripplanner/model/plan/grouppriority/TransitGroupPriorityItineraryDecorator.java index 80b7b39bd0c..9c89e9d4ca8 100644 --- a/src/main/java/org/opentripplanner/model/plan/grouppriority/TransitGroupPriorityItineraryDecorator.java +++ b/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); diff --git a/src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitGroupPriorityCalculator.java b/src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitGroupPriorityCalculator.java index 06c10b51daf..2b96a7b1470 100644 --- a/src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitGroupPriorityCalculator.java +++ b/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. diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitGroupPriorityRideFactory.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitGroupPriorityRideFactory.java index 79ae1558836..323067cc240 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitGroupPriorityRideFactory.java +++ b/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); } } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java index e8b8ed43c1c..071814a7abf 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/ItineraryListFilterChainBuilder.java +++ b/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 = 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 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 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 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); + } + } } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/SingleCriteriaComparator.java b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/SingleCriteriaComparator.java new file mode 100644 index 00000000000..fdaaf0d7657 --- /dev/null +++ b/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: + *
    + *
  1. This applies to one criteria, not multiple.
    2. + *
  2. This interface applies to itineraries; It is not generic.
    3. + *
+ * 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}. + *

+ * 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 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; + } + }; + } +} diff --git a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/Group.java b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/Group.java new file mode 100644 index 00000000000..7bfdad83e8f --- /dev/null +++ b/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 { + + private final List 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 iterator() { + return items.iterator(); + } +} diff --git a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/Item.java b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/Item.java new file mode 100644 index 00000000000..36c3d662493 --- /dev/null +++ b/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()); + } +} diff --git a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/McMaxLimitFilter.java b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/McMaxLimitFilter.java new file mode 100644 index 00000000000..c0b07e7400a --- /dev/null +++ b/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.SingleCriteriaComparator; +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}. + *

+ * 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 remaining 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 REMAINING CRITERION USED IN THE RAPTOR SEARCH. + *

+ * IMPLEMENTATION DETAILS + *

+ * 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 which 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. + *

+ * 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. + *

+ * Note! For 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. + *

+ * Let's discuss an example (this example also exists as a unit-test case): + * 

+ *   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) ]
+ *   ]
+ *
+ * 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)). + *

+ * 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. + *

+ * 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 comparators; + + public McMaxLimitFilter( + String name, + int minNumItineraries, + List comparators + ) { + this.name = name; + this.minNumItineraries = minNumItineraries; + this.comparators = comparators; + } + + @Override + public String name() { + return name; + } + + @Override + public List flagForRemoval(List 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(); + } +} diff --git a/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/State.java b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/State.java new file mode 100644 index 00000000000..93b8b1097c9 --- /dev/null +++ b/src/main/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/State.java @@ -0,0 +1,201 @@ +package org.opentripplanner.routing.algorithm.filterchain.filters.system.mcmax; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import javax.annotation.Nullable; +import org.opentripplanner.model.plan.Itinerary; +import org.opentripplanner.routing.algorithm.filterchain.filters.system.SingleCriteriaComparator; + +/** + * Keep a list of items, groups and the result in progress. This is just a class for + * simple bookkeeping for the state of the filter. + */ +class State { + + private final List items; + private final List groups; + private final List result = new ArrayList<>(); + + /** + * Initialize the state by wrapping each itinerary in an item (with index) and create groups for + * each criterion with the best itineraries (can be more than one with, for example, the same + * cost). There should be at least one itinerary from each group surviving the filtering process. + * The same itinerary can exist in multiple groups. + */ + State(List itineraries, List comparators) { + this.items = createListOfItems(itineraries); + this.groups = createGroups(items, comparators); + } + + List getResult() { + return result.stream().map(Item::item).toList(); + } + + /** + * Find and add all groups with a single item in them and add them to the result + */ + void findAllSingleItemGroupsAndAddTheItemToTheResult() { + var item = findItemInFirstSingleItemGroup(groups); + while (item != null) { + addToResult(item); + item = findItemInFirstSingleItemGroup(groups); + } + } + + /** + * Find the items with the highest group count and the lowest index. Theoretically, there might be + * a smaller set of itineraries that TOGETHER represent all groups than what we achieve here, but + * it is far more complicated to compute - so this is probably good enough. + */ + void findTheBestItemsUntilAllGroupsAreRepresentedInTheResult() { + while (!groups.isEmpty()) { + addToResult(findBestItem(groups)); + } + } + + /** + * Fill up with itineraries until the minimum number of itineraries is reached + */ + void fillUpTheResultWithMinimumNumberOfItineraries(int minNumItineraries) { + int end = Math.min(items.size(), minNumItineraries); + for (int i = 0; result.size() < end; ++i) { + var it = items.get(i); + if (!result.contains(it)) { + result.add(it); + } + } + } + + private void addToResult(Item item) { + result.add(item); + removeGroupsWitchContainsItem(item); + } + + /** + * If an itinerary is accepted into the final result, then all groups that contain that itinerary + * can be removed. In addition, the item groupCount should be decremented if a group is dropped. + * This makes sure that the groups represented in the final result do not count when selecting the + * next item. + */ + private void removeGroupsWitchContainsItem(Item item) { + for (Group group : groups) { + if (group.contains(item)) { + group.removeAllItems(); + } + } + groups.removeIf(Group::isEmpty); + } + + /** + * The best item is the one which exists in most groups, and in case of a tie, the sort order/ + * itinerary index is used. + */ + private static Item findBestItem(List groups) { + var candidate = groups.getFirst().first(); + for (Group group : groups) { + for (Item item : group) { + if (item.betterThan(candidate)) { + candidate = item; + } + } + } + return candidate; + } + + /** + * Search through all groups and return all items witch comes from groups with only one item. + */ + @Nullable + private static Item findItemInFirstSingleItemGroup(List groups) { + return groups + .stream() + .filter(Group::isSingleItemGroup) + .findFirst() + .map(Group::first) + .orElse(null); + } + + private static ArrayList createListOfItems(List itineraries) { + var items = new ArrayList(); + for (int i = 0; i < itineraries.size(); i++) { + items.add(new Item(itineraries.get(i), i)); + } + return items; + } + + private static List createGroups( + Collection items, + List comparators + ) { + List groups = new ArrayList<>(); + for (SingleCriteriaComparator comparator : comparators) { + if (comparator.strictOrder()) { + groups.add(createOrderedGroup(items, comparator)); + } else { + groups.addAll(createUnorderedGroups(items, comparator)); + } + } + return groups; + } + + /** + * In a strict ordered group only one optimal value exist for the criteria defined by the given + * {@code comparator}. All items that have this value should be included in the group created. + */ + private static Group createOrderedGroup( + Collection items, + SingleCriteriaComparator comparator + ) { + Group group = null; + for (Item item : items) { + if (group == null) { + group = new Group(item); + continue; + } + var current = group.first(); + if (comparator.leftDominanceExist(item.item(), current.item())) { + group.addNewDominantItem(item); + } else if (!comparator.leftDominanceExist(current.item(), item.item())) { + group.add(item); + } + } + return group; + } + + /** + * For a none strict ordered criteria, multiple optimal values exist. The criterion is defined by + * the given {@code comparator}. This method will create a group for each optimal value found in + * the given set of items. + * + * @see #createOrderedGroup(Collection, SingleCriteriaComparator) + */ + private static Collection createUnorderedGroups( + Collection items, + SingleCriteriaComparator comparator + ) { + List result = new ArrayList<>(); + + for (Item item : items) { + int groupCount = result.size(); + for (Group group : result) { + var groupItem = group.first().item(); + if (comparator.leftDominanceExist(groupItem, item.item())) { + if (comparator.leftDominanceExist(item.item(), groupItem)) { + // Mutual dominance => the item belong in another group + --groupCount; + } + } else { + if (comparator.leftDominanceExist(item.item(), groupItem)) { + group.removeAllItems(); + } + group.add(item); + } + } + if (groupCount == 0) { + result.add(new Group(item)); + } + } + return result; + } +} diff --git a/src/main/java/org/opentripplanner/routing/algorithm/mapping/RouteRequestToFilterChainMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/mapping/RouteRequestToFilterChainMapper.java index 651d94b4eac..c1fab68f999 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/mapping/RouteRequestToFilterChainMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/mapping/RouteRequestToFilterChainMapper.java @@ -94,6 +94,10 @@ public static ItineraryListFilterChain createFilterChain( .withRemoveTransitIfWalkingIsBetter(true) .withDebugEnabled(params.debug()); + if (!request.preferences().transit().relaxTransitGroupPriority().isNormal()) { + builder.withTransitGroupPriority(); + } + var fareService = context.graph().getFareService(); if (fareService != null) { builder.withFareDecorator(new DecorateWithFare(fareService)); diff --git a/src/main/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculator.java b/src/main/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculator.java index df5d2abef8a..3e96cf18f95 100644 --- a/src/main/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculator.java +++ b/src/main/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculator.java @@ -10,7 +10,7 @@ public final class DefaultTransitGroupPriorityCalculator implements RaptorTransitGroupPriorityCalculator { @Override - public int mergeGroupIds(int currentGroupIds, int boardingGroupId) { + public int mergeInGroupId(int currentGroupIds, int boardingGroupId) { return TransitGroupPriority32n.mergeInGroupId(currentGroupIds, boardingGroupId); } diff --git a/src/test/java/org/opentripplanner/datastore/file/ZipFileDataSourceTest.java b/src/test/java/org/opentripplanner/datastore/file/ZipFileDataSourceTest.java index da46fc430b6..b5b8f797eb3 100644 --- a/src/test/java/org/opentripplanner/datastore/file/ZipFileDataSourceTest.java +++ b/src/test/java/org/opentripplanner/datastore/file/ZipFileDataSourceTest.java @@ -74,7 +74,6 @@ public void testIO() throws IOException { Collection content = subject.content(); Collection names = content.stream().map(DataSource::name).toList(); - //System.out.println(names); assertTrue( names.containsAll(List.of("agency.txt", "stops.txt", "trips.txt")), names.toString() diff --git a/src/test/java/org/opentripplanner/datastore/file/ZipStreamDataSourceDecoratorTest.java b/src/test/java/org/opentripplanner/datastore/file/ZipStreamDataSourceDecoratorTest.java index 09479d85478..21d4bc6bed0 100644 --- a/src/test/java/org/opentripplanner/datastore/file/ZipStreamDataSourceDecoratorTest.java +++ b/src/test/java/org/opentripplanner/datastore/file/ZipStreamDataSourceDecoratorTest.java @@ -100,7 +100,6 @@ void testIO() throws IOException { Collection content = subject.content(); Collection names = content.stream().map(DataSource::name).toList(); - System.out.println(names); assertTrue(names.containsAll(EXPECTED_ZIP_ENTRIES), names.toString()); DataSource entry = subject.entry("agency.txt"); diff --git a/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java b/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java index 28be5b3a7e2..edaafabd753 100644 --- a/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java +++ b/src/test/java/org/opentripplanner/model/plan/TestItineraryBuilder.java @@ -55,6 +55,8 @@ */ public class TestItineraryBuilder implements PlanTestConstants { + private static final int NOT_SET = -1_999_999; + public static final LocalDate SERVICE_DAY = LocalDate.of(2020, Month.FEBRUARY, 2); public static final Route BUS_ROUTE = route("1").withMode(TransitMode.BUS).build(); public static final Route RAIL_ROUTE = route("2").withMode(TransitMode.RAIL).build(); @@ -69,7 +71,8 @@ public class TestItineraryBuilder implements PlanTestConstants { private final List legs = new ArrayList<>(); private Place lastPlace; private int lastEndTime; - private int cost = 0; + private int c1 = 0; + private int c2 = NOT_SET; private TestItineraryBuilder(Place origin, int startTime) { this.lastPlace = origin; @@ -241,7 +244,7 @@ public TestItineraryBuilder flex(int start, int end, Place to) { FlexibleTransitLeg leg = new FlexibleTransitLeg(edge, newTime(start), newTime(end), legCost); legs.add(leg); - cost += legCost; + c1 += legCost; // Setup for adding another leg lastEndTime = end; @@ -330,17 +333,6 @@ public TestItineraryBuilder faresV2Rail( ); } - public Itinerary egress(int walkDuration) { - walk(walkDuration, null); - return build(); - } - - public Itinerary build() { - Itinerary itinerary = new Itinerary(legs); - itinerary.setGeneralizedCost(cost); - return itinerary; - } - public TestItineraryBuilder frequencyBus(int tripId, int startTime, int endTime, Place to) { return transit( RAIL_ROUTE, @@ -401,6 +393,34 @@ public TestItineraryBuilder carHail(int duration, Place to) { return this; } + public TestItineraryBuilder withGeneralizedCost2(int c2) { + this.c2 = c2; + return this; + } + + public Itinerary egress(int walkDuration) { + walk(walkDuration, null); + return build(); + } + + /** + * Override any value set for c1. The given value will be assigned to the itinerary + * independent of any values set on the legs. + */ + public Itinerary build(int c1) { + this.c1 = c1; + return build(); + } + + public Itinerary build() { + Itinerary itinerary = new Itinerary(legs); + itinerary.setGeneralizedCost(c1); + if (c2 != NOT_SET) { + itinerary.setGeneralizedCost2(c2); + } + return itinerary; + } + /* private methods */ /** Create a dummy trip */ @@ -506,7 +526,7 @@ public TestItineraryBuilder transit( leg.setDistanceMeters(speed(leg.getMode()) * (end - start)); legs.add(leg); - cost += legCost; + c1 += legCost; // Setup for adding another leg lastEndTime = end; @@ -536,7 +556,7 @@ private StreetLeg streetLeg( .build(); legs.add(leg); - cost += legCost; + c1 += legCost; // Setup for adding another leg lastEndTime = endTime; diff --git a/src/test/java/org/opentripplanner/raptor/moduletests/support/TestGroupPriorityCalculator.java b/src/test/java/org/opentripplanner/raptor/moduletests/support/TestGroupPriorityCalculator.java index 3234dc126fb..3debd487345 100644 --- a/src/test/java/org/opentripplanner/raptor/moduletests/support/TestGroupPriorityCalculator.java +++ b/src/test/java/org/opentripplanner/raptor/moduletests/support/TestGroupPriorityCalculator.java @@ -15,11 +15,11 @@ public class TestGroupPriorityCalculator implements RaptorTransitGroupPriorityCa public static final int GROUP_B = 0x02; public static final int GROUP_C = 0x04; - private static final int GROUP_AB = PRIORITY_CALCULATOR.mergeGroupIds(GROUP_A, GROUP_B); - private static final int GROUP_AC = PRIORITY_CALCULATOR.mergeGroupIds(GROUP_A, GROUP_C); + private static final int GROUP_AB = PRIORITY_CALCULATOR.mergeInGroupId(GROUP_A, GROUP_B); + private static final int GROUP_AC = PRIORITY_CALCULATOR.mergeInGroupId(GROUP_A, GROUP_C); @Override - public int mergeGroupIds(int currentGroupIds, int boardingGroupId) { + public int mergeInGroupId(int currentGroupIds, int boardingGroupId) { return currentGroupIds | boardingGroupId; } diff --git a/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/SingleCriteriaComparatorTest.java b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/SingleCriteriaComparatorTest.java new file mode 100644 index 00000000000..3626ada63b3 --- /dev/null +++ b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/SingleCriteriaComparatorTest.java @@ -0,0 +1,110 @@ +package org.opentripplanner.routing.algorithm.filterchain.filters.system; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opentripplanner.model.plan.TestItineraryBuilder.newItinerary; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.opentripplanner.model.plan.Itinerary; +import org.opentripplanner.model.plan.Place; +import org.opentripplanner.transit.model._data.TransitModelForTest; +import org.opentripplanner.transit.model.network.grouppriority.DefaultTransitGroupPriorityCalculator; + +class SingleCriteriaComparatorTest { + + private static final TransitModelForTest TEST_MODEL = TransitModelForTest.of(); + private static final DefaultTransitGroupPriorityCalculator GROUP_PRIORITY_CALCULATOR = new DefaultTransitGroupPriorityCalculator(); + + private static final Place A = TEST_MODEL.place("A", 10, 11); + private static final Place B = TEST_MODEL.place("B", 10, 13); + private static final Place C = TEST_MODEL.place("C", 10, 14); + private static final Place D = TEST_MODEL.place("D", 10, 15); + + private static final int START = 1000; + private static final int TX_AT = 1500; + private static final int END_LOW = 2000; + + // [Tx, Cost] => [0, 1240] + private static final Itinerary zeroTransferLowCost = newItinerary(A) + .bus(1, START, END_LOW, B) + .walk(60, C) + .build(); + // [Tx, Cost] => [0, 1360] + private static final Itinerary zeroTransferHighCost = newItinerary(A) + .bus(1, START, END_LOW, B) + .walk(120, C) + .build(); + // [Tx, Cost] => [1, 1240] + private static final Itinerary oneTransferLowCost = newItinerary(A) + .bus(1, START, TX_AT, B) + .bus(2, TX_AT, END_LOW, C) + .build(); + + @BeforeAll + static void setUp() { + assertEquals(0, zeroTransferLowCost.getNumberOfTransfers()); + assertEquals(0, zeroTransferHighCost.getNumberOfTransfers()); + assertEquals(1, oneTransferLowCost.getNumberOfTransfers()); + + int expectedCost = zeroTransferLowCost.getGeneralizedCost(); + assertTrue(expectedCost < zeroTransferHighCost.getGeneralizedCost()); + assertEquals(expectedCost, oneTransferLowCost.getGeneralizedCost()); + } + + @Test + void strictOrder() { + assertTrue(SingleCriteriaComparator.compareNumTransfers().strictOrder()); + assertTrue(SingleCriteriaComparator.compareGeneralizedCost().strictOrder()); + assertFalse(SingleCriteriaComparator.compareTransitGroupsPriority().strictOrder()); + } + + @Test + void compareNumTransfers() { + var subject = SingleCriteriaComparator.compareNumTransfers(); + + // leftDominanceExist + assertFalse(subject.leftDominanceExist(zeroTransferHighCost, zeroTransferLowCost)); + assertTrue(subject.leftDominanceExist(zeroTransferLowCost, oneTransferLowCost)); + assertFalse(subject.leftDominanceExist(oneTransferLowCost, zeroTransferLowCost)); + + // strict order expected + assertTrue(subject.strictOrder()); + } + + @Test + void compareGeneralizedCost() { + var subject = SingleCriteriaComparator.compareGeneralizedCost(); + + // leftDominanceExist + assertFalse(subject.leftDominanceExist(zeroTransferHighCost, zeroTransferLowCost)); + assertTrue(subject.leftDominanceExist(zeroTransferLowCost, zeroTransferHighCost)); + assertFalse(subject.leftDominanceExist(zeroTransferLowCost, oneTransferLowCost)); + + // strict order expected + assertTrue(subject.strictOrder()); + } + + @Test + void compareTransitPriorityGroups() { + var group1 = newItinerary(A).bus(1, START, END_LOW, C).withGeneralizedCost2(1).build(); + var group2 = newItinerary(A).bus(1, START, END_LOW, C).withGeneralizedCost2(2).build(); + var group1And2 = newItinerary(A) + .bus(1, START, END_LOW, C) + .withGeneralizedCost2(GROUP_PRIORITY_CALCULATOR.mergeInGroupId(1, 2)) + .build(); + + var subject = SingleCriteriaComparator.compareTransitGroupsPriority(); + + assertTrue(subject.leftDominanceExist(group1, group2)); + assertTrue(subject.leftDominanceExist(group2, group1)); + assertTrue(subject.leftDominanceExist(group1, group1And2)); + assertTrue(subject.leftDominanceExist(group2, group1And2)); + assertFalse(subject.leftDominanceExist(group1And2, group1)); + assertFalse(subject.leftDominanceExist(group1And2, group1)); + + // Cannot be ordered => compare will fail + assertFalse(subject.strictOrder()); + } +} diff --git a/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/ItemTest.java b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/ItemTest.java new file mode 100644 index 00000000000..c8cd04212e5 --- /dev/null +++ b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/ItemTest.java @@ -0,0 +1,56 @@ +package org.opentripplanner.routing.algorithm.filterchain.filters.system.mcmax; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opentripplanner.model.plan.TestItineraryBuilder.newItinerary; + +import org.junit.jupiter.api.Test; +import org.opentripplanner.model.plan.Itinerary; +import org.opentripplanner.model.plan.Place; +import org.opentripplanner.transit.model._data.TransitModelForTest; + +class ItemTest { + + private static final TransitModelForTest TEST_MODEL = TransitModelForTest.of(); + private static final Place A = TEST_MODEL.place("A", 10, 11); + private static final Place B = TEST_MODEL.place("B", 10, 11); + private static final Itinerary ITINERARY = newItinerary(A).bus(1, 1, 2, B).build(); + + @Test + void betterThan() { + var i1 = new Item(ITINERARY, 3); + var i2 = new Item(ITINERARY, 7); + + // i1 is better than i2 because the index is lower + assertTrue(i1.betterThan(i2)); + assertFalse(i2.betterThan(i1)); + + // Incrementing both does not change anything + i1.incGroupCount(); + i2.incGroupCount(); + assertTrue(i1.betterThan(i2)); + assertFalse(i2.betterThan(i1)); + + // Incrementing i2 make it better + i2.incGroupCount(); + assertFalse(i1.betterThan(i2)); + assertTrue(i2.betterThan(i1)); + } + + @Test + void item() { + assertSame(ITINERARY, new Item(ITINERARY, 7).item()); + } + + @Test + void testToString() { + Item item = new Item(ITINERARY, 7); + assertEquals("Item #7 {count:0, A ~ BUS 1 0:00:01 0:00:02 ~ B [C₁121]}", item.toString()); + item.incGroupCount(); + assertEquals("Item #7 {count:1, A ~ BUS 1 0:00:01 0:00:02 ~ B [C₁121]}", item.toString()); + item.decGroupCount(); + assertEquals("Item #7 {count:0, A ~ BUS 1 0:00:01 0:00:02 ~ B [C₁121]}", item.toString()); + } +} diff --git a/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/McMaxLimitFilterTest.java b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/McMaxLimitFilterTest.java new file mode 100644 index 00000000000..d5f2323ad90 --- /dev/null +++ b/src/test/java/org/opentripplanner/routing/algorithm/filterchain/filters/system/mcmax/McMaxLimitFilterTest.java @@ -0,0 +1,196 @@ +package org.opentripplanner.routing.algorithm.filterchain.filters.system.mcmax; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opentripplanner.model.plan.TestItineraryBuilder.newItinerary; + +import java.util.List; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.opentripplanner.model.plan.Itinerary; +import org.opentripplanner.model.plan.Place; +import org.opentripplanner.routing.algorithm.filterchain.filters.system.SingleCriteriaComparator; +import org.opentripplanner.transit.model._data.TransitModelForTest; +import org.opentripplanner.transit.model.network.grouppriority.DefaultTransitGroupPriorityCalculator; + +class McMaxLimitFilterTest { + + private static final TransitModelForTest TEST_MODEL = TransitModelForTest.of(); + private static final DefaultTransitGroupPriorityCalculator GROUP_PRIORITY_CALCULATOR = new DefaultTransitGroupPriorityCalculator(); + + private static final Place A = TEST_MODEL.place("A", 10, 11); + private static final Place B = TEST_MODEL.place("B", 10, 13); + private static final Place C = TEST_MODEL.place("C", 10, 14); + private static final Place D = TEST_MODEL.place("D", 10, 15); + private static final Place E = TEST_MODEL.place("E", 10, 15); + private static final Place[] PLACES = { A, B, C, D, E }; + + private static final int START = 3600 * 10; + + // Note! Each group id needs to be a power of 2. This is implementation-specific, but using the + // TransitGroupPriorityService here to generate these ids is a bit over-kill. + private static final int GROUP_A = 0x01; + private static final int GROUP_B = 0x02; + private static final int GROUP_C = 0x04; + private static final int GROUP_AB = GROUP_PRIORITY_CALCULATOR.mergeInGroupId(GROUP_A, GROUP_B); + private static final int GROUP_BC = GROUP_PRIORITY_CALCULATOR.mergeInGroupId(GROUP_B, GROUP_C); + private static final int GROUP_ABC = GROUP_PRIORITY_CALCULATOR.mergeInGroupId(GROUP_AB, GROUP_C); + + private static final boolean EXP_KEEP = true; + private static final boolean EXP_DROP = false; + + private static final int COST_LOW = 1000; + private static final int COST_MED = 1200; + private static final int COST_HIGH = 1500; + + private static final int TX_0 = 0; + private static final int TX_1 = 1; + private static final int TX_2 = 2; + + private final McMaxLimitFilter subject = new McMaxLimitFilter( + "test", + 2, + List.of( + SingleCriteriaComparator.compareGeneralizedCost(), + SingleCriteriaComparator.compareNumTransfers(), + SingleCriteriaComparator.compareTransitGroupsPriority() + ) + ); + + static TestRow row( + boolean expected, + int c1, + int nTransfers, + int transitGroups, + String description + ) { + return new TestRow(expected, c1, nTransfers, transitGroups); + } + + static List> filterTestCases() { + return List.of( + List.of(/* Should not fail for an empty list of itineraries*/), + List.of( + // Test minNumItinerariesLimit = 2 + row(EXP_KEEP, COST_LOW, TX_1, GROUP_A, "Best in everything"), + row(EXP_KEEP, COST_HIGH, TX_2, GROUP_AB, "Worse, kept because minNumItinerariesLimit is 2") + ), + List.of( + // Test minNumItinerariesLimit, first is added + row(EXP_KEEP, COST_HIGH, TX_2, GROUP_ABC, "Worst, kept because of minNumItinerariesLimit"), + row(EXP_KEEP, COST_LOW, TX_0, GROUP_A, "Best in everything"), + row(EXP_DROP, COST_HIGH, TX_1, GROUP_AB, "Dropped because not better than #2.") + ), + List.of( + // The minNumItinerariesLimit is met, so no extra itinerary(#0) is added + row(EXP_DROP, COST_HIGH, TX_2, GROUP_AB, "First element is dropped"), + row(EXP_KEEP, COST_LOW, TX_1, GROUP_B, "Best cost and group B"), + row(EXP_KEEP, COST_MED, TX_0, GROUP_A, "Best nTransfers and group A") + ), + List.of( + row(EXP_KEEP, COST_LOW, TX_2, GROUP_A, "Best: c1 and group A"), + row(EXP_DROP, COST_LOW, TX_1, GROUP_AB, "Best compromise: c1, Tx, and group AB"), + row(EXP_KEEP, COST_LOW, TX_2, GROUP_C, "Best: c1 and group C"), + row(EXP_KEEP, COST_MED, TX_0, GROUP_BC, "Best: num-of-transfers") + ), + /** + * This is the example explained in JavaDoc {@link McMaxLimitFilter} + */ + List.of( + row(EXP_DROP, COST_LOW, TX_1, GROUP_A, ""), + row(EXP_DROP, COST_LOW, TX_2, GROUP_AB, ""), + row(EXP_KEEP, COST_LOW, TX_2, GROUP_B, "Kept -> Only one in group B"), + row(EXP_DROP, COST_MED, TX_0, GROUP_AB, ""), + row(EXP_KEEP, COST_MED, TX_0, GROUP_A, "Kept -> Best transfer and group A"), + row(EXP_KEEP, COST_HIGH, TX_1, GROUP_C, "Kept -> Best group C, tie with #6"), + row(EXP_DROP, COST_HIGH, TX_2, GROUP_C, "") + ) + ); + } + + @ParameterizedTest + @MethodSource("filterTestCases") + void filterTest(List rows) { + var input = rows.stream().map(TestRow::create).toList(); + var expected = rows.stream().filter(TestRow::expected).map(TestRow::create).toList(); + + var result = subject.removeMatchesForTest(input); + + assertEquals(toStr(expected), toStr(result)); + } + + @Test + void testName() { + assertEquals("test", subject.name()); + } + + /** + * Make sure the test setup is correct - this does not test anything in src/main + */ + @Test + void testGroupsToString() { + assertEquals("A", groupsToString(GROUP_A)); + assertEquals("B", groupsToString(GROUP_B)); + assertEquals("C", groupsToString(GROUP_C)); + assertEquals("AB", groupsToString(GROUP_AB)); + assertEquals("BC", groupsToString(GROUP_BC)); + assertEquals("ABC", groupsToString(GROUP_ABC)); + } + + private static String groupsToString(int groups) { + var buf = new StringBuilder(); + char ch = 'A'; + // Check for 5 groups - the test does not use so many, but it does not matter + for (int i = 0; i < 5; ++i) { + int mask = 1 << i; + if ((groups & mask) != 0) { + buf.append(ch); + } + ch = (char) (ch + 1); + } + return buf.toString(); + } + + private static String toStr(List list) { + return list + .stream() + .map(i -> + "[ %d %d %s ]".formatted( + i.getGeneralizedCost(), + i.getNumberOfTransfers(), + groupsToString(i.getGeneralizedCost2().orElse(-1)) + ) + ) + .collect(Collectors.joining(", ")); + } + + record TestRow(boolean expected, int c1, int nTransfers, int transitGroupIds) { + Itinerary create() { + int start = START; + var builder = newItinerary(A); + + if (nTransfers < 0) { + builder.drive(start, ++start, E); + } else { + builder.bus(1, ++start, ++start, PLACES[1]); + for (int i = 0; i < nTransfers; i++) { + builder.bus(1, ++start, ++start, PLACES[i + 2]); + } + builder.withGeneralizedCost2(transitGroupIds); + } + return builder.build(c1); + } + + @Override + public String toString() { + // The red-x is a unicode character(U+274C) and should be visible in most IDEs. + return "%s %d %d %s".formatted( + expected ? "" : "❌", + c1, + nTransfers, + groupsToString(transitGroupIds) + ); + } + } +} diff --git a/src/test/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculatorTest.java b/src/test/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculatorTest.java index 745442e3e7b..34a7d3ef698 100644 --- a/src/test/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculatorTest.java +++ b/src/test/java/org/opentripplanner/transit/model/network/grouppriority/DefaultTransitGroupPriorityCalculatorTest.java @@ -13,7 +13,7 @@ class DefaultTransitGroupPriorityCalculatorTest { @Test void mergeGroupIds() { // Smoke test, should not fail - subject.mergeGroupIds(1, 2); + subject.mergeInGroupId(1, 2); } @Test