diff --git a/docs/RouteRequest.md b/docs/RouteRequest.md index c3e2da2a1ca..f9a7c12dabd 100644 --- a/docs/RouteRequest.md +++ b/docs/RouteRequest.md @@ -58,7 +58,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe | numItineraries | `integer` | The maximum number of itineraries to return. | *Optional* | `50` | 2.0 | | [optimize](#rd_optimize) | `enum` | The set of characteristics that the user wants to optimize for. | *Optional* | `"safe"` | 2.0 | | [otherThanPreferredRoutesPenalty](#rd_otherThanPreferredRoutesPenalty) | `integer` | Penalty added for using every route that is not preferred if user set any route as preferred. | *Optional* | `300` | 2.0 | -| [relaxTransitPriorityGroup](#rd_relaxTransitPriorityGroup) | `string` | The relax function for transit-priority-groups | *Optional* | `"0s + 1.00 t"` | 2.5 | +| [relaxTransitGroupPriority](#rd_relaxTransitGroupPriority) | `string` | The relax function for transit-group-priority | *Optional* | `"0s + 1.00 t"` | 2.5 | | [relaxTransitSearchGeneralizedCostAtDestination](#rd_relaxTransitSearchGeneralizedCostAtDestination) | `double` | Whether non-optimal transit paths at the destination should be returned | *Optional* | | 2.3 | | [searchWindow](#rd_searchWindow) | `duration` | The duration of the search-window. | *Optional* | | 2.0 | | stairsReluctance | `double` | Used instead of walkReluctance for stairs. | *Optional* | `2.0` | 2.0 | @@ -109,7 +109,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe |    [extraStopBoardAlightCostsFactor](#rd_to_extraStopBoardAlightCostsFactor) | `double` | Add an extra board- and alight-cost for prioritized stops. | *Optional* | `0.0` | 2.1 | |    [minSafeWaitTimeFactor](#rd_to_minSafeWaitTimeFactor) | `double` | Used to set a maximum wait-time cost, base on min-safe-transfer-time. | *Optional* | `5.0` | 2.1 | |    [optimizeTransferWaitTime](#rd_to_optimizeTransferWaitTime) | `boolean` | This enables the transfer wait time optimization. | *Optional* | `true` | 2.1 | -| [transitPriorityGroups](#rd_transitPriorityGroups) | `object` | Transit priority groups configuration | *Optional* | | 2.5 | +| [transitGroupPriority](#rd_transitGroupPriority) | `object` | Group transit patterns and give each group a mutual advantage in the Raptor search. | *Optional* | | 2.5 | | [transitReluctanceForMode](#rd_transitReluctanceForMode) | `enum map of double` | Transit reluctance for a given transport mode | *Optional* | | 2.1 | | [unpreferred](#rd_unpreferred) | `object` | Parameters listing authorities or lines that preferably should not be used in trip patters. | *Optional* | | 2.2 | |    [agencies](#rd_unpreferred_agencies) | `feed-scoped-id[]` | The ids of the agencies that incur an extra cost when being used. Format: `FeedId:AgencyId` | *Optional* | | 2.2 | @@ -248,16 +248,16 @@ Penalty added for using every route that is not preferred if user set any route We return number of seconds that we are willing to wait for preferred route. -

relaxTransitPriorityGroup

+

relaxTransitGroupPriority

**Since version:** `2.5` ∙ **Type:** `string` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"0s + 1.00 t"` **Path:** /routingDefaults -The relax function for transit-priority-groups +The relax function for transit-group-priority -A path is considered optimal if the generalized-cost is less than the -generalized-cost of another path. If this parameter is set, the comparison is relaxed -further if they belong to different transit-priority-groups. +A path is considered optimal if the generalized-cost is less than the generalized-cost of +another path. If this parameter is set, the comparison is relaxed further if they belong +to different transit groups.

relaxTransitSearchGeneralizedCostAtDestination

@@ -813,22 +813,20 @@ This enables the transfer wait time optimization. If not enabled generalizedCost function is used to pick the optimal transfer point. -

transitPriorityGroups

+

transitGroupPriority

**Since version:** `2.5` ∙ **Type:** `object` ∙ **Cardinality:** `Optional` **Path:** /routingDefaults -Transit priority groups configuration +Group transit patterns and give each group a mutual advantage in the Raptor search. Use this to separate transit patterns into groups. Each group will be given a group-id. A path (multiple legs) will then have a set of group-ids based on the group-id from each leg. Hence, two paths with a different set of group-ids will BOTH be optimal unless the cost is -worse than the relaxation specified in the `relaxTransitPriorityGroup` parameter. This is +worse than the relaxation specified in the `relaxTransitGroupPriority` parameter. This is only available in the TransmodelAPI for now. -Unmatched patterns are put in the BASE priority-group (group id: 0). This group is special. -If a path only have legs in the base group, then that path dominates other paths, but other -paths must be better to make it. +Unmatched patterns are put in the BASE priority-group. **THIS IS STILL AN EXPERIMENTAL FEATURE - IT MAY CHANGE WITHOUT ANY NOTICE!** diff --git a/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturHackSorlandsBanen.java b/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturHackSorlandsBanen.java index d006c99c585..492b950a1fa 100644 --- a/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturHackSorlandsBanen.java +++ b/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturHackSorlandsBanen.java @@ -58,7 +58,7 @@ public static RaptorRequest enableHack( return raptorRequest; } - if (!request.preferences().transit().relaxTransitPriorityGroup().isNormal()) { + if (!request.preferences().transit().relaxTransitGroupPriority().isNormal()) { return raptorRequest; } diff --git a/src/main/java/org/opentripplanner/api/common/RequestToPreferencesMapper.java b/src/main/java/org/opentripplanner/api/common/RequestToPreferencesMapper.java index 20dbc551885..3561dd3ec08 100644 --- a/src/main/java/org/opentripplanner/api/common/RequestToPreferencesMapper.java +++ b/src/main/java/org/opentripplanner/api/common/RequestToPreferencesMapper.java @@ -95,10 +95,8 @@ private BoardAndAlightSlack mapTransit() { setIfNotNull(req.otherThanPreferredRoutesPenalty, tr::setOtherThanPreferredRoutesPenalty); setIfNotNull(req.ignoreRealtimeUpdates, tr::setIgnoreRealtimeUpdates); - if (req.relaxTransitPriorityGroup != null) { - tr.withTransitGroupPriorityGeneralizedCostSlack( - CostLinearFunction.of(req.relaxTransitPriorityGroup) - ); + if (req.relaxTransitGroupPriority != null) { + tr.withRelaxTransitGroupPriority(CostLinearFunction.of(req.relaxTransitGroupPriority)); } else { setIfNotNull( req.relaxTransitSearchGeneralizedCostAtDestination, diff --git a/src/main/java/org/opentripplanner/api/common/RoutingResource.java b/src/main/java/org/opentripplanner/api/common/RoutingResource.java index 3a78a3d73d4..02c1fea8537 100644 --- a/src/main/java/org/opentripplanner/api/common/RoutingResource.java +++ b/src/main/java/org/opentripplanner/api/common/RoutingResource.java @@ -661,8 +661,8 @@ public abstract class RoutingResource { @QueryParam("useVehicleParkingAvailabilityInformation") protected Boolean useVehicleParkingAvailabilityInformation; - @QueryParam("relaxTransitPriorityGroup") - protected String relaxTransitPriorityGroup; + @QueryParam("relaxTransitGroupPriority") + protected String relaxTransitGroupPriority; /** * Whether non-optimal transit paths at the destination should be returned. diff --git a/src/main/java/org/opentripplanner/apis/transmodel/mapping/preferences/TransitPreferencesMapper.java b/src/main/java/org/opentripplanner/apis/transmodel/mapping/preferences/TransitPreferencesMapper.java index caa8ebf7715..28643bf8199 100644 --- a/src/main/java/org/opentripplanner/apis/transmodel/mapping/preferences/TransitPreferencesMapper.java +++ b/src/main/java/org/opentripplanner/apis/transmodel/mapping/preferences/TransitPreferencesMapper.java @@ -1,8 +1,11 @@ package org.opentripplanner.apis.transmodel.mapping.preferences; import graphql.schema.DataFetchingEnvironment; +import java.util.Map; import org.opentripplanner.apis.transmodel.model.TransportModeSlack; +import org.opentripplanner.apis.transmodel.model.plan.RelaxCostType; import org.opentripplanner.apis.transmodel.support.DataFetcherDecorator; +import org.opentripplanner.routing.api.request.framework.CostLinearFunction; import org.opentripplanner.routing.api.request.preference.TransitPreferences; public class TransitPreferencesMapper { @@ -34,8 +37,11 @@ public static void mapTransitPreferences( callWith.argument("includePlannedCancellations", transit::setIncludePlannedCancellations); callWith.argument("includeRealtimeCancellations", transit::setIncludeRealtimeCancellations); callWith.argument( - "relaxTransitPriorityGroup", - transit::withTransitGroupPriorityGeneralizedCostSlack + "relaxTransitGroupPriority", + it -> + transit.withRelaxTransitGroupPriority( + RelaxCostType.mapToDomain((Map) it, CostLinearFunction.NORMAL) + ) ); callWith.argument( "relaxTransitSearchGeneralizedCostAtDestination", diff --git a/src/main/java/org/opentripplanner/apis/transmodel/model/plan/RelaxCostType.java b/src/main/java/org/opentripplanner/apis/transmodel/model/plan/RelaxCostType.java index 3bd3ed129ef..d3455083695 100644 --- a/src/main/java/org/opentripplanner/apis/transmodel/model/plan/RelaxCostType.java +++ b/src/main/java/org/opentripplanner/apis/transmodel/model/plan/RelaxCostType.java @@ -2,13 +2,15 @@ import graphql.Scalars; import graphql.language.FloatValue; -import graphql.language.IntValue; import graphql.language.ObjectField; import graphql.language.ObjectValue; +import graphql.language.StringValue; import graphql.schema.GraphQLInputObjectField; import graphql.schema.GraphQLInputObjectType; -import graphql.schema.GraphQLList; -import graphql.schema.GraphQLNonNull; +import java.util.Map; +import org.opentripplanner.framework.graphql.scalar.CostScalarFactory; +import org.opentripplanner.framework.model.Cost; +import org.opentripplanner.framework.time.DurationUtils; import org.opentripplanner.routing.api.request.framework.CostLinearFunction; public class RelaxCostType { @@ -26,8 +28,8 @@ public class RelaxCostType { with twice as high cost as another one, is accepted. A `constant=$300` means a "fixed" constant is added to the limit. A `{ratio=1.0, constant=0}` is said to be the NORMAL relaxed cost - the limit is the same as the cost used to calculate the limit. The NORMAL is usually - the default. We can express the RelaxCost as a function `f(x) = constant + ratio * x`. - `f(x)=x` is the NORMAL function. + the default. We can express the RelaxCost as a function `f(t) = constant + ratio * t`. + `f(t)=t` is the NORMAL function. """ ) .field( @@ -44,11 +46,12 @@ public class RelaxCostType { .newInputObjectField() .name(CONSTANT) .description( - "The constant value to add to the limit. Must be a positive number. The unit" + - " is cost-seconds." + "The constant value to add to the limit. Must be a positive number. The value is" + + "equivalent to transit-cost-seconds. Integers is treated as seconds, but you may use " + + "the duration format. Example: '3665 = 'DT1h1m5s' = '1h1m5s'." ) - .defaultValueLiteral(IntValue.of(0)) - .type(new GraphQLList(new GraphQLNonNull(Scalars.GraphQLID))) + .defaultValueProgrammatic("0s") + .type(CostScalarFactory.costScalar()) .build() ) .build(); @@ -63,9 +66,31 @@ public static ObjectValue valueOf(CostLinearFunction value) { ObjectField .newObjectField() .name(CONSTANT) - .value(IntValue.of(value.constant().toSeconds())) + // We only use this to display default value (this is an input type), so using the + // lenient OTP version of duration is ok - it is slightly more readable. + .value(StringValue.of(DurationUtils.durationToStr(value.constant().asDuration()))) .build() ) .build(); } + + public static CostLinearFunction mapToDomain( + Map input, + CostLinearFunction defaultValue + ) { + if (input == null || input.isEmpty()) { + return defaultValue; + } + + double ratio = 1.0; + Cost constant = Cost.ZERO; + + if (input.containsKey(RATIO)) { + ratio = (Double) input.get(RATIO); + } + if (input.containsKey(CONSTANT)) { + constant = (Cost) input.get(CONSTANT); + } + return CostLinearFunction.of(constant, ratio); + } } diff --git a/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripPatternType.java b/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripPatternType.java index 31e33ae6e4d..158d6a9a042 100644 --- a/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripPatternType.java +++ b/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripPatternType.java @@ -193,6 +193,18 @@ public static GraphQLObjectType create( .dataFetcher(env -> itinerary(env).getGeneralizedCost()) .build() ) + .field( + GraphQLFieldDefinition + .newFieldDefinition() + .name("generalizedCost2") + .description( + "A second cost or weight of the itinerary. Some use-cases like pass-through " + + "and transit-priority-groups use a second cost during routing. This is used for debugging." + ) + .type(Scalars.GraphQLInt) + .dataFetcher(env -> itinerary(env).getGeneralizedCost2().orElse(null)) + .build() + ) .field( GraphQLFieldDefinition .newFieldDefinition() diff --git a/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripQuery.java b/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripQuery.java index e5864057669..7727fc3555f 100644 --- a/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripQuery.java +++ b/src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripQuery.java @@ -277,30 +277,30 @@ public static GraphQLFieldDefinition create( .argument( GraphQLArgument .newArgument() - .name("relaxTransitPriorityGroup") + .name("relaxTransitGroupPriority") .description( """ Relax generalized-cost when comparing trips with a different set of - transit-priority-groups. The groups are set server side for service-journey and + transit-group-priorities. The groups are set server side for service-journey and can not be configured in the API. This mainly helps to return competition neutral - services. Long distance authorities are put in different transit-priority-groups. + services. Long distance authorities are put in different transit-groups. This relaxes the comparison inside the routing engine for each stop-arrival. If two - paths have a different set of transit-priority-groups, then the generalized-cost + paths have a different set of transit-group-priorities, then the generalized-cost comparison is relaxed. The final set of paths are filtered through the normal itinerary-filters. - The `ratio` must be greater or equal to 1.0 and less then 1.2. - - The `slack` must be greater or equal to 0 and less then 3600. + - The `constant` must be greater or equal to '0s' and less then '1h'. THIS IS STILL AN EXPERIMENTAL FEATURE - IT MAY CHANGE WITHOUT ANY NOTICE! """.stripIndent() ) .type(RelaxCostType.INPUT_TYPE) .defaultValueLiteral( - preferences.transit().relaxTransitPriorityGroup().isNormal() + preferences.transit().relaxTransitGroupPriority().isNormal() ? NullValue.of() - : RelaxCostType.valueOf(preferences.transit().relaxTransitPriorityGroup()) + : RelaxCostType.valueOf(preferences.transit().relaxTransitGroupPriority()) ) .build() ) @@ -523,7 +523,7 @@ public static GraphQLFieldDefinition create( GraphQLArgument .newArgument() .name("relaxTransitSearchGeneralizedCostAtDestination") - .deprecate("This is replaced by 'relaxTransitPriorityGroup'.") + .deprecate("This is replaced by 'relaxTransitGroupPriority'.") .description( """ Whether non-optimal transit paths at the destination should be returned. Let c be the diff --git a/src/main/java/org/opentripplanner/framework/graphql/scalar/CostScalarFactory.java b/src/main/java/org/opentripplanner/framework/graphql/scalar/CostScalarFactory.java new file mode 100644 index 00000000000..6f710328174 --- /dev/null +++ b/src/main/java/org/opentripplanner/framework/graphql/scalar/CostScalarFactory.java @@ -0,0 +1,83 @@ +package org.opentripplanner.framework.graphql.scalar; + +import graphql.GraphQLContext; +import graphql.execution.CoercedVariables; +import graphql.language.StringValue; +import graphql.language.Value; +import graphql.schema.Coercing; +import graphql.schema.CoercingParseLiteralException; +import graphql.schema.CoercingParseValueException; +import graphql.schema.GraphQLScalarType; +import java.util.Locale; +import java.util.NoSuchElementException; +import javax.annotation.Nonnull; +import org.opentripplanner.framework.model.Cost; +import org.opentripplanner.framework.time.DurationUtils; + +public class CostScalarFactory { + + private static final String TYPENAME = "Cost"; + + private static final String DOCUMENTATION = + "A cost value, normally a value of 1 is equivalent to riding transit for 1 second, " + + "but it might not depending on the use-case. Format: 3665 = DT1h1m5s = 1h1m5s"; + + private static final GraphQLScalarType SCALAR_INSTANCE = createCostScalar(); + + private CostScalarFactory() {} + + public static GraphQLScalarType costScalar() { + return SCALAR_INSTANCE; + } + + private static GraphQLScalarType createCostScalar() { + return GraphQLScalarType + .newScalar() + .name(TYPENAME) + .description(DOCUMENTATION) + .coercing(createCoercing()) + .build(); + } + + private static String serializeCost(Cost cost) { + return cost.asDuration().toString(); + } + + private static Cost parseCost(String input) throws CoercingParseValueException { + try { + return Cost.fromDuration(DurationUtils.parseSecondsOrDuration(input).orElseThrow()); + } catch (IllegalArgumentException | NoSuchElementException e) { + throw new CoercingParseValueException(e.getMessage(), e); + } + } + + private static Coercing createCoercing() { + return new Coercing<>() { + @Override + public String serialize(@Nonnull Object result, GraphQLContext c, Locale l) { + return serializeCost((Cost) result); + } + + @Override + public Cost parseValue(Object input, GraphQLContext c, Locale l) + throws CoercingParseValueException { + return parseCost((String) input); + } + + @Override + public Cost parseLiteral(Value input, CoercedVariables v, GraphQLContext c, Locale l) + throws CoercingParseLiteralException { + if (input instanceof StringValue stringValue) { + return parseCost(stringValue.getValue()); + } + return null; + } + + @Override + @Nonnull + public Value valueToLiteral(Object input, GraphQLContext c, Locale l) { + return StringValue.of((String) input); + } + }; + } +} diff --git a/src/main/java/org/opentripplanner/model/plan/Itinerary.java b/src/main/java/org/opentripplanner/model/plan/Itinerary.java index 58320bf1652..d1252c45f2d 100644 --- a/src/main/java/org/opentripplanner/model/plan/Itinerary.java +++ b/src/main/java/org/opentripplanner/model/plan/Itinerary.java @@ -17,6 +17,7 @@ import org.opentripplanner.framework.tostring.ToStringBuilder; import org.opentripplanner.model.SystemNotice; import org.opentripplanner.model.fare.ItineraryFares; +import org.opentripplanner.raptor.api.model.RaptorConstants; import org.opentripplanner.raptor.api.path.PathStringBuilder; import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter; import org.opentripplanner.routing.api.request.RouteRequest; @@ -43,6 +44,7 @@ public class Itinerary implements ItinerarySortKey { private Double elevationLost = 0.0; private Double elevationGained = 0.0; private int generalizedCost = UNKNOWN; + private Integer generalizedCost2 = null; private TimeAndCost accessPenalty = null; private TimeAndCost egressPenalty = null; private int waitTimeOptimizedCost = UNKNOWN; @@ -260,6 +262,7 @@ public String toString() { .addDuration("transitTime", transitDuration) .addDuration("waitingTime", waitingDuration) .addNum("generalizedCost", generalizedCost, UNKNOWN) + .addNum("generalizedCost2", generalizedCost2) .addNum("waitTimeOptimizedCost", waitTimeOptimizedCost, UNKNOWN) .addNum("transferPriorityCost", transferPriorityCost, UNKNOWN) .addNum("nonTransitDistance", nonTransitDistanceMeters, "m") @@ -306,7 +309,12 @@ public String toStr() { buf.stop(leg.getTo().name.toString()); } - buf.summary(RaptorCostConverter.toRaptorCost(generalizedCost)); + // The generalizedCost2 is printed as is, it is a special cost and the scale depends on the + // use-case. + buf.summary( + RaptorCostConverter.toRaptorCost(generalizedCost), + getGeneralizedCost2().orElse(RaptorConstants.NOT_SET) + ); return buf.toString(); } @@ -495,6 +503,24 @@ public void setGeneralizedCost(int generalizedCost) { this.generalizedCost = generalizedCost; } + /** + * The transit router allows the usage of a second generalized-cost parameter to be used in + * routing. In Raptor this is called c2, but in OTP it is generalized-cost-2. What this cost + * represents depends on the use-case and the unit and scale is also given by the use-case. + *

+ * Currently, the pass-through search and the transit-priority uses this. This is relevant for + * anyone who wants to debug a search and tune the system. + *

+ * {@link RaptorConstants#NOT_SET} indicate that the cost is not set/computed. + */ + public Optional getGeneralizedCost2() { + return Optional.ofNullable(generalizedCost2); + } + + public void setGeneralizedCost2(Integer generalizedCost2) { + this.generalizedCost2 = generalizedCost2; + } + @Nullable public TimeAndCost getAccessPenalty() { return accessPenalty; diff --git a/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripPattern.java b/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripPattern.java index 98a2533486b..e5be1cab0d5 100644 --- a/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripPattern.java +++ b/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripPattern.java @@ -46,7 +46,7 @@ public interface RaptorTripPattern { int slackIndex(); /** - * A pattern may belong to a transit-priority-group. Each group is given an advantage during + * A pattern may belong to a transit-group-priority. Each group is given an advantage during * the multi-criteria search, so the best alternative for each group is found. */ int priorityGroupId(); diff --git a/src/main/java/org/opentripplanner/raptor/api/path/PathStringBuilder.java b/src/main/java/org/opentripplanner/raptor/api/path/PathStringBuilder.java index 4cf13e362fd..b96d1a96f14 100644 --- a/src/main/java/org/opentripplanner/raptor/api/path/PathStringBuilder.java +++ b/src/main/java/org/opentripplanner/raptor/api/path/PathStringBuilder.java @@ -119,8 +119,8 @@ public PathStringBuilder numberOfTransfers(int nTransfers) { : this; } - public PathStringBuilder summary(int c1) { - return summaryStart().c1(c1).summaryEnd(); + public PathStringBuilder summary(int c1, int c2) { + return summaryStart().c1(c1).c2(c2).summaryEnd(); } public PathStringBuilder summary(int startTime, int endTime, int nTransfers, int c1, int c2) { diff --git a/src/main/java/org/opentripplanner/raptor/api/path/RaptorPath.java b/src/main/java/org/opentripplanner/raptor/api/path/RaptorPath.java index 36caaf856e8..ca67599e262 100644 --- a/src/main/java/org/opentripplanner/raptor/api/path/RaptorPath.java +++ b/src/main/java/org/opentripplanner/raptor/api/path/RaptorPath.java @@ -3,6 +3,7 @@ import java.util.List; import java.util.stream.Stream; import javax.annotation.Nullable; +import org.opentripplanner.raptor.api.model.RaptorConstants; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; import org.opentripplanner.raptor.api.model.RelaxFunction; @@ -56,8 +57,15 @@ public interface RaptorPath extends Comparable { private final RelaxFunction relaxC1; @Nullable - private final RaptorTransitPriorityGroupCalculator transitPriorityCalculator; + private final RaptorTransitGroupCalculator transitPriorityCalculator; private final List passThroughPoints; @@ -64,15 +63,7 @@ public RelaxFunction relaxC1() { return relaxC1; } - @Deprecated - @Nullable - public RelaxFunction relaxC1AtDestination() { - return relaxC1.isNormal() - ? relaxCostAtDestination().map(GeneralizedCostRelaxFunction::of).orElse(RelaxFunction.NORMAL) - : relaxC1; - } - - public Optional transitPriorityCalculator() { + public Optional transitPriorityCalculator() { return Optional.ofNullable(transitPriorityCalculator); } @@ -85,8 +76,8 @@ public List passThroughPoints() { } /** - * Whether to accept non-optimal trips if they are close enough - if and only if they represent an - * optimal path for their given iteration. In other words this slack only relaxes the pareto + * Whether to accept non-optimal trips if they are close enough - if and only if they represent + * an optimal path for their given iteration. In other words this slack only relaxes the pareto * comparison at the destination. *

* Let {@code c} be the existing minimum pareto optimal cost to beat. Then a trip with cost @@ -102,8 +93,9 @@ public List passThroughPoints() { * is replaced by {@link #relaxC1()}. This parameter is ignored if {@link #relaxC1()} exist. */ @Deprecated - public Optional relaxCostAtDestination() { - return Optional.ofNullable(relaxCostAtDestination); + @Nullable + public Double relaxCostAtDestination() { + return relaxCostAtDestination; } @Override @@ -148,7 +140,7 @@ public static class Builder { private final MultiCriteriaRequest original; private RelaxFunction relaxC1; - private RaptorTransitPriorityGroupCalculator transitPriorityCalculator; + private RaptorTransitGroupCalculator transitPriorityCalculator; private List passThroughPoints; private Double relaxCostAtDestination; @@ -171,11 +163,11 @@ public Builder withRelaxC1(RelaxFunction relaxC1) { } @Nullable - public RaptorTransitPriorityGroupCalculator transitPriorityCalculator() { + public RaptorTransitGroupCalculator transitPriorityCalculator() { return transitPriorityCalculator; } - public Builder withTransitPriorityCalculator(RaptorTransitPriorityGroupCalculator value) { + public Builder withTransitPriorityCalculator(RaptorTransitGroupCalculator value) { transitPriorityCalculator = value; return this; } diff --git a/src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitPriorityGroupCalculator.java b/src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitGroupCalculator.java similarity index 71% rename from src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitPriorityGroupCalculator.java rename to src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitGroupCalculator.java index c3890fa47b3..b5f0598415e 100644 --- a/src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitPriorityGroupCalculator.java +++ b/src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitGroupCalculator.java @@ -2,19 +2,19 @@ import org.opentripplanner.raptor.api.model.DominanceFunction; -public interface RaptorTransitPriorityGroupCalculator { +public interface RaptorTransitGroupCalculator { /** - * Merge in the trip transit priority group id with an existing set. Note! Both the set + * Merge in the transit group id with an existing set. Note! Both the set * and the group id type is {@code int}. * * @param currentGroupIds the set of groupIds for all legs in a path. * @param boardingGroupId the transit group id to add to the given set. * @return the new computed set of groupIds */ - int mergeTransitPriorityGroupIds(int currentGroupIds, int boardingGroupId); + int mergeGroupIds(int currentGroupIds, int boardingGroupId); /** - * This is the dominance function to use for comparing transit-priority-groupIds. + * This is the dominance function to use for comparing transit-groups. * It is critical that the implementation is "static" so it can be inlined, since it * is run in the innermost loop of Raptor. */ diff --git a/src/main/java/org/opentripplanner/raptor/path/PathBuilder.java b/src/main/java/org/opentripplanner/raptor/path/PathBuilder.java index c046a8f240d..7612cc0b3ba 100644 --- a/src/main/java/org/opentripplanner/raptor/path/PathBuilder.java +++ b/src/main/java/org/opentripplanner/raptor/path/PathBuilder.java @@ -182,6 +182,10 @@ public int c2() { return tail.isC2Set() ? tail.c2() : c2; } + public boolean isC2Set() { + return tail.isC2Set() || c2 != RaptorConstants.NOT_SET; + } + public RaptorPath build() { updateAggregatedFields(); var pathLegs = createPathLegs(costCalculator, slackProvider); diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java index 8c35f103106..060d3a2e018 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/SystemErrDebugLogger.java @@ -219,7 +219,7 @@ private void print(PatternRideView p, String action) { } private String path(ArrivalView a) { - return path(a, new PathStringBuilder(null)).summary(a.c1()).toString(); + return path(a, new PathStringBuilder(null)).summary(a.c1(), a.c2()).toString(); } private PathStringBuilder path(ArrivalView a, PathStringBuilder buf) { diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java index eda2cabc0a1..1706c879a2c 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java @@ -1,5 +1,9 @@ package org.opentripplanner.raptor.rangeraptor.context; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_ARRIVAL_TIME; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_DEPARTURE_TIME; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_TIMETABLE; + import java.util.Collection; import java.util.HashMap; import java.util.List; @@ -20,6 +24,7 @@ import org.opentripplanner.raptor.api.request.RaptorTuningParameters; import org.opentripplanner.raptor.api.request.SearchParams; import org.opentripplanner.raptor.rangeraptor.debug.DebugHandlerFactory; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime; import org.opentripplanner.raptor.rangeraptor.internalapi.RoundProvider; import org.opentripplanner.raptor.rangeraptor.internalapi.SlackProvider; import org.opentripplanner.raptor.rangeraptor.internalapi.WorkerLifeCycle; @@ -218,6 +223,13 @@ public TimeBasedBoardingSupport createTimeBasedBoardingSupport() { ); } + /** + * Resolve which pareto-set time config to use. + */ + public ParetoSetTime paretoSetTimeConfig() { + return paretoSetTimeConfig(searchParams(), searchDirection()); + } + /** * The multi-criteria state can handle multiple access/egress paths to a single stop, but the * Standard and BestTime states do not. To get a deterministic behaviour we filter the paths and @@ -307,4 +319,17 @@ private static EgressPaths egressPaths(RaptorRequest request) { var paths = forward ? params.egressPaths() : params.accessPaths(); return EgressPaths.create(paths, request.profile()); } + + static ParetoSetTime paretoSetTimeConfig( + SearchParams searchParams, + SearchDirection searchDirection + ) { + if (searchParams.timetable()) { + return USE_TIMETABLE; + } + boolean preferLatestDeparture = + searchParams.preferLateArrival() != searchDirection.isInReverse(); + + return preferLatestDeparture ? USE_DEPARTURE_TIME : USE_ARRIVAL_TIME; + } } diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/ParetoSetCost.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/ParetoSetCost.java new file mode 100644 index 00000000000..7ae3706bb4e --- /dev/null +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/ParetoSetCost.java @@ -0,0 +1,48 @@ +package org.opentripplanner.raptor.rangeraptor.internalapi; + +/** + * These are the different cost configuration Raptor support. Each configuration will + * be used to change the pareto-function used to compare arrivals and paths. We add + * new values here when needed by a new use-case. + */ +public enum ParetoSetCost { + /** + * Cost is not used. + */ + NONE, + /** + * One cost parameter is used. A small c1 value is better than a large value. + */ + USE_C1, + /** + * Same as {@link #USE_C1}, but the relax function is used to relax the cost at the destination. + * DO not use this! This will be removed as soon as the Vy, Entur, Norway has migrated off + * this feature. + */ + @Deprecated + USE_C1_RELAX_DESTINATION, + /** + * Use both c1 and c2 in the pareto function. A small value is better than a large one. + */ + USE_C1_AND_C2, + /** + * Use c1 in the pareto function, but relax c1 is c2 is optimal. This allows slightly worse + * c1 values if a path is considered better based on the c2 value. Another way of looking at + * this, is that all paths are grouped by the c2 value. When two paths are compared inside a group + * the normal c1 comparison is used, and when comparing paths from different groups the relaxed + * c1 comparison is used. + */ + USE_C1_RELAXED_IF_C2_IS_OPTIMAL; + + public boolean includeC1() { + return this != NONE; + } + + /** + * Use c2 as input to the pareto function. The c2 value is used as a criteria, or it is used + * to modify the function ({@link #USE_C1_RELAXED_IF_C2_IS_OPTIMAL}). + */ + public boolean includeC2() { + return this == USE_C1_AND_C2 || this == USE_C1_RELAXED_IF_C2_IS_OPTIMAL; + } +} diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/ParetoSetTime.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/ParetoSetTime.java new file mode 100644 index 00000000000..c0c6f7f08e1 --- /dev/null +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/ParetoSetTime.java @@ -0,0 +1,21 @@ +package org.opentripplanner.raptor.rangeraptor.internalapi; + +/** + * These are the different time configurations Raptor supports. Each configuration will + * be used to change the pareto-function. + */ +public enum ParetoSetTime { + /** + * Uses iteration-departure-time and arrival-time as criteria in pareto function. Note! + * iteration-departure-time is slightly different from the more precise departure-time. + */ + USE_TIMETABLE, + /** + * Uses arrival-time as criteria in pareto function. + */ + USE_ARRIVAL_TIME, + /** + * Uses departure-time as criteria in pareto function. + */ + USE_DEPARTURE_TIME, +} diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/PassThroughPointsService.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/PassThroughPointsService.java index 69899b55688..4932d9c46fe 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/PassThroughPointsService.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/PassThroughPointsService.java @@ -54,7 +54,7 @@ default boolean isNoop() { void updateC2Value(int currentPathC2, IntConsumer update); /** - * This is the dominance function to use for comparing transit-priority-groupIds. + * This is the dominance function to use for comparing transit-group-priorityIds. * It is critical that the implementation is "static" so it can be inlined, since it * is run in the innermost loop of Raptor. */ diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/configure/McRangeRaptorConfig.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/configure/McRangeRaptorConfig.java index 649737bc42f..8eef90950dd 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/configure/McRangeRaptorConfig.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/configure/McRangeRaptorConfig.java @@ -6,9 +6,10 @@ import org.opentripplanner.raptor.api.model.DominanceFunction; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; import org.opentripplanner.raptor.api.request.MultiCriteriaRequest; -import org.opentripplanner.raptor.api.request.RaptorTransitPriorityGroupCalculator; +import org.opentripplanner.raptor.api.request.RaptorTransitGroupCalculator; import org.opentripplanner.raptor.rangeraptor.context.SearchContext; import org.opentripplanner.raptor.rangeraptor.internalapi.Heuristics; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost; import org.opentripplanner.raptor.rangeraptor.internalapi.PassThroughPointsService; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorWorker; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorWorkerState; @@ -28,7 +29,7 @@ import org.opentripplanner.raptor.rangeraptor.multicriteria.ride.c1.PatternRideC1; import org.opentripplanner.raptor.rangeraptor.multicriteria.ride.c2.PassThroughRideFactory; import org.opentripplanner.raptor.rangeraptor.multicriteria.ride.c2.PatternRideC2; -import org.opentripplanner.raptor.rangeraptor.multicriteria.ride.c2.TransitPriorityGroupRideFactory; +import org.opentripplanner.raptor.rangeraptor.multicriteria.ride.c2.TransitGroupPriorityRideFactory; import org.opentripplanner.raptor.rangeraptor.path.DestinationArrivalPaths; import org.opentripplanner.raptor.rangeraptor.path.configure.PathConfig; import org.opentripplanner.raptor.util.paretoset.ParetoComparator; @@ -157,7 +158,8 @@ private > ParetoSet createPatternRideParetoSet( private DestinationArrivalPaths createDestinationArrivalPaths() { if (paths == null) { - paths = pathConfig.createDestArrivalPaths(true, includeC2() ? dominanceFunctionC2() : null); + var c2Comp = includeC2() ? dominanceFunctionC2() : null; + paths = pathConfig.createDestArrivalPaths(resolveCostConfig(), c2Comp); } return paths; } @@ -171,7 +173,8 @@ private MultiCriteriaRequest mcRequest() { } /** - * Currently "transit-priority-groups" is the only feature using two multi-criteria(c2). + * Use c2 in the search, this is use-case specific. For example the pass-through or + * transit-group-priority features uses the c2 value. */ private boolean includeC2() { return mcRequest().includeC2(); @@ -182,7 +185,7 @@ private PatternRideFactory> createPatternRideC2Factory() { return new PassThroughRideFactory<>(passThroughPointsService); } if (isTransitPriority()) { - return new TransitPriorityGroupRideFactory<>(getTransitPriorityGroupCalculator()); + return new TransitGroupPriorityRideFactory<>(getTransitGroupPriorityCalculator()); } throw new IllegalStateException("Only pass-through and transit-priority uses c2."); } @@ -193,12 +196,12 @@ private DominanceFunction dominanceFunctionC2() { return passThroughPointsService.dominanceFunction(); } if (isTransitPriority()) { - return getTransitPriorityGroupCalculator().dominanceFunction(); + return getTransitGroupPriorityCalculator().dominanceFunction(); } return null; } - private RaptorTransitPriorityGroupCalculator getTransitPriorityGroupCalculator() { + private RaptorTransitGroupCalculator getTransitGroupPriorityCalculator() { return mcRequest().transitPriorityCalculator().orElseThrow(); } @@ -209,4 +212,17 @@ private boolean isPassThrough() { private boolean isTransitPriority() { return mcRequest().transitPriorityCalculator().isPresent(); } + + private ParetoSetCost resolveCostConfig() { + if (isTransitPriority()) { + return ParetoSetCost.USE_C1_RELAXED_IF_C2_IS_OPTIMAL; + } + if (isPassThrough()) { + return ParetoSetCost.USE_C1_AND_C2; + } + if (context.multiCriteria().relaxCostAtDestination() != null) { + return ParetoSetCost.USE_C1_RELAX_DESTINATION; + } + return ParetoSetCost.USE_C1; + } } diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitPriorityGroupRideFactory.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitGroupPriorityRideFactory.java similarity index 67% rename from src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitPriorityGroupRideFactory.java rename to src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitGroupPriorityRideFactory.java index eca049233b9..5d65c40d021 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitPriorityGroupRideFactory.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/c2/TransitGroupPriorityRideFactory.java @@ -2,25 +2,25 @@ import org.opentripplanner.raptor.api.model.RaptorTripPattern; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; -import org.opentripplanner.raptor.api.request.RaptorTransitPriorityGroupCalculator; +import org.opentripplanner.raptor.api.request.RaptorTransitGroupCalculator; import org.opentripplanner.raptor.rangeraptor.multicriteria.arrivals.McStopArrival; import org.opentripplanner.raptor.rangeraptor.multicriteria.ride.PatternRide; import org.opentripplanner.raptor.rangeraptor.multicriteria.ride.PatternRideFactory; /** - * This factory creates new {@link PatternRide}s and merge in transit-priority-group ids + * This factory creates new {@link PatternRide}s and merge in transit-group-priority ids * into c2. */ -public class TransitPriorityGroupRideFactory +public class TransitGroupPriorityRideFactory implements PatternRideFactory> { private int currentPatternGroupPriority; - private final RaptorTransitPriorityGroupCalculator transitPriorityGroupCalculator; + private final RaptorTransitGroupCalculator transitGroupPriorityCalculator; - public TransitPriorityGroupRideFactory( - RaptorTransitPriorityGroupCalculator transitPriorityGroupCalculator + public TransitGroupPriorityRideFactory( + RaptorTransitGroupCalculator transitGroupPriorityCalculator ) { - this.transitPriorityGroupCalculator = transitPriorityGroupCalculator; + this.transitGroupPriorityCalculator = transitGroupPriorityCalculator; } @Override @@ -52,12 +52,9 @@ public void prepareForTransitWith(RaptorTripPattern pattern) { } /** - * Currently transit-priority-group is the only usage of c2 + * Currently transit-group-priority is the only usage of c2 */ private int calculateC2(int c2) { - return transitPriorityGroupCalculator.mergeTransitPriorityGroupIds( - c2, - currentPatternGroupPriority - ); + return transitGroupPriorityCalculator.mergeGroupIds(c2, currentPatternGroupPriority); } } diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparators.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparators.java index 220e014b836..00ea6d11fe5 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparators.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparators.java @@ -7,12 +7,15 @@ import static org.opentripplanner.raptor.api.path.RaptorPath.compareIterationDepartureTime; import static org.opentripplanner.raptor.api.path.RaptorPath.compareNumberOfTransfers; +import java.util.Objects; import javax.annotation.Nonnull; import org.opentripplanner.raptor.api.model.DominanceFunction; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; import org.opentripplanner.raptor.api.model.RelaxFunction; import org.opentripplanner.raptor.api.model.SearchDirection; import org.opentripplanner.raptor.api.path.RaptorPath; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime; import org.opentripplanner.raptor.util.paretoset.ParetoComparator; /** @@ -25,6 +28,14 @@ *

  • Number of transfers
  • *
  • Total travel duration time
  • * + * Optional features are : + *
      + *
    • Prefer late arrival - arriveBy search
    • + *
    • Include c1 - include c1 in pareto function (generalized-cost).
    • + *
    • Include c2 - include c2 in pareto function (custom criteria).
    • + *
    • Relax c1 - accept c1 values which is slightly worse than the best result.
    • + *
    • Relax c1, if c2 is optimal
    • + *
    * The {@code travelDuration} is added as a criteria to the pareto comparator in addition to the * parameters used for each stop arrivals. The {@code travelDuration} is only needed at the * destination, because Range Raptor works in iterations backwards in time. @@ -36,98 +47,52 @@ private PathParetoSetComparators() {} /** * Create pareto-set comparison function. - * @param includeC1 Whether to include generalized cost as a criteria. - * @param includeTimetable // TODO: 2023-07-31 What is this parameter doing exactly? - * @param preferLateArrival // TODO: 2023-07-31 What is this parameter doing exactly? - * @param relaxC1 Relax function for the generalized cost - * @param c2Comp Dominance function for accumulated criteria TWO. If function is null, C2 will - * not be included in the comparison. + * + * @param timeConfig Which time information (arrival-time, departure-time, or timetable) to include in comparator. + * @param costConfig Supported configurations of c1, c2 and relaxed cost(c1). + * @param relaxC1 Relax function for the generalized cost + * @param c2Comp Dominance function for accumulated criteria TWO. If function is null, + * C2 will not be included in the comparison. */ public static ParetoComparator> paretoComparator( - final boolean includeC1, - final boolean includeTimetable, - final boolean preferLateArrival, - final SearchDirection searchDirection, - final RelaxFunction relaxC1, - final DominanceFunction c2Comp + ParetoSetTime timeConfig, + ParetoSetCost costConfig, + RelaxFunction relaxC1, + DominanceFunction c2Comp ) { - /* - * TODO pass-through: I would like to see if we can refactor this with something like this, and - * still get the same performance: - * - * if(c2Comp == null) { - * return paretoComparator(...); - * } - * else { - * return paretoComparator(...) || c2Comp.leftDominateRight(l.c2(), r.c2()); - * } - */ - boolean includeRelaxedCost = includeC1 && !relaxC1.isNormal(); - boolean preferLatestDeparture = preferLateArrival != searchDirection.isInReverse(); - - if (includeRelaxedCost) { - if (includeTimetable) { - if (c2Comp != null) { - return comparatorTimetableAndRelaxedC1AndC2(relaxC1, c2Comp); - } else { - return comparatorTimetableAndRelaxedC1(relaxC1); - } - } - if (preferLateArrival) { - if (c2Comp != null) { - return comparatorDepartureTimeAndRelaxedC1AndC2(relaxC1, c2Comp); - } else { - return comparatorDepartureTimeAndRelaxedC1(relaxC1); - } - } else { - if (c2Comp != null) { - return comparatorArrivalTimeAndRelaxedC1AndC2(relaxC1, c2Comp); - } else { - return comparatorArrivalTimeAndRelaxedC1(relaxC1); - } - } - } + Objects.requireNonNull(timeConfig); + Objects.requireNonNull(costConfig); - if (includeC1) { - if (includeTimetable) { - if (c2Comp != null) { - return comparatorTimetableAndC1AndC2(c2Comp); - } else { - return comparatorTimetableAndC1(); - } - } - if (preferLatestDeparture) { - if (c2Comp != null) { - return comparatorDepartureTimeAndC1AndC2(c2Comp); - } else { - return comparatorDepartureTimeAndC1(); - } - } - if (c2Comp != null) { - return comparatorWithC1AndC2(c2Comp); - } else { - return comparatorWithC1(); - } - } - - if (includeTimetable) { - if (c2Comp != null) { - return comparatorTimetableAndC2(c2Comp); - } else { - return comparatorTimetable(); - } - } - if (preferLatestDeparture) { - if (c2Comp != null) { - return comparatorStandardDepartureTimeAndC2(c2Comp); - } else { - return comparatorStandardDepartureTime(); - } - } - if (c2Comp != null) { - return comparatorStandardArrivalTimeAndC2(c2Comp); - } - return comparatorStandardArrivalTime(); + return switch (costConfig) { + case NONE -> switch (timeConfig) { + case USE_TIMETABLE -> comparatorTimetable(); + case USE_ARRIVAL_TIME -> comparatorStandardArrivalTime(); + case USE_DEPARTURE_TIME -> comparatorStandardDepartureTime(); + }; + case USE_C1 -> switch (timeConfig) { + case USE_TIMETABLE -> comparatorTimetableAndC1(); + case USE_ARRIVAL_TIME -> comparatorArrivalTimeAndC1(); + case USE_DEPARTURE_TIME -> comparatorDepartureTimeAndC1(); + }; + case USE_C1_AND_C2 -> switch (timeConfig) { + case USE_TIMETABLE -> comparatorTimetableAndC1AndC2(c2Comp); + case USE_ARRIVAL_TIME -> comparatorWithC1AndC2(c2Comp); + case USE_DEPARTURE_TIME -> comparatorDepartureTimeAndC1AndC2(c2Comp); + }; + case USE_C1_RELAXED_IF_C2_IS_OPTIMAL -> switch (timeConfig) { + case USE_TIMETABLE -> comparatorTimetableAndRelaxedC1IfC2IsOptimal(relaxC1, c2Comp); + case USE_ARRIVAL_TIME -> comparatorArrivalTimeAndRelaxedC1IfC2IsOptimal(relaxC1, c2Comp); + case USE_DEPARTURE_TIME -> comparatorDepartureTimeAndRelaxedC1IfC2IsOptimal( + relaxC1, + c2Comp + ); + }; + case USE_C1_RELAX_DESTINATION -> switch (timeConfig) { + case USE_TIMETABLE -> comparatorTimetableAndRelaxedC1(relaxC1); + case USE_ARRIVAL_TIME -> comparatorArrivalTimeAndRelaxedC1(relaxC1); + case USE_DEPARTURE_TIME -> comparatorDepartureTimeAndRelaxedC1(relaxC1); + }; + }; } private static < @@ -136,34 +101,12 @@ > ParetoComparator> comparatorStandardArrivalTime() { return (l, r) -> compareArrivalTime(l, r) || compareNumberOfTransfers(l, r); } - private static < - T extends RaptorTripSchedule - > ParetoComparator> comparatorStandardArrivalTimeAndC2( - @Nonnull final DominanceFunction c2Comp - ) { - return (l, r) -> - compareArrivalTime(l, r) || - compareNumberOfTransfers(l, r) || - c2Comp.leftDominateRight(l.c2(), r.c2()); - } - private static < T extends RaptorTripSchedule > ParetoComparator> comparatorStandardDepartureTime() { return (l, r) -> compareDepartureTime(l, r) || compareNumberOfTransfers(l, r); } - private static < - T extends RaptorTripSchedule - > ParetoComparator> comparatorStandardDepartureTimeAndC2( - @Nonnull final DominanceFunction c2Comp - ) { - return (l, r) -> - compareDepartureTime(l, r) || - compareNumberOfTransfers(l, r) || - c2Comp.leftDominateRight(l.c2(), r.c2()); - } - private static < T extends RaptorTripSchedule > ParetoComparator> comparatorTimetable() { @@ -173,18 +116,6 @@ > ParetoComparator> comparatorTimetable() { compareNumberOfTransfers(l, r); } - private static < - T extends RaptorTripSchedule - > ParetoComparator> comparatorTimetableAndC2( - @Nonnull final DominanceFunction c2Comp - ) { - return (l, r) -> - compareIterationDepartureTime(l, r) || - compareArrivalTime(l, r) || - compareNumberOfTransfers(l, r) || - c2Comp.leftDominateRight(l.c2(), r.c2()); - } - private static < T extends RaptorTripSchedule > ParetoComparator> comparatorTimetableAndC1() { @@ -209,7 +140,9 @@ > ParetoComparator> comparatorTimetableAndRelaxedC1( compareC1(relaxCost, l, r); } - private static ParetoComparator> comparatorWithC1() { + private static < + T extends RaptorTripSchedule + > ParetoComparator> comparatorArrivalTimeAndC1() { return (l, r) -> compareArrivalTime(l, r) || compareNumberOfTransfers(l, r) || @@ -230,7 +163,7 @@ > ParetoComparator> comparatorDepartureTimeAndC1() { private static < T extends RaptorTripSchedule > ParetoComparator> comparatorArrivalTimeAndRelaxedC1( - @Nonnull final RelaxFunction relaxCost + @Nonnull RelaxFunction relaxCost ) { return (l, r) -> compareArrivalTime(l, r) || @@ -242,7 +175,7 @@ > ParetoComparator> comparatorArrivalTimeAndRelaxedC1( private static < T extends RaptorTripSchedule > ParetoComparator> comparatorDepartureTimeAndRelaxedC1( - @Nonnull final RelaxFunction relaxCost + @Nonnull RelaxFunction relaxCost ) { return (l, r) -> compareDepartureTime(l, r) || @@ -254,7 +187,7 @@ > ParetoComparator> comparatorDepartureTimeAndRelaxedC1( private static < T extends RaptorTripSchedule > ParetoComparator> comparatorTimetableAndC1AndC2( - @Nonnull final DominanceFunction c2Comp + @Nonnull DominanceFunction c2Comp ) { return (l, r) -> compareIterationDepartureTime(l, r) || @@ -267,22 +200,21 @@ > ParetoComparator> comparatorTimetableAndC1AndC2( private static < T extends RaptorTripSchedule - > ParetoComparator> comparatorTimetableAndRelaxedC1AndC2( - @Nonnull final RelaxFunction relaxCost, - @Nonnull final DominanceFunction c2Comp + > ParetoComparator> comparatorTimetableAndRelaxedC1IfC2IsOptimal( + @Nonnull RelaxFunction relaxCost, + @Nonnull DominanceFunction c2Comp ) { return (l, r) -> compareIterationDepartureTime(l, r) || compareArrivalTime(l, r) || compareNumberOfTransfers(l, r) || compareDuration(l, r) || - compareC1(relaxCost, l, r) || - c2Comp.leftDominateRight(l.c2(), r.c2()); + compareC1RelaxedIfC2IsOptimal(l, r, relaxCost, c2Comp); } private static < T extends RaptorTripSchedule - > ParetoComparator> comparatorWithC1AndC2(@Nonnull final DominanceFunction c2Comp) { + > ParetoComparator> comparatorWithC1AndC2(@Nonnull DominanceFunction c2Comp) { return (l, r) -> compareArrivalTime(l, r) || compareNumberOfTransfers(l, r) || @@ -294,7 +226,7 @@ > ParetoComparator> comparatorWithC1AndC2(@Nonnull final Dominance private static < T extends RaptorTripSchedule > ParetoComparator> comparatorDepartureTimeAndC1AndC2( - @Nonnull final DominanceFunction c2Comp + @Nonnull DominanceFunction c2Comp ) { return (l, r) -> compareDepartureTime(l, r) || @@ -306,29 +238,36 @@ > ParetoComparator> comparatorDepartureTimeAndC1AndC2( private static < T extends RaptorTripSchedule - > ParetoComparator> comparatorArrivalTimeAndRelaxedC1AndC2( - @Nonnull final RelaxFunction relaxCost, - @Nonnull final DominanceFunction c2Comp + > ParetoComparator> comparatorArrivalTimeAndRelaxedC1IfC2IsOptimal( + @Nonnull RelaxFunction relaxCost, + @Nonnull DominanceFunction c2Comp ) { return (l, r) -> compareArrivalTime(l, r) || compareNumberOfTransfers(l, r) || compareDuration(l, r) || - compareC1(relaxCost, l, r) || - c2Comp.leftDominateRight(l.c2(), r.c2()); + compareC1RelaxedIfC2IsOptimal(l, r, relaxCost, c2Comp); } private static < T extends RaptorTripSchedule - > ParetoComparator> comparatorDepartureTimeAndRelaxedC1AndC2( - @Nonnull final RelaxFunction relaxCost, - @Nonnull final DominanceFunction c2Comp + > ParetoComparator> comparatorDepartureTimeAndRelaxedC1IfC2IsOptimal( + @Nonnull RelaxFunction relaxCost, + @Nonnull DominanceFunction c2Comp ) { return (l, r) -> compareDepartureTime(l, r) || compareNumberOfTransfers(l, r) || compareDuration(l, r) || - compareC1(relaxCost, l, r) || - c2Comp.leftDominateRight(l.c2(), r.c2()); + compareC1RelaxedIfC2IsOptimal(l, r, relaxCost, c2Comp); + } + + private static boolean compareC1RelaxedIfC2IsOptimal( + @Nonnull RaptorPath l, + @Nonnull RaptorPath r, + @Nonnull RelaxFunction relaxCost, + @Nonnull DominanceFunction c2Comp + ) { + return c2Comp.leftDominateRight(l.c2(), r.c2()) ? compareC1(relaxCost, l, r) : compareC1(l, r); } } diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java index 4403e72375c..89b2f447ca4 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java @@ -3,12 +3,16 @@ import static org.opentripplanner.raptor.rangeraptor.path.PathParetoSetComparators.paretoComparator; import org.opentripplanner.raptor.api.model.DominanceFunction; +import org.opentripplanner.raptor.api.model.GeneralizedCostRelaxFunction; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; +import org.opentripplanner.raptor.api.model.RelaxFunction; import org.opentripplanner.raptor.api.model.SearchDirection; import org.opentripplanner.raptor.api.path.RaptorPath; import org.opentripplanner.raptor.api.path.RaptorStopNameResolver; import org.opentripplanner.raptor.api.request.RaptorProfile; import org.opentripplanner.raptor.rangeraptor.context.SearchContext; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime; import org.opentripplanner.raptor.rangeraptor.internalapi.WorkerLifeCycle; import org.opentripplanner.raptor.rangeraptor.path.DestinationArrivalPaths; import org.opentripplanner.raptor.rangeraptor.path.ForwardPathMapper; @@ -35,23 +39,26 @@ public PathConfig(SearchContext context) { this.ctx = context; } + public DestinationArrivalPaths createDestArrivalPathsStdSearch() { + return createDestArrivalPaths(ParetoSetCost.NONE, DominanceFunction.noop()); + } + /** * Create a new {@link DestinationArrivalPaths}. - * @param includeC1Cost whether to include generalized cost in the pareto set criteria. - * It will be generated for each leg and a total for the path. + * @param costConfig Supported configurations of c1, c2 and relaxed cost(c1). * @param c2Comp c2 comparator function to be used in the pareto set criteria. If c2 comparator is null * then no c2 comparison will be used. */ public DestinationArrivalPaths createDestArrivalPaths( - boolean includeC1Cost, - final DominanceFunction c2Comp + ParetoSetCost costConfig, + DominanceFunction c2Comp ) { return new DestinationArrivalPaths<>( - createPathParetoComparator(includeC1Cost, c2Comp), + createPathParetoComparator(costConfig, c2Comp), ctx.calculator(), - includeC1Cost ? ctx.costCalculator() : null, + costConfig.includeC1() ? ctx.costCalculator() : null, ctx.slackProvider(), - createPathMapper(includeC1Cost), + createPathMapper(costConfig.includeC1()), ctx.debugFactory(), ctx.stopNameResolver(), ctx.lifeCycle() @@ -61,17 +68,30 @@ public DestinationArrivalPaths createDestArrivalPaths( /* private members */ private ParetoComparator> createPathParetoComparator( - boolean includeC1, - final DominanceFunction c2Comp + ParetoSetCost costConfig, + DominanceFunction c2Comp ) { - return paretoComparator( - includeC1, - ctx.searchParams().timetable(), - ctx.searchParams().preferLateArrival(), - ctx.searchDirection(), - ctx.multiCriteria().relaxC1AtDestination(), - c2Comp - ); + // This code goes away when the USE_C1_RELAX_DESTINATION is deleted + var relaxC1 = + switch (costConfig) { + case USE_C1_RELAXED_IF_C2_IS_OPTIMAL -> ctx.multiCriteria().relaxC1(); + case USE_C1_RELAX_DESTINATION -> GeneralizedCostRelaxFunction.of( + ctx.multiCriteria().relaxCostAtDestination() + ); + default -> RelaxFunction.NORMAL; + }; + + return paretoComparator(paretoSetTimeConfig(), costConfig, relaxC1, c2Comp); + } + + private ParetoSetTime paretoSetTimeConfig() { + boolean preferLatestDeparture = + ctx.searchParams().preferLateArrival() != ctx.searchDirection().isInReverse(); + + ParetoSetTime timeConfig = ctx.searchParams().timetable() + ? ParetoSetTime.USE_TIMETABLE + : (preferLatestDeparture ? ParetoSetTime.USE_DEPARTURE_TIME : ParetoSetTime.USE_ARRIVAL_TIME); + return timeConfig; } private PathMapper createPathMapper(boolean includeCost) { diff --git a/src/main/java/org/opentripplanner/raptor/rangeraptor/standard/configure/StdRangeRaptorConfig.java b/src/main/java/org/opentripplanner/raptor/rangeraptor/standard/configure/StdRangeRaptorConfig.java index d3f57c9443f..6e0c3ee5afd 100644 --- a/src/main/java/org/opentripplanner/raptor/rangeraptor/standard/configure/StdRangeRaptorConfig.java +++ b/src/main/java/org/opentripplanner/raptor/rangeraptor/standard/configure/StdRangeRaptorConfig.java @@ -6,9 +6,9 @@ import java.util.HashSet; import java.util.Set; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; -import org.opentripplanner.raptor.api.model.RelaxFunction; import org.opentripplanner.raptor.rangeraptor.context.SearchContext; import org.opentripplanner.raptor.rangeraptor.internalapi.Heuristics; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorWorkerResult; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorWorkerState; import org.opentripplanner.raptor.rangeraptor.internalapi.RoutingStrategy; @@ -174,7 +174,7 @@ private StopArrivalsState wrapStopArrivalsStateWithDebugger(StopArrivalsState } private DestinationArrivalPaths destinationArrivalPaths() { - var destinationArrivalPaths = pathConfig.createDestArrivalPaths(false, null); + var destinationArrivalPaths = pathConfig.createDestArrivalPathsStdSearch(); // Add egressArrivals to stops and bind them to the destination arrival paths. The // adapter notify the destination on each new egress stop arrival. @@ -244,25 +244,15 @@ private BestNumberOfTransfers resolveBestNumberOfTransfers() { } private UnknownPathFactory unknownPathFactory() { - return oneOf( - new UnknownPathFactory<>( - resolveBestTimes(), - resolveBestNumberOfTransfers(), - ctx.calculator(), - ctx.slackProvider().transferSlack(), - ctx.egressPaths(), - MIN_TRAVEL_DURATION.is(ctx.profile()), - paretoComparator( - false, - ctx.searchParams().timetable(), - ctx.searchParams().preferLateArrival(), - ctx.searchDirection(), - RelaxFunction.NORMAL, - null - ), - ctx.lifeCycle() - ), - UnknownPathFactory.class + return new UnknownPathFactory<>( + resolveBestTimes(), + resolveBestNumberOfTransfers(), + ctx.calculator(), + ctx.slackProvider().transferSlack(), + ctx.egressPaths(), + MIN_TRAVEL_DURATION.is(ctx.profile()), + paretoComparator(ctx.paretoSetTimeConfig(), ParetoSetCost.NONE, null, null), + ctx.lifeCycle() ); } diff --git a/src/main/java/org/opentripplanner/raptor/spi/UnknownPath.java b/src/main/java/org/opentripplanner/raptor/spi/UnknownPath.java index b95417e86b6..de6a846f480 100644 --- a/src/main/java/org/opentripplanner/raptor/spi/UnknownPath.java +++ b/src/main/java/org/opentripplanner/raptor/spi/UnknownPath.java @@ -125,7 +125,7 @@ public String toString(RaptorStopNameResolver stopNameTranslator) { public String toString() { PathStringBuilder pathBuilder = new PathStringBuilder(null); if (departureTime == 0 && arrivalTime == 0) { - pathBuilder.summary(c1()); + pathBuilder.summary(c1(), c2()); } else { pathBuilder.summary(startTime(), endTime(), numberOfTransfers, c1(), c2()); } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java index a9b042083bf..fe20592576e 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/mapping/RaptorPathToItineraryMapper.java @@ -153,6 +153,9 @@ else if (pathLeg.isTransferLeg()) { if (egressPathLeg.egress() instanceof DefaultAccessEgress ae) { itinerary.setAccessPenalty(ae.penalty()); } + if (path.isC2Set()) { + itinerary.setGeneralizedCost2(path.c2()); + } return itinerary; } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitPriorityGroup32n.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitGroupPriority32n.java similarity index 76% rename from src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitPriorityGroup32n.java rename to src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitGroupPriority32n.java index 9b744932b8b..feb3f6f7b3a 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitPriorityGroup32n.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitGroupPriority32n.java @@ -1,33 +1,33 @@ package org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.grouppriority; import org.opentripplanner.raptor.api.model.DominanceFunction; -import org.opentripplanner.raptor.api.request.RaptorTransitPriorityGroupCalculator; +import org.opentripplanner.raptor.api.request.RaptorTransitGroupCalculator; /** * This is a "BitSet" implementation for groupId. It can store upto 32 groups, * a set with few elements does NOT dominate a set with more elements. */ -public class TransitPriorityGroup32n { +public class TransitGroupPriority32n { private static final int GROUP_ZERO = 0; private static final int MIN_SEQ_NO = 0; private static final int MAX_SEQ_NO = 32; - public static RaptorTransitPriorityGroupCalculator priorityCalculator() { - return new RaptorTransitPriorityGroupCalculator() { + public static RaptorTransitGroupCalculator priorityCalculator() { + return new RaptorTransitGroupCalculator() { @Override - public int mergeTransitPriorityGroupIds(int currentGroupIds, int boardingGroupId) { + public int mergeGroupIds(int currentGroupIds, int boardingGroupId) { return mergeInGroupId(currentGroupIds, boardingGroupId); } @Override public DominanceFunction dominanceFunction() { - return TransitPriorityGroup32n::dominate; + return TransitGroupPriority32n::dominate; } @Override public String toString() { - return "TransitPriorityGroup32nCalculator{}"; + return "TransitGroupPriority32nCalculator{}"; } }; } @@ -42,7 +42,7 @@ public static boolean dominate(int left, int right) { @Override public String toString() { - return "TransitPriorityGroup32n{}"; + return "TransitGroupPriority32n{}"; } /** @@ -64,12 +64,12 @@ public static int mergeInGroupId(final int currentSetOfGroupIds, final int newGr private static void assertValidGroupSeqNo(int priorityGroupIndex) { if (priorityGroupIndex < MIN_SEQ_NO) { throw new IllegalArgumentException( - "Transit priority group can not be a negative number: " + priorityGroupIndex + "Transit group priority can not be a negative number: " + priorityGroupIndex ); } if (priorityGroupIndex > MAX_SEQ_NO) { throw new IllegalArgumentException( - "Transit priority group exceeds max number of groups: " + + "Transit group priority exceeds max number of groups: " + priorityGroupIndex + " (MAX=" + MAX_SEQ_NO + diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java index 6093ce56fa3..8cff5264846 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/RaptorRequestMapper.java @@ -23,7 +23,7 @@ import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitLayer; import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripSchedule; import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter; -import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.grouppriority.TransitPriorityGroup32n; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.grouppriority.TransitGroupPriority32n; import org.opentripplanner.routing.api.request.RouteRequest; import org.opentripplanner.routing.api.request.framework.CostLinearFunction; import org.opentripplanner.transit.model.site.StopLocation; @@ -125,9 +125,9 @@ private RaptorRequest doMap() { builder.withMultiCriteria(mcBuilder -> { var pt = preferences.transit(); var r = pt.raptor(); - if (!pt.relaxTransitPriorityGroup().isNormal()) { - mcBuilder.withTransitPriorityCalculator(TransitPriorityGroup32n.priorityCalculator()); - mcBuilder.withRelaxC1(mapRelaxCost(pt.relaxTransitPriorityGroup())); + if (!pt.relaxTransitGroupPriority().isNormal()) { + mcBuilder.withTransitPriorityCalculator(TransitGroupPriority32n.priorityCalculator()); + mcBuilder.withRelaxC1(mapRelaxCost(pt.relaxTransitGroupPriority())); } else { mcBuilder.withPassThroughPoints(mapPassThroughPoints()); r.relaxGeneralizedCostAtDestination().ifPresent(mcBuilder::withRelaxCostAtDestination); diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/BinarySetOperator.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/BinarySetOperator.java new file mode 100644 index 00000000000..35e5b8c0918 --- /dev/null +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/BinarySetOperator.java @@ -0,0 +1,20 @@ +package org.opentripplanner.routing.algorithm.raptoradapter.transit.request; + +/** + * Used to concatenate matches with either the logical "AND" or "OR" operator. + */ +enum BinarySetOperator { + AND("&"), + OR("|"); + + private final String token; + + BinarySetOperator(String token) { + this.token = token; + } + + @Override + public String toString() { + return token; + } +} diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfigurator.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfigurator.java index ba9af45adba..826b9c09a13 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfigurator.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfigurator.java @@ -8,14 +8,14 @@ import java.util.List; import java.util.stream.Stream; import org.opentripplanner.framework.lang.ArrayUtils; -import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.grouppriority.TransitPriorityGroup32n; -import org.opentripplanner.routing.api.request.request.filter.TransitPriorityGroupSelect; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.grouppriority.TransitGroupPriority32n; +import org.opentripplanner.routing.api.request.request.filter.TransitGroupSelect; import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.network.RoutingTripPattern; /** * This class dynamically builds an index of transit-group-ids from the - * provided {@link TransitPriorityGroupSelect}s while serving the caller with + * provided {@link TransitGroupSelect}s while serving the caller with * group-ids for each requested pattern. It is made for optimal * performance, since it is used in request scope. *

    @@ -23,8 +23,29 @@ */ public class PriorityGroupConfigurator { - private static final int BASE_GROUP_ID = TransitPriorityGroup32n.groupId(0); - private int groupIndexCounter = 0; + /** + * There are two ways we can treat the base (local-traffic) transit priority group: + *

      + *
    1. We can assign group id 1 (one) to the base group and it will be treated as any other group. + *
    2. We can assign group id 0 (zero) to the base and it will not be added to the set of groups + * a given path has. + *
    + * When we compare paths we compare sets of group ids. A set is dominating another set if it is + * a smaller subset or different from the other set. + *

    + * Example - base-group-id = 0 (zero) + *

    + * Let B be the base and G be concrete group. Then: (B) dominates (G), (G) dominates (B), (B) + * dominates (BG), but (G) does not dominate (BG). In other words, paths with only agency + * X (group G) is not given an advantage in the routing over paths with a combination of agency + * X (group G) and local traffic (group B). + *

    + * TODO: Experiment with base-group-id=0 and make it configurable. + */ + private static final int GROUP_INDEX_COUNTER_START = 1; + + private final int baseGroupId = TransitGroupPriority32n.groupId(GROUP_INDEX_COUNTER_START); + private int groupIndexCounter = GROUP_INDEX_COUNTER_START; private final boolean enabled; private final PriorityGroupMatcher[] agencyMatchers; private final PriorityGroupMatcher[] globalMatchers; @@ -42,8 +63,8 @@ private PriorityGroupConfigurator() { } private PriorityGroupConfigurator( - Collection byAgency, - Collection global + Collection byAgency, + Collection global ) { this.agencyMatchers = PriorityGroupMatcher.of(byAgency); this.globalMatchers = PriorityGroupMatcher.of(global); @@ -59,8 +80,8 @@ public static PriorityGroupConfigurator empty() { } public static PriorityGroupConfigurator of( - Collection byAgency, - Collection global + Collection byAgency, + Collection global ) { if (Stream.of(byAgency, global).allMatch(Collection::isEmpty)) { return empty(); @@ -73,9 +94,9 @@ public static PriorityGroupConfigurator of( *

    * @throws IllegalArgumentException if more than 32 group-ids are requested. */ - public int lookupTransitPriorityGroupId(RoutingTripPattern tripPattern) { + public int lookupTransitGroupPriorityId(RoutingTripPattern tripPattern) { if (!enabled || tripPattern == null) { - return BASE_GROUP_ID; + return baseGroupId; } var p = tripPattern.getPattern(); @@ -99,11 +120,15 @@ public int lookupTransitPriorityGroupId(RoutingTripPattern tripPattern) { } } // Fallback to base-group-id - return BASE_GROUP_ID; + return baseGroupId; + } + + public int baseGroupId() { + return baseGroupId; } private int nextGroupId() { - return TransitPriorityGroup32n.groupId(++groupIndexCounter); + return TransitGroupPriority32n.groupId(++groupIndexCounter); } /** Pair of matcher and groupId. Used only inside this class. */ diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcher.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcher.java index dd7c1b46636..c017f2862ab 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcher.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcher.java @@ -1,5 +1,8 @@ package org.opentripplanner.routing.algorithm.raptoradapter.transit.request; +import static org.opentripplanner.routing.algorithm.raptoradapter.transit.request.BinarySetOperator.AND; +import static org.opentripplanner.routing.algorithm.raptoradapter.transit.request.BinarySetOperator.OR; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -12,13 +15,13 @@ import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.opentripplanner.routing.api.request.request.filter.TransitPriorityGroupSelect; +import org.opentripplanner.routing.api.request.request.filter.TransitGroupSelect; import org.opentripplanner.transit.model.basic.TransitMode; import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.network.TripPattern; /** - * This class turns a {@link TransitPriorityGroupSelect} into a matcher. + * This class turns a {@link TransitGroupSelect} into a matcher. *

    * Design: It uses the composite design pattern. A matcher is created for each * value in the "select", then the list of non-empty matchers is merged into @@ -39,7 +42,7 @@ boolean isEmpty() { } }; - public static PriorityGroupMatcher of(TransitPriorityGroupSelect select) { + public static PriorityGroupMatcher of(TransitGroupSelect select) { if (select.isEmpty()) { return NOOP; } @@ -59,10 +62,10 @@ public static PriorityGroupMatcher of(TransitPriorityGroupSelect select) { if (!select.routeIds().isEmpty()) { list.add(new IdMatcher("Route", select.routeIds(), p -> p.getRoute().getId())); } - return compositeOf(list); + return andOf(list); } - static PriorityGroupMatcher[] of(Collection selectors) { + static PriorityGroupMatcher[] of(Collection selectors) { return selectors .stream() .map(PriorityGroupMatcher::of) @@ -70,15 +73,15 @@ static PriorityGroupMatcher[] of(Collection selector .toArray(PriorityGroupMatcher[]::new); } - private static String arrayToString(T[] values) { - return colToString(Arrays.asList(values)); + private static String arrayToString(BinarySetOperator op, T[] values) { + return colToString(op, Arrays.asList(values)); } - private static String colToString(Collection values) { - return values.stream().map(Objects::toString).collect(Collectors.joining(" | ")); + private static String colToString(BinarySetOperator op, Collection values) { + return values.stream().map(Objects::toString).collect(Collectors.joining(" " + op + " ")); } - private static PriorityGroupMatcher compositeOf(List list) { + private static PriorityGroupMatcher andOf(List list) { // Remove empty/noop matchers list = list.stream().filter(Predicate.not(PriorityGroupMatcher::isEmpty)).toList(); @@ -88,7 +91,7 @@ private static PriorityGroupMatcher compositeOf(List list) if (list.size() == 1) { return list.get(0); } - return new CompositeMatcher(list); + return new AndMatcher(list); } abstract boolean match(TripPattern pattern); @@ -112,7 +115,7 @@ boolean match(TripPattern pattern) { @Override public String toString() { - return "Mode(" + colToString(modes) + ')'; + return "Mode(" + colToString(OR, modes) + ')'; } } @@ -145,7 +148,7 @@ boolean match(TripPattern pattern) { @Override public String toString() { - return typeName + "Regexp(" + arrayToString(subModeRegexp) + ')'; + return typeName + "Regexp(" + arrayToString(OR, subModeRegexp) + ')'; } } @@ -172,35 +175,35 @@ boolean match(TripPattern pattern) { @Override public String toString() { - return typeName + "Id(" + colToString(ids) + ')'; + return typeName + "Id(" + colToString(OR, ids) + ')'; } } /** - * Take a list of matchers and provide a single interface. At least one matcher in the - * list must match for the composite matcher to return a match. + * Takes a list of matchers and provide a single interface. All matchers in the list must match + * for the composite matcher to return a match. */ - private static final class CompositeMatcher extends PriorityGroupMatcher { + private static final class AndMatcher extends PriorityGroupMatcher { private final PriorityGroupMatcher[] matchers; - public CompositeMatcher(List matchers) { + public AndMatcher(List matchers) { this.matchers = matchers.toArray(PriorityGroupMatcher[]::new); } @Override boolean match(TripPattern pattern) { for (var m : matchers) { - if (m.match(pattern)) { - return true; + if (!m.match(pattern)) { + return false; } } - return false; + return true; } @Override public String toString() { - return "(" + arrayToString(matchers) + ')'; + return "(" + arrayToString(AND, matchers) + ')'; } } } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java index 16a9baec95a..0d6b4497c1b 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java @@ -95,7 +95,7 @@ public RaptorRoutingRequestTransitData( additionalPastSearchDays, additionalFutureSearchDays, filter, - createTransitPriorityGroupConfigurator(request) + createTransitGroupPriorityConfigurator(request) ); this.patternIndex = transitDataCreator.createPatternIndex(tripPatterns); this.activeTripPatternsPerStop = transitDataCreator.createTripPatternsPerStop(tripPatterns); @@ -246,8 +246,8 @@ public RaptorConstrainedBoardingSearch transferConstraintsReverseS return new ConstrainedBoardingSearch(false, toStopTransfers, fromStopTransfers); } - private PriorityGroupConfigurator createTransitPriorityGroupConfigurator(RouteRequest request) { - if (request.preferences().transit().relaxTransitPriorityGroup().isNormal()) { + private PriorityGroupConfigurator createTransitGroupPriorityConfigurator(RouteRequest request) { + if (request.preferences().transit().relaxTransitGroupPriority().isNormal()) { return PriorityGroupConfigurator.empty(); } var transitRequest = request.journey().transit(); diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java index 0cb155facd5..b8f915d6eb4 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitDataCreator.java @@ -147,7 +147,7 @@ static List merge( tripPattern.getAlightingPossible(), BoardAlight.ALIGHT ), - priorityGroupConfigurator.lookupTransitPriorityGroupId(tripPattern) + priorityGroupConfigurator.lookupTransitGroupPriorityId(tripPattern) ) ); } diff --git a/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainService.java b/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainService.java index 91b08212d2f..bfe5c3de841 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainService.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainService.java @@ -8,6 +8,7 @@ import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nullable; +import org.opentripplanner.raptor.api.model.RaptorConstants; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; import org.opentripplanner.raptor.api.path.RaptorPath; import org.opentripplanner.raptor.api.path.RaptorStopNameResolver; @@ -115,6 +116,8 @@ public Set> findBestTransitPath(RaptorPath originalPath) { var filteredTails = filter.filterFinalResult(tails); + setC2IfNotSet(originalPath, filteredTails); + return filteredTails.stream().map(OptimizedPathTail::build).collect(toSet()); } @@ -255,4 +258,20 @@ private List>> sortTransfersOnArrivalStopPosInDecOrde ) .collect(Collectors.toList()); } + + /** + * Copy over c2 value from origin to new path if the c2 value is not generated by this service. + */ + private static void setC2IfNotSet( + RaptorPath originalPath, + Set> filteredTails + ) { + if (originalPath.isC2Set()) { + for (OptimizedPathTail tail : filteredTails) { + if (!tail.isC2Set()) { + tail.c2(originalPath.c2()); + } + } + } + } } diff --git a/src/main/java/org/opentripplanner/routing/api/request/preference/TransitPreferences.java b/src/main/java/org/opentripplanner/routing/api/request/preference/TransitPreferences.java index 094a8b3458c..3397f5cbba7 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/preference/TransitPreferences.java +++ b/src/main/java/org/opentripplanner/routing/api/request/preference/TransitPreferences.java @@ -26,7 +26,7 @@ public final class TransitPreferences implements Serializable { private final Map reluctanceForMode; private final Cost otherThanPreferredRoutesPenalty; private final CostLinearFunction unpreferredCost; - private final CostLinearFunction relaxTransitPriorityGroup; + private final CostLinearFunction relaxTransitGroupPriority; private final boolean ignoreRealtimeUpdates; private final boolean includePlannedCancellations; private final boolean includeRealtimeCancellations; @@ -38,7 +38,7 @@ private TransitPreferences() { this.reluctanceForMode = Map.of(); this.otherThanPreferredRoutesPenalty = Cost.costOfMinutes(5); this.unpreferredCost = CostLinearFunction.NORMAL; - this.relaxTransitPriorityGroup = CostLinearFunction.NORMAL; + this.relaxTransitGroupPriority = CostLinearFunction.NORMAL; this.ignoreRealtimeUpdates = false; this.includePlannedCancellations = false; this.includeRealtimeCancellations = false; @@ -52,7 +52,7 @@ private TransitPreferences(Builder builder) { this.reluctanceForMode = Map.copyOf(requireNonNull(builder.reluctanceForMode)); this.otherThanPreferredRoutesPenalty = builder.otherThanPreferredRoutesPenalty; this.unpreferredCost = requireNonNull(builder.unpreferredCost); - this.relaxTransitPriorityGroup = Objects.requireNonNull(builder.relaxTransitPriorityGroup); + this.relaxTransitGroupPriority = Objects.requireNonNull(builder.relaxTransitGroupPriority); this.ignoreRealtimeUpdates = builder.ignoreRealtimeUpdates; this.includePlannedCancellations = builder.includePlannedCancellations; this.includeRealtimeCancellations = builder.includeRealtimeCancellations; @@ -131,13 +131,13 @@ public CostLinearFunction unpreferredCost() { } /** - * This is used to relax the cost when comparing transit-priority-groups. The default is the - * NORMAL function({@code f(x) = x}. This is the same as not using priority-groups. The + * This is used to relax the cost when comparing transit-groups. The default is the + * NORMAL function({@code f(t) = t}. This is the same as not using priority-groups. The * coefficient must be in range {@code [1.0 to 4.0]} and the constant must be in range * {@code [$0 to $1440(4h)]}. */ - public CostLinearFunction relaxTransitPriorityGroup() { - return relaxTransitPriorityGroup; + public CostLinearFunction relaxTransitGroupPriority() { + return relaxTransitGroupPriority; } /** @@ -184,7 +184,7 @@ public boolean equals(Object o) { reluctanceForMode.equals(that.reluctanceForMode) && otherThanPreferredRoutesPenalty == that.otherThanPreferredRoutesPenalty && unpreferredCost.equals(that.unpreferredCost) && - Objects.equals(relaxTransitPriorityGroup, that.relaxTransitPriorityGroup) && + Objects.equals(relaxTransitGroupPriority, that.relaxTransitGroupPriority) && ignoreRealtimeUpdates == that.ignoreRealtimeUpdates && includePlannedCancellations == that.includePlannedCancellations && includeRealtimeCancellations == that.includeRealtimeCancellations && @@ -201,7 +201,7 @@ public int hashCode() { reluctanceForMode, otherThanPreferredRoutesPenalty, unpreferredCost, - relaxTransitPriorityGroup, + relaxTransitGroupPriority, ignoreRealtimeUpdates, includePlannedCancellations, includeRealtimeCancellations, @@ -223,7 +223,7 @@ public String toString() { DEFAULT.otherThanPreferredRoutesPenalty ) .addObj("unpreferredCost", unpreferredCost, DEFAULT.unpreferredCost) - .addObj("relaxTransitPriorityGroup", relaxTransitPriorityGroup, CostLinearFunction.NORMAL) + .addObj("relaxTransitGroupPriority", relaxTransitGroupPriority, CostLinearFunction.NORMAL) .addBoolIfTrue( "ignoreRealtimeUpdates", ignoreRealtimeUpdates != DEFAULT.ignoreRealtimeUpdates @@ -250,7 +250,7 @@ public static class Builder { private Map reluctanceForMode; private Cost otherThanPreferredRoutesPenalty; private CostLinearFunction unpreferredCost; - private CostLinearFunction relaxTransitPriorityGroup; + private CostLinearFunction relaxTransitGroupPriority; private boolean ignoreRealtimeUpdates; private boolean includePlannedCancellations; private boolean includeRealtimeCancellations; @@ -264,7 +264,7 @@ public Builder(TransitPreferences original) { this.reluctanceForMode = original.reluctanceForMode; this.otherThanPreferredRoutesPenalty = original.otherThanPreferredRoutesPenalty; this.unpreferredCost = original.unpreferredCost; - this.relaxTransitPriorityGroup = original.relaxTransitPriorityGroup; + this.relaxTransitGroupPriority = original.relaxTransitGroupPriority; this.ignoreRealtimeUpdates = original.ignoreRealtimeUpdates; this.includePlannedCancellations = original.includePlannedCancellations; this.includeRealtimeCancellations = original.includeRealtimeCancellations; @@ -314,8 +314,8 @@ public Builder setUnpreferredCostString(String constFunction) { return setUnpreferredCost(CostLinearFunction.of(constFunction)); } - public Builder withTransitGroupPriorityGeneralizedCostSlack(CostLinearFunction value) { - this.relaxTransitPriorityGroup = value; + public Builder withRelaxTransitGroupPriority(CostLinearFunction value) { + this.relaxTransitGroupPriority = value; return this; } diff --git a/src/main/java/org/opentripplanner/routing/api/request/request/TransitRequest.java b/src/main/java/org/opentripplanner/routing/api/request/request/TransitRequest.java index 67a56249328..b5626c479a0 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/request/TransitRequest.java +++ b/src/main/java/org/opentripplanner/routing/api/request/request/TransitRequest.java @@ -8,7 +8,7 @@ import org.opentripplanner.routing.api.request.DebugRaptor; import org.opentripplanner.routing.api.request.request.filter.AllowAllTransitFilter; import org.opentripplanner.routing.api.request.request.filter.TransitFilter; -import org.opentripplanner.routing.api.request.request.filter.TransitPriorityGroupSelect; +import org.opentripplanner.routing.api.request.request.filter.TransitGroupSelect; import org.opentripplanner.transit.model.framework.FeedScopedId; // TODO VIA: Javadoc @@ -31,8 +31,8 @@ public class TransitRequest implements Cloneable, Serializable { private List unpreferredRoutes = List.of(); - private List priorityGroupsByAgency = new ArrayList<>(); - private List priorityGroupsGlobal = new ArrayList<>(); + private List priorityGroupsByAgency = new ArrayList<>(); + private List priorityGroupsGlobal = new ArrayList<>(); private DebugRaptor raptorDebugging = new DebugRaptor(); public void setBannedTripsFromString(String ids) { @@ -64,16 +64,14 @@ public void setFilters(List filters) { *

    * Note! Entities that are not matched are put in the BASE-GROUP with id 0. */ - public List priorityGroupsByAgency() { + public List priorityGroupsByAgency() { return priorityGroupsByAgency; } /** * All patterns matching the same select will be assigned the same group-id. */ - public void addPriorityGroupsByAgency( - Collection priorityGroupsByAgency - ) { + public void addPriorityGroupsByAgency(Collection priorityGroupsByAgency) { this.priorityGroupsByAgency.addAll(priorityGroupsByAgency); } @@ -82,11 +80,11 @@ public void addPriorityGroupsByAgency( *

    * Note! Entities that are not matched are put in the BASE-GROUP with id 0. */ - public List priorityGroupsGlobal() { + public List priorityGroupsGlobal() { return priorityGroupsGlobal; } - public void addPriorityGroupsGlobal(Collection priorityGroupsGlobal) { + public void addPriorityGroupsGlobal(Collection priorityGroupsGlobal) { this.priorityGroupsGlobal.addAll(priorityGroupsGlobal); } diff --git a/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitPriorityGroupSelect.java b/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitGroupSelect.java similarity index 87% rename from src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitPriorityGroupSelect.java rename to src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitGroupSelect.java index 6d763e9c3bc..dfa5daa0e31 100644 --- a/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitPriorityGroupSelect.java +++ b/src/main/java/org/opentripplanner/routing/api/request/request/filter/TransitGroupSelect.java @@ -21,23 +21,23 @@ *

  • {@code Entity(mode:SUBWAY, agency:A3)}
  • * */ -public class TransitPriorityGroupSelect { +public class TransitGroupSelect { - private static final TransitPriorityGroupSelect DEFAULT = new TransitPriorityGroupSelect(); + private static final TransitGroupSelect DEFAULT = new TransitGroupSelect(); private final List modes; private final List subModeRegexp; private final List agencyIds; private final List routeIds; - public TransitPriorityGroupSelect() { + public TransitGroupSelect() { this.modes = List.of(); this.subModeRegexp = List.of(); this.agencyIds = List.of(); this.routeIds = List.of(); } - private TransitPriorityGroupSelect(Builder builder) { + private TransitGroupSelect(Builder builder) { // Sort and keep only unique entries, this make this // implementation consistent for eq/hc/toString. this.modes = @@ -77,7 +77,7 @@ public boolean isEmpty() { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - TransitPriorityGroupSelect that = (TransitPriorityGroupSelect) o; + TransitGroupSelect that = (TransitGroupSelect) o; return ( Objects.equals(modes, that.modes) && Objects.equals(subModeRegexp, that.subModeRegexp) && @@ -96,7 +96,7 @@ public String toString() { return isEmpty() ? "TransitGroupSelect{ EMPTY }" : ToStringBuilder - .of(TransitPriorityGroupSelect.class) + .of(TransitGroupSelect.class) .addCol("modes", modes) .addCol("subModeRegexp", subModeRegexp) .addCol("agencyIds", agencyIds) @@ -106,13 +106,13 @@ public String toString() { public static class Builder { - private final TransitPriorityGroupSelect original; + private final TransitGroupSelect original; private final List modes; private final List subModeRegexp; private final List agencyIds; private final List routeIds; - public Builder(TransitPriorityGroupSelect original) { + public Builder(TransitGroupSelect original) { this.original = original; this.modes = new ArrayList<>(original.modes); this.subModeRegexp = new ArrayList<>(original.subModeRegexp); @@ -140,8 +140,8 @@ public Builder addRouteIds(Collection routeIds) { return this; } - public TransitPriorityGroupSelect build() { - var obj = new TransitPriorityGroupSelect(this); + public TransitGroupSelect build() { + var obj = new TransitGroupSelect(this); return original.equals(obj) ? original : obj; } } diff --git a/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java index 1d80fe4b104..843221d296c 100644 --- a/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java @@ -167,7 +167,7 @@ cost function. The cost function (`unpreferredCost`) is defined as a linear func .asFeedScopedIds(request.journey().transit().unpreferredAgencies()) ); - TransitPriorityGroupConfig.mapTransitRequest(c, request.journey().transit()); + TransitGroupPriorityConfig.mapTransitRequest(c, request.journey().transit()); // Map preferences request.withPreferences(preferences -> mapPreferences(c, request, preferences)); @@ -297,25 +297,24 @@ The board time is added to the time when going from the stop (offboard) to onboa .asCostLinearFunction(dft.unpreferredCost()) ); - String relaxTransitPriorityGroupValue = c - .of("relaxTransitPriorityGroup") + String relaxTransitGroupPriorityValue = c + .of("relaxTransitGroupPriority") .since(V2_5) - .summary("The relax function for transit-priority-groups") + .summary("The relax function for transit-group-priority") .description( """ - A path is considered optimal if the generalized-cost is less than the - generalized-cost of another path. If this parameter is set, the comparison is relaxed - further if they belong to different transit-priority-groups. + A path is considered optimal if the generalized-cost is less than the generalized-cost of + another path. If this parameter is set, the comparison is relaxed further if they belong + to different transit groups. """ ) - .asString(dft.relaxTransitPriorityGroup().toString()); + .asString(dft.relaxTransitGroupPriority().toString()); - if (relaxTransitPriorityGroupValue != null) { - builder.withTransitGroupPriorityGeneralizedCostSlack( - CostLinearFunction.of(relaxTransitPriorityGroupValue) - ); + if (relaxTransitGroupPriorityValue != null) { + builder.withRelaxTransitGroupPriority(CostLinearFunction.of(relaxTransitGroupPriorityValue)); } + // TODO REMOVE THIS builder .withRaptor(it -> c diff --git a/src/main/java/org/opentripplanner/standalone/config/routerequest/TransitPriorityGroupConfig.java b/src/main/java/org/opentripplanner/standalone/config/routerequest/TransitGroupPriorityConfig.java similarity index 76% rename from src/main/java/org/opentripplanner/standalone/config/routerequest/TransitPriorityGroupConfig.java rename to src/main/java/org/opentripplanner/standalone/config/routerequest/TransitGroupPriorityConfig.java index 51faafc7cbf..e5f1ccd784a 100644 --- a/src/main/java/org/opentripplanner/standalone/config/routerequest/TransitPriorityGroupConfig.java +++ b/src/main/java/org/opentripplanner/standalone/config/routerequest/TransitGroupPriorityConfig.java @@ -6,36 +6,36 @@ import java.util.Collection; import java.util.List; import org.opentripplanner.routing.api.request.request.TransitRequest; -import org.opentripplanner.routing.api.request.request.filter.TransitPriorityGroupSelect; +import org.opentripplanner.routing.api.request.request.filter.TransitGroupSelect; import org.opentripplanner.standalone.config.framework.json.NodeAdapter; import org.opentripplanner.standalone.config.framework.json.OtpVersion; import org.opentripplanner.transit.model.basic.TransitMode; -public class TransitPriorityGroupConfig { +public class TransitGroupPriorityConfig { public static void mapTransitRequest(NodeAdapter root, TransitRequest transit) { var c = root - .of("transitPriorityGroups") + .of("transitGroupPriority") .since(OtpVersion.V2_5) - .summary("Transit priority groups configuration") + .summary( + "Group transit patterns and give each group a mutual advantage in the Raptor search." + ) .description( """ Use this to separate transit patterns into groups. Each group will be given a group-id. A path (multiple legs) will then have a set of group-ids based on the group-id from each leg. Hence, two paths with a different set of group-ids will BOTH be optimal unless the cost is - worse than the relaxation specified in the `relaxTransitPriorityGroup` parameter. This is + worse than the relaxation specified in the `relaxTransitGroupPriority` parameter. This is only available in the TransmodelAPI for now. - Unmatched patterns are put in the BASE priority-group (group id: 0). This group is special. - If a path only have legs in the base group, then that path dominates other paths, but other - paths must be better to make it. + Unmatched patterns are put in the BASE priority-group. """ ) .experimentalFeature() .asObject(); transit.addPriorityGroupsByAgency( - TransitPriorityGroupConfig.mapList( + TransitGroupPriorityConfig.mapList( c, "byAgency", "All groups here are split by agency. For example if you list mode " + @@ -44,7 +44,7 @@ public static void mapTransitRequest(NodeAdapter root, TransitRequest transit) { ) ); transit.addPriorityGroupsGlobal( - TransitPriorityGroupConfig.mapList( + TransitGroupPriorityConfig.mapList( c, "global", "All services matching a 'global' group will get the same group-id. Use this " + @@ -53,7 +53,7 @@ public static void mapTransitRequest(NodeAdapter root, TransitRequest transit) { ); } - private static Collection mapList( + private static Collection mapList( NodeAdapter root, String parameterName, String description @@ -61,13 +61,13 @@ private static Collection mapList( return root .of(parameterName) .since(V2_5) - .summary("Configuration for transit priority groups.") + .summary("List of transit groups.") .description(description + " The max total number of group-ids are 32, so be careful.") - .asObjects(TransitPriorityGroupConfig::mapTransitGroupSelect); + .asObjects(TransitGroupPriorityConfig::mapTransitGroupSelect); } - private static TransitPriorityGroupSelect mapTransitGroupSelect(NodeAdapter c) { - return TransitPriorityGroupSelect + private static TransitGroupSelect mapTransitGroupSelect(NodeAdapter c) { + return TransitGroupSelect .of() .addModes( c diff --git a/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql b/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql index 75ebc33cccd..0fccc1644b4 100644 --- a/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql +++ b/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql @@ -827,21 +827,21 @@ type QueryType { passThroughPoints: [PassThroughPoint!], """ Relax generalized-cost when comparing trips with a different set of - transit-priority-groups. The groups are set server side for service-journey and + transit-group-priorities. The groups are set server side for service-journey and can not be configured in the API. This mainly helps to return competition neutral - services. Long distance authorities are put in different transit-priority-groups. + services. Long distance authorities are put in different transit-groups. This relaxes the comparison inside the routing engine for each stop-arrival. If two - paths have a different set of transit-priority-groups, then the generalized-cost + paths have a different set of transit-group-priorities, then the generalized-cost comparison is relaxed. The final set of paths are filtered through the normal itinerary-filters. - The `ratio` must be greater or equal to 1.0 and less then 1.2. - - The `slack` must be greater or equal to 0 and less then 3600. + - The `constant` must be greater or equal to '0s' and less then '1h'. THIS IS STILL AN EXPERIMENTAL FEATURE - IT MAY CHANGE WITHOUT ANY NOTICE! """ - relaxTransitPriorityGroup: RelaxCostInput = null, + relaxTransitGroupPriority: RelaxCostInput = null, """ Whether non-optimal transit paths at the destination should be returned. Let c be the existing minimum pareto optimal generalized-cost to beat. Then a trip with cost c' is @@ -854,7 +854,7 @@ type QueryType { Values less than 1.0 is not allowed, and values greater than 2.0 are not supported, due to performance reasons. """ - relaxTransitSearchGeneralizedCostAtDestination: Float = null @deprecated(reason : "This is replaced by 'relaxTransitPriorityGroup'."), + relaxTransitSearchGeneralizedCostAtDestination: Float = null @deprecated(reason : "This is replaced by 'relaxTransitGroupPriority'."), """ The length of the search-window in minutes. This parameter is optional. @@ -1288,6 +1288,8 @@ type TripPattern { expectedStartTime: DateTime! "Generalized cost or weight of the itinerary. Used for debugging." generalizedCost: Int + "A second cost or weight of the itinerary. Some use-cases like pass-through and transit-priority-groups use a second cost during routing. This is used for debugging." + generalizedCost2: Int "A list of legs. Each leg is either a walking (cycling, car) portion of the trip, or a ride leg on a particular vehicle. So a trip where the use walks to the Q train, transfers to the 6, then walks to their destination, has four legs." legs: [Leg!]! "Time that the trip departs." @@ -1867,6 +1869,9 @@ enum WheelchairBoarding { "List of coordinates like: [[60.89, 11.12], [62.56, 12.10]]" scalar Coordinates +"A cost value, normally a value of 1 is equivalent to riding transit for 1 second, but it might not depending on the use-case. Format: 3665 = DT1h1m5s = 1h1m5s" +scalar Cost + "Local date using the ISO 8601 format: `YYYY-MM-DD`. Example: `2020-05-17`." scalar Date @@ -2028,12 +2033,12 @@ This is used to include more results into the result. A `ratio=2.0` means a path with twice as high cost as another one, is accepted. A `constant=$300` means a "fixed" constant is added to the limit. A `{ratio=1.0, constant=0}` is said to be the NORMAL relaxed cost - the limit is the same as the cost used to calculate the limit. The NORMAL is usually -the default. We can express the RelaxCost as a function `f(x) = constant + ratio * x`. -`f(x)=x` is the NORMAL function. +the default. We can express the RelaxCost as a function `f(t) = constant + ratio * t`. +`f(t)=t` is the NORMAL function. """ input RelaxCostInput { - "The constant value to add to the limit. Must be a positive number. The unit is cost-seconds." - constant: [ID!] = 0 + "The constant value to add to the limit. Must be a positive number. The value isequivalent to transit-cost-seconds. Integers is treated as seconds, but you may use the duration format. Example: '3665 = 'DT1h1m5s' = '1h1m5s'." + constant: Cost = "0s" "The factor to multiply with the 'other cost'. Minimum value is 1.0." ratio: Float = 1.0 } diff --git a/src/test/java/org/opentripplanner/apis/transmodel/model/plan/RelaxCostTypeTest.java b/src/test/java/org/opentripplanner/apis/transmodel/model/plan/RelaxCostTypeTest.java new file mode 100644 index 00000000000..b4da249742c --- /dev/null +++ b/src/test/java/org/opentripplanner/apis/transmodel/model/plan/RelaxCostTypeTest.java @@ -0,0 +1,71 @@ +package org.opentripplanner.apis.transmodel.model.plan; + +import static org.junit.jupiter.api.Assertions.*; +import static org.opentripplanner.apis.transmodel.model.plan.RelaxCostType.CONSTANT; +import static org.opentripplanner.apis.transmodel.model.plan.RelaxCostType.RATIO; + +import graphql.language.FloatValue; +import graphql.language.ObjectField; +import graphql.language.ObjectValue; +import graphql.language.StringValue; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.opentripplanner.framework.model.Cost; +import org.opentripplanner.routing.api.request.framework.CostLinearFunction; + +class RelaxCostTypeTest { + + @Test + void valueOf() { + assertEquals( + ObjectValue + .newObjectValue() + .objectField(ObjectField.newObjectField().name(RATIO).value(FloatValue.of(1.0)).build()) + .objectField( + ObjectField.newObjectField().name(CONSTANT).value(StringValue.of("0s")).build() + ) + .build() + .toString(), + RelaxCostType.valueOf(CostLinearFunction.NORMAL).toString() + ); + assertEquals( + ObjectValue + .newObjectValue() + .objectField(ObjectField.newObjectField().name(RATIO).value(FloatValue.of(1.3)).build()) + .objectField( + ObjectField.newObjectField().name(CONSTANT).value(StringValue.of("1m7s")).build() + ) + .build() + .toString(), + RelaxCostType.valueOf(CostLinearFunction.of(Cost.costOfSeconds(67), 1.3)).toString() + ); + } + + @Test + void mapToDomain() { + Map input; + + input = Map.of(RATIO, 1.0, CONSTANT, Cost.ZERO); + assertEquals( + CostLinearFunction.NORMAL, + RelaxCostType.mapToDomain(input, CostLinearFunction.ZERO) + ); + + input = Map.of(RATIO, 0.0, CONSTANT, Cost.ZERO); + assertEquals( + CostLinearFunction.ZERO, + RelaxCostType.mapToDomain(input, CostLinearFunction.ZERO) + ); + + input = Map.of(RATIO, 1.7, CONSTANT, Cost.costOfSeconds(3600 + 3 * 60 + 7)); + assertEquals( + CostLinearFunction.of("1h3m7s + 1.7t"), + RelaxCostType.mapToDomain(input, CostLinearFunction.ZERO) + ); + assertEquals( + CostLinearFunction.NORMAL, + RelaxCostType.mapToDomain(null, CostLinearFunction.NORMAL) + ); + assertEquals(CostLinearFunction.ZERO, RelaxCostType.mapToDomain(null, CostLinearFunction.ZERO)); + } +} diff --git a/src/test/java/org/opentripplanner/raptor/_data/api/TestPathBuilderTestRaptor.java b/src/test/java/org/opentripplanner/raptor/_data/api/TestPathBuilderTestRaptor.java index bc11f32e4cc..de40c29d583 100644 --- a/src/test/java/org/opentripplanner/raptor/_data/api/TestPathBuilderTestRaptor.java +++ b/src/test/java/org/opentripplanner/raptor/_data/api/TestPathBuilderTestRaptor.java @@ -1,6 +1,7 @@ package org.opentripplanner.raptor._data.api; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.framework.time.DurationUtils.durationInSeconds; import static org.opentripplanner.framework.time.TimeUtils.time; import static org.opentripplanner.model.transfer.TransferConstraint.REGULAR_TRANSFER; @@ -84,14 +85,12 @@ public void testBasicPath() { ) .egress(BasicPathTestCase.EGRESS_DURATION); - Assertions.assertEquals( - BasicPathTestCase.BASIC_PATH_AS_STRING, - path.toString(this::stopIndexToName) - ); - Assertions.assertEquals( + assertEquals(BasicPathTestCase.BASIC_PATH_AS_STRING, path.toString(this::stopIndexToName)); + assertEquals( BasicPathTestCase.BASIC_PATH_AS_DETAILED_STRING, path.toStringDetailed(this::stopIndexToName) ); - Assertions.assertEquals(BasicPathTestCase.TOTAL_C1, path.c1()); + assertEquals(BasicPathTestCase.TOTAL_C1, path.c1()); + assertTrue(path.isC2Set()); } } diff --git a/src/test/java/org/opentripplanner/raptor/moduletests/K01_TransitPriorityTest.java b/src/test/java/org/opentripplanner/raptor/moduletests/K01_TransitPriorityTest.java index be544f26a11..43351b06eb8 100644 --- a/src/test/java/org/opentripplanner/raptor/moduletests/K01_TransitPriorityTest.java +++ b/src/test/java/org/opentripplanner/raptor/moduletests/K01_TransitPriorityTest.java @@ -22,7 +22,7 @@ import org.opentripplanner.raptor.api.model.DominanceFunction; import org.opentripplanner.raptor.api.request.RaptorProfile; import org.opentripplanner.raptor.api.request.RaptorRequestBuilder; -import org.opentripplanner.raptor.api.request.RaptorTransitPriorityGroupCalculator; +import org.opentripplanner.raptor.api.request.RaptorTransitGroupCalculator; import org.opentripplanner.raptor.configure.RaptorConfig; import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter; @@ -33,9 +33,9 @@ */ public class K01_TransitPriorityTest { - private static final RaptorTransitPriorityGroupCalculator PRIORITY_GROUP_CALCULATOR = new RaptorTransitPriorityGroupCalculator() { + private static final RaptorTransitGroupCalculator PRIORITY_GROUP_CALCULATOR = new RaptorTransitGroupCalculator() { @Override - public int mergeTransitPriorityGroupIds(int currentGroupIds, int boardingGroupId) { + public int mergeGroupIds(int currentGroupIds, int boardingGroupId) { return currentGroupIds | boardingGroupId; } @@ -51,14 +51,8 @@ public DominanceFunction dominanceFunction() { 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 = PRIORITY_GROUP_CALCULATOR.mergeTransitPriorityGroupIds( - GROUP_A, - GROUP_B - ); - private static final int GROUP_AC = PRIORITY_GROUP_CALCULATOR.mergeTransitPriorityGroupIds( - GROUP_A, - GROUP_C - ); + private static final int GROUP_AB = PRIORITY_GROUP_CALCULATOR.mergeGroupIds(GROUP_A, GROUP_B); + private static final int GROUP_AC = PRIORITY_GROUP_CALCULATOR.mergeGroupIds(GROUP_A, GROUP_C); private static final int C1_SLACK_90s = RaptorCostConverter.toRaptorCost(90); private final TestTransitData data = new TestTransitData(); @@ -67,13 +61,20 @@ public DominanceFunction dominanceFunction() { RaptorConfig.defaultConfigForTest() ); + /** + * Each pattern departs at the same time, but arrives at different times. They may belong to + * different groups. Line U1 is not optimal, because it slower than L1 and is in the same + * group as L1. Given a slack on the cost equals to ~90s makes both L1 and L2 optimal (since + * they are in different groups), but not L3 (which is in its own group, but its cost is + * outside the range allowed by the slack). + */ @BeforeEach private void prepareRequest() { - // Each pattern depart at the same time, but arrive with 60s between them. - // Given a slack on the cost equals to ~90s make both L1 and L2 optimal, but no L3 data.withRoutes( route(pattern("L1", STOP_B, STOP_C).withPriorityGroup(GROUP_A)) .withTimetable(schedule("00:02 00:12")), + route(pattern("U1", STOP_B, STOP_C).withPriorityGroup(GROUP_A)) + .withTimetable(schedule("00:02 00:12:01")), route(pattern("L2", STOP_B, STOP_C).withPriorityGroup(GROUP_B)) .withTimetable(schedule("00:02 00:13")), route(pattern("L3", STOP_B, STOP_C).withPriorityGroup(GROUP_C)) diff --git a/src/test/java/org/opentripplanner/raptor/moduletests/K02_TransitPriorityDestinationTest.java b/src/test/java/org/opentripplanner/raptor/moduletests/K02_TransitPriorityDestinationTest.java new file mode 100644 index 00000000000..6954ac0d41a --- /dev/null +++ b/src/test/java/org/opentripplanner/raptor/moduletests/K02_TransitPriorityDestinationTest.java @@ -0,0 +1,144 @@ +package org.opentripplanner.raptor.moduletests; + +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.raptor._data.RaptorTestConstants.STOP_B; +import static org.opentripplanner.raptor._data.RaptorTestConstants.STOP_C; +import static org.opentripplanner.raptor._data.RaptorTestConstants.STOP_D; +import static org.opentripplanner.raptor._data.RaptorTestConstants.STOP_E; +import static org.opentripplanner.raptor._data.RaptorTestConstants.STOP_F; +import static org.opentripplanner.raptor._data.RaptorTestConstants.T00_00; +import static org.opentripplanner.raptor._data.RaptorTestConstants.T01_00; +import static org.opentripplanner.raptor._data.api.PathUtils.pathsToString; +import static org.opentripplanner.raptor._data.transit.TestAccessEgress.walk; +import static org.opentripplanner.raptor._data.transit.TestRoute.route; +import static org.opentripplanner.raptor._data.transit.TestTripPattern.pattern; +import static org.opentripplanner.raptor._data.transit.TestTripSchedule.schedule; + +import java.time.Duration; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.opentripplanner.raptor.RaptorService; +import org.opentripplanner.raptor._data.transit.TestTransitData; +import org.opentripplanner.raptor._data.transit.TestTripSchedule; +import org.opentripplanner.raptor.api.model.DominanceFunction; +import org.opentripplanner.raptor.api.request.RaptorProfile; +import org.opentripplanner.raptor.api.request.RaptorRequestBuilder; +import org.opentripplanner.raptor.api.request.RaptorTransitGroupCalculator; +import org.opentripplanner.raptor.configure.RaptorConfig; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter; + +/** + * FEATURE UNDER TEST + * + * Raptor should be able to handle route request with transit-priority. + */ +public class K02_TransitPriorityDestinationTest { + + private static final RaptorTransitGroupCalculator PRIORITY_GROUP_CALCULATOR = new RaptorTransitGroupCalculator() { + @Override + public int mergeGroupIds(int currentGroupIds, int boardingGroupId) { + return currentGroupIds | boardingGroupId; + } + + /** + * Left dominate right, if right has at least one priority group not in left. + */ + @Override + public DominanceFunction dominanceFunction() { + return (l, r) -> ((l ^ r) & r) != 0; + } + }; + + 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 = PRIORITY_GROUP_CALCULATOR.mergeGroupIds(GROUP_A, GROUP_B); + private static final int GROUP_AC = PRIORITY_GROUP_CALCULATOR.mergeGroupIds(GROUP_A, GROUP_C); + private static final int C1_SLACK_90s = RaptorCostConverter.toRaptorCost(90); + + private final TestTransitData data = new TestTransitData(); + private final RaptorRequestBuilder requestBuilder = new RaptorRequestBuilder<>(); + private final RaptorService raptorService = new RaptorService<>( + RaptorConfig.defaultConfigForTest() + ); + + @BeforeEach + private void prepareRequest() { + // Each pattern depart at the same time, but arrive at different times and they may + // belong to different groups. + // Line U1 is not optimal, because it slower than L1 and is in the same group. + // Given a slack on the cost equals to ~90s this makes both L1 and L2 optimal (since + // they are in different groups), but not L3 (which certainly is in its own group but + // its cost is outside the range allowed by the slack). + data.withRoutes( + route(pattern("L1", STOP_B, STOP_C).withPriorityGroup(GROUP_A)) + .withTimetable(schedule("00:02 00:12")), + route(pattern("U1", STOP_B, STOP_D).withPriorityGroup(GROUP_A)) + .withTimetable(schedule("00:02 00:12:01")), + route(pattern("L2", STOP_B, STOP_E).withPriorityGroup(GROUP_B)) + .withTimetable(schedule("00:02 00:13")), + route(pattern("L3", STOP_B, STOP_F).withPriorityGroup(GROUP_C)) + .withTimetable(schedule("00:02 00:14")) + ); + + requestBuilder + .profile(RaptorProfile.MULTI_CRITERIA) + // TODO: 2023-07-24 Currently heuristics does not work with pass-through so we + // have to turn them off. Make sure to re-enable optimization later when it's fixed + .clearOptimizations(); + + requestBuilder + .searchParams() + .earliestDepartureTime(T00_00) + .latestArrivalTime(T01_00) + .searchWindow(Duration.ofMinutes(2)) + .timetable(true); + + requestBuilder.withMultiCriteria(mc -> + // Raptor cost 9000 ~= 90 seconds slack + mc + .withRelaxC1(value -> value + C1_SLACK_90s) + .withTransitPriorityCalculator(PRIORITY_GROUP_CALCULATOR) + ); + // Add 1 second access/egress paths + requestBuilder + .searchParams() + .addAccessPaths(walk(STOP_B, 1)) + .addEgressPaths(walk(STOP_C, 1)) + .addEgressPaths(walk(STOP_D, 1)) + .addEgressPaths(walk(STOP_E, 1)) + .addEgressPaths(walk(STOP_F, 1)); + assetGroupCalculatorIsSetupCorrect(); + } + + @Test + public void transitPriority() { + // We expect L1 & L2 but not L3, since the cost of L3 is > $90.00. + assertEquals( + """ + Walk 1s ~ B ~ BUS L1 0:02 0:12 ~ C ~ Walk 1s [0:01:59 0:12:01 10m2s Tₓ0 C₁1_204 C₂1] + Walk 1s ~ B ~ BUS L2 0:02 0:13 ~ E ~ Walk 1s [0:01:59 0:13:01 11m2s Tₓ0 C₁1_264 C₂2] + """.trim(), + pathsToString(raptorService.route(requestBuilder.build(), data)) + ); + } + + /** + * Make sure the calculator and group setup is done correct. + */ + void assetGroupCalculatorIsSetupCorrect() { + var d = PRIORITY_GROUP_CALCULATOR.dominanceFunction(); + + assertTrue(d.leftDominateRight(GROUP_A, GROUP_B)); + assertTrue(d.leftDominateRight(GROUP_B, GROUP_A)); + assertFalse(d.leftDominateRight(GROUP_A, GROUP_A)); + // 3 = 1&2, 5 = 1&4 + assertTrue(d.leftDominateRight(GROUP_A, GROUP_AB)); + assertFalse(d.leftDominateRight(GROUP_AB, GROUP_A)); + assertFalse(d.leftDominateRight(GROUP_AB, GROUP_AB)); + assertTrue(d.leftDominateRight(GROUP_AB, GROUP_AC)); + assertTrue(d.leftDominateRight(GROUP_AC, GROUP_AB)); + } +} diff --git a/src/test/java/org/opentripplanner/raptor/rangeraptor/context/SearchContextTest.java b/src/test/java/org/opentripplanner/raptor/rangeraptor/context/SearchContextTest.java index 8ac0af5b3a8..4ffa4ed1cd9 100644 --- a/src/test/java/org/opentripplanner/raptor/rangeraptor/context/SearchContextTest.java +++ b/src/test/java/org/opentripplanner/raptor/rangeraptor/context/SearchContextTest.java @@ -1,9 +1,14 @@ package org.opentripplanner.raptor.rangeraptor.context; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opentripplanner.raptor.api.model.SearchDirection.FORWARD; +import static org.opentripplanner.raptor.api.model.SearchDirection.REVERSE; import static org.opentripplanner.raptor.api.request.RaptorProfile.MULTI_CRITERIA; import static org.opentripplanner.raptor.api.request.RaptorProfile.STANDARD; import static org.opentripplanner.raptor.rangeraptor.context.SearchContext.accessOrEgressPaths; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_ARRIVAL_TIME; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_DEPARTURE_TIME; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_TIMETABLE; import java.util.Collection; import java.util.Comparator; @@ -11,14 +16,20 @@ import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.raptor._data.RaptorTestConstants; import org.opentripplanner.raptor._data.transit.TestAccessEgress; import org.opentripplanner.raptor._data.transit.TestTripSchedule; import org.opentripplanner.raptor.api.model.RaptorAccessEgress; +import org.opentripplanner.raptor.api.model.SearchDirection; import org.opentripplanner.raptor.api.request.RaptorRequestBuilder; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime; class SearchContextTest implements RaptorTestConstants { + public static final TestAccessEgress ANY_WALK = TestAccessEgress.walk(1, 1); private final boolean GET_ACCESS = true; private final boolean GET_EGRESS = false; private final RaptorAccessEgress PATH_A_10s = TestAccessEgress.walk(STOP_A, D10s); @@ -82,4 +93,43 @@ private static List sort(Collection c) { .sorted(Comparator.comparingInt(it -> it.stop() * 10_000 + it.durationInSeconds())) .collect(Collectors.toList()); } + + static List paretoSetTimeConfigTestCase() { + return List.of( + Arguments.of(USE_TIMETABLE, true, false, FORWARD), + Arguments.of(USE_TIMETABLE, true, false, REVERSE), + Arguments.of(USE_DEPARTURE_TIME, false, true, FORWARD), + Arguments.of(USE_DEPARTURE_TIME, false, false, REVERSE), + Arguments.of(USE_ARRIVAL_TIME, false, true, REVERSE), + Arguments.of(USE_ARRIVAL_TIME, false, false, FORWARD) + ); + } + + @ParameterizedTest + @MethodSource("paretoSetTimeConfigTestCase") + void testParetoSetTimeConfig( + ParetoSetTime expected, + boolean timetable, + boolean prefLateArrival, + SearchDirection searchDirection + ) { + assertEquals( + expected, + SearchContext.paretoSetTimeConfig( + new RaptorRequestBuilder() + .searchParams() + // EDT, LAT, access and egress is required, but not used + .addAccessPaths(ANY_WALK) + .addEgressPaths(ANY_WALK) + .earliestDepartureTime(12) + .latestArrivalTime(120) + // Relevant parameters + .timetable(timetable) + .preferLateArrival(prefLateArrival) + .build() + .searchParams(), + searchDirection + ) + ); + } } diff --git a/src/test/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparatorsTest.java b/src/test/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparatorsTest.java index d8d5ddfb472..296103499a1 100644 --- a/src/test/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparatorsTest.java +++ b/src/test/java/org/opentripplanner/raptor/rangeraptor/path/PathParetoSetComparatorsTest.java @@ -2,8 +2,20 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.junit.jupiter.api.Test; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost.NONE; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost.USE_C1; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost.USE_C1_AND_C2; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost.USE_C1_RELAXED_IF_C2_IS_OPTIMAL; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost.USE_C1_RELAX_DESTINATION; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_ARRIVAL_TIME; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_DEPARTURE_TIME; +import static org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime.USE_TIMETABLE; +import static org.opentripplanner.raptor.rangeraptor.path.PathParetoSetComparators.paretoComparator; + +import java.util.List; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.raptor._data.api.TestRaptorPath; import org.opentripplanner.raptor.api.model.DominanceFunction; import org.opentripplanner.raptor.api.model.GeneralizedCostRelaxFunction; @@ -11,325 +23,93 @@ import org.opentripplanner.raptor.api.model.RelaxFunction; import org.opentripplanner.raptor.api.model.SearchDirection; import org.opentripplanner.raptor.api.path.RaptorPath; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetCost; +import org.opentripplanner.raptor.rangeraptor.internalapi.ParetoSetTime; import org.opentripplanner.raptor.util.paretoset.ParetoComparator; public class PathParetoSetComparatorsTest { - final int BIG_VALUE = 999; - final int SMALL_VALUE = 100; - final int NO_VALUE = 0; - - private final DominanceFunction DOMINANCE_FUNCTION = (left, right) -> left > right; - private final RelaxFunction RELAX_FUNCTION = GeneralizedCostRelaxFunction.of(1.5, 0); - - @Test - public void testComparatorStandardArrivalTime() { - var comparator = PathParetoSetComparators.paretoComparator( - false, - false, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - null - ); - - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - } - - @Test - public void testComparatorStandardArrivalTimeAndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - false, - false, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - DOMINANCE_FUNCTION - ); - - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyC2Comparator(comparator); - } - - @Test - public void testComparatorStandardDepartureTime() { - var comparator = PathParetoSetComparators.paretoComparator( - false, - false, - true, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - null - ); - - verifyStartTimeComparator(comparator); - - verifyNumberOfTransfers(comparator); - } - - @Test - public void testComparatorStandardDepartureTimeAndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - false, - false, - true, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - DOMINANCE_FUNCTION - ); - - verifyStartTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyC2Comparator(comparator); - } - - @Test - public void testComparatorTimetable() { - var comparator = PathParetoSetComparators.paretoComparator( - false, - true, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - null - ); - - verifyRangeRaptorIterationDepartureTime(comparator); - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - } - - @Test - public void testComparatorTimetableAndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - false, - true, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - DOMINANCE_FUNCTION - ); - - verifyRangeRaptorIterationDepartureTime(comparator); - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyC2Comparator(comparator); - } - - @Test - public void testComparatorTimetableAndC1() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - true, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - null - ); - - verifyRangeRaptorIterationDepartureTime(comparator); - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyC1Comparator(comparator); - } - - @Test - public void testComparatorTimetableAndRelaxedC1() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - true, - false, - SearchDirection.FORWARD, - RELAX_FUNCTION, - null - ); - - verifyRangeRaptorIterationDepartureTime(comparator); - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyRelaxedC1Comparator(comparator); - } - - @Test - public void testComparatorWithC1() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - null - ); - - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyC1Comparator(comparator); - } - - @Test - public void testComparatorDepartureTimeAndC1() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - true, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - null - ); - - verifyStartTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyC1Comparator(comparator); - } - - @Test - public void comparatorArrivalTimeAndRelaxedC1() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - false, - SearchDirection.FORWARD, - RELAX_FUNCTION, - null - ); - - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyRelaxedC1Comparator(comparator); - } - - @Test - public void testComparatorDepartureTimeAndRelaxedC1() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - true, - SearchDirection.FORWARD, - RELAX_FUNCTION, - null - ); - - verifyStartTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyRelaxedC1Comparator(comparator); - } - - @Test - public void testComparatorTimetableAndC1AndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - true, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - DOMINANCE_FUNCTION - ); - - verifyRangeRaptorIterationDepartureTime(comparator); - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyC1Comparator(comparator); - verifyC2Comparator(comparator); - } - - @Test - public void testComparatorTimetableAndRelaxedC1AndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - true, - false, - SearchDirection.FORWARD, - RELAX_FUNCTION, - DOMINANCE_FUNCTION + private static final int ANY = 0; + private static final int NORMAL = 100; + private static final int SMALL = 50; + private static final int LARGE = 999; + private static final int RELAXED = 140; + private static final int RELAXED_DELTA_LIMIT = 50; + + private static final DominanceFunction DOMINANCE_FN = (left, right) -> left < right; + private static final DominanceFunction NO_COMP = null; + + private static final RelaxFunction RELAX_FN = GeneralizedCostRelaxFunction.of( + 1.0, + RELAXED_DELTA_LIMIT + ); + private static final RelaxFunction NO_RELAX_FN = RelaxFunction.NORMAL; + + static List testCases() { + return List.of( + // Arrival time + Arguments.of(USE_ARRIVAL_TIME, NONE, NO_RELAX_FN, NO_COMP), + Arguments.of(USE_ARRIVAL_TIME, USE_C1, NO_RELAX_FN, NO_COMP), + Arguments.of(USE_ARRIVAL_TIME, USE_C1_AND_C2, NO_RELAX_FN, DOMINANCE_FN), + // TODO: This is failing, error in implementation + Arguments.of(USE_ARRIVAL_TIME, USE_C1_RELAXED_IF_C2_IS_OPTIMAL, RELAX_FN, DOMINANCE_FN), + Arguments.of(USE_ARRIVAL_TIME, USE_C1_RELAX_DESTINATION, RELAX_FN, NO_COMP), + // Departure time + Arguments.of(USE_DEPARTURE_TIME, NONE, NO_RELAX_FN, NO_COMP), + Arguments.of(USE_DEPARTURE_TIME, USE_C1_AND_C2, NO_RELAX_FN, DOMINANCE_FN), + // TODO: This is failing, error in implementation + Arguments.of(USE_DEPARTURE_TIME, USE_C1_RELAXED_IF_C2_IS_OPTIMAL, RELAX_FN, DOMINANCE_FN), + Arguments.of(USE_DEPARTURE_TIME, USE_C1_RELAX_DESTINATION, RELAX_FN, NO_COMP), + // Timetable + Arguments.of(USE_TIMETABLE, NONE, NO_RELAX_FN, NO_COMP), + Arguments.of(USE_TIMETABLE, USE_C1_AND_C2, NO_RELAX_FN, DOMINANCE_FN), + // TODO: This is failing, error in implementation + Arguments.of(USE_TIMETABLE, USE_C1_RELAXED_IF_C2_IS_OPTIMAL, RELAX_FN, DOMINANCE_FN), + Arguments.of(USE_TIMETABLE, USE_C1_RELAX_DESTINATION, RELAX_FN, NO_COMP) ); - - verifyRangeRaptorIterationDepartureTime(comparator); - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyRelaxedC1Comparator(comparator); - verifyC2Comparator(comparator); - } - - @Test - public void testComparatorWithC1AndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - false, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - DOMINANCE_FUNCTION - ); - - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyC1Comparator(comparator); - verifyC2Comparator(comparator); } - @Test - public void testComparatorDepartureTimeAndC1AndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - true, - SearchDirection.FORWARD, - RelaxFunction.NORMAL, - DOMINANCE_FUNCTION - ); - - verifyStartTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyC1Comparator(comparator); - verifyC2Comparator(comparator); - } - - @Test - public void testComparatorArrivalTimeAndRelaxedC1AndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - false, - SearchDirection.FORWARD, - RELAX_FUNCTION, - DOMINANCE_FUNCTION - ); - - verifyEndTimeComparator(comparator); - verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyRelaxedC1Comparator(comparator); - verifyC2Comparator(comparator); - } - - @Test - public void testComparatorDepartureTimeAndRelaxedC1AndC2() { - var comparator = PathParetoSetComparators.paretoComparator( - true, - false, - true, - SearchDirection.FORWARD, - RELAX_FUNCTION, - DOMINANCE_FUNCTION - ); - - verifyStartTimeComparator(comparator); + @ParameterizedTest + @MethodSource("testCases") + public void testComparator( + ParetoSetTime time, + ParetoSetCost cost, + RelaxFunction relaxC1, + DominanceFunction comp2 + ) { + var comparator = paretoComparator(time, cost, relaxC1, comp2); verifyNumberOfTransfers(comparator); - verifyDurationComparator(comparator); - verifyRelaxedC1Comparator(comparator); - verifyC2Comparator(comparator); + switch (time) { + case USE_ARRIVAL_TIME: + verifyEndTimeComparator(comparator); + break; + case USE_DEPARTURE_TIME: + verifyStartTimeComparator(comparator); + break; + case USE_TIMETABLE: + verifyIterationDepartureTime(comparator); + verifyEndTimeComparator(comparator); + break; + } + + switch (cost) { + case USE_C1: + verifyC1Comparator(comparator); + break; + case USE_C1_AND_C2: + verifyC1Comparator(comparator); + verifyC2Comparator(comparator); + break; + case USE_C1_RELAXED_IF_C2_IS_OPTIMAL: + verifyRelaxedC1IfC2Optimal(comparator); + break; + case USE_C1_RELAX_DESTINATION: + verifyRelaxedC1Comparator(comparator); + break; + case NONE: + default: + break; + } } /** @@ -340,14 +120,14 @@ private void verifyStartTimeComparator( ) { assertTrue( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, NORMAL, ANY, ANY, ANY, LARGE, ANY), + new TestRaptorPath(ANY, SMALL, ANY, ANY, ANY, ANY, ANY) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, NORMAL, ANY, ANY, ANY, LARGE, ANY), + new TestRaptorPath(ANY, LARGE, ANY, ANY, ANY, ANY, ANY) ) ); } @@ -355,20 +135,20 @@ private void verifyStartTimeComparator( /** * Verify that bigger rangeRaptorIterationDepartureTime always wins */ - private void verifyRangeRaptorIterationDepartureTime( + private void verifyIterationDepartureTime( ParetoComparator> comparator ) { // Verify that bigger rangeRaptorIterationDepartureTime always wins assertTrue( comparator.leftDominanceExist( - new TestRaptorPath(BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(NORMAL, ANY, ANY, ANY, ANY, LARGE, ANY), + new TestRaptorPath(SMALL, ANY, ANY, ANY, ANY, ANY, ANY) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath(SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(NORMAL, ANY, ANY, ANY, ANY, LARGE, ANY), + new TestRaptorPath(LARGE, ANY, ANY, ANY, ANY, ANY, ANY) ) ); } @@ -382,14 +162,14 @@ private void verifyEndTimeComparator( // Verify that lower endTime always wins assertTrue( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, NORMAL, ANY, ANY, LARGE, ANY), + new TestRaptorPath(ANY, ANY, LARGE, ANY, ANY, SMALL, ANY) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, NORMAL, ANY, ANY, LARGE, ANY), + new TestRaptorPath(ANY, ANY, SMALL, ANY, ANY, SMALL, ANY) ) ); } @@ -402,14 +182,14 @@ private void verifyNumberOfTransfers( ) { assertTrue( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, NORMAL, LARGE, ANY), + new TestRaptorPath(ANY, ANY, ANY, ANY, LARGE, SMALL, ANY) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, NORMAL, LARGE, ANY), + new TestRaptorPath(ANY, ANY, ANY, ANY, SMALL, SMALL, ANY) ) ); } @@ -422,14 +202,14 @@ private void verifyDurationComparator( ) { assertTrue( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, NORMAL, ANY, ANY, ANY), + new TestRaptorPath(ANY, ANY, ANY, LARGE, ANY, ANY, ANY) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE, NO_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE, NO_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, NORMAL, ANY, ANY, ANY), + new TestRaptorPath(ANY, ANY, ANY, SMALL, ANY, ANY, ANY) ) ); } @@ -440,14 +220,14 @@ private void verifyDurationComparator( private void verifyC1Comparator(ParetoComparator> comparator) { assertTrue( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, ANY), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, LARGE, ANY) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE, NO_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, ANY), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, SMALL, ANY) ) ); } @@ -460,34 +240,63 @@ private void verifyRelaxedC1Comparator( ) { assertTrue( comparator.leftDominanceExist( - new TestRaptorPath( - NO_VALUE, - NO_VALUE, - NO_VALUE, - NO_VALUE, - NO_VALUE, - (int) (SMALL_VALUE * (1.4)), - NO_VALUE - ), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, RELAXED, ANY), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, ANY) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath( - NO_VALUE, - NO_VALUE, - NO_VALUE, - NO_VALUE, - NO_VALUE, - (int) (SMALL_VALUE * (1.6)), - NO_VALUE - ), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE, NO_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, LARGE, ANY), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, ANY) ) ); } + /** + * Verify that relax function is used in the c1 comparator, if and only if c2 is optimal. + */ + private void verifyRelaxedC1IfC2Optimal( + ParetoComparator> comparator + ) { + assertTrue( + comparator.leftDominanceExist( + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, SMALL, NORMAL), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, NORMAL) + ), + "c1 is optimal, c2 is not => path is optimal" + ); + assertTrue( + comparator.leftDominanceExist( + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, RELAXED, SMALL), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, NORMAL) + ), + "c2 is optimal, c1 is within relaxed limit => path is optimal" + ); + assertFalse( + comparator.leftDominanceExist( + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, RELAXED, ANY), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, ANY) + ), + "c2 is not optimal, c1 is within relaxed limit => path is NOT optimal" + ); + assertFalse( + // c2 is optimal, but c1 is not within relaxed limit + comparator.leftDominanceExist( + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, LARGE, SMALL), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, NORMAL) + ), + "c2 is optimal, c1 is not within relaxed limit => path is NOT optimal" + ); + assertFalse( + // c1 and c2 is not optimal (they are equal) + comparator.leftDominanceExist( + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, NORMAL), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, NORMAL, NORMAL) + ), + "c1 and c2 is not optimal" + ); + } + /** * Verify that dominance function is used in a comparator. This method operates on an assumption * that dominance function is l.c2 > r.c2 @@ -496,14 +305,14 @@ private void verifyC2Comparator(ParetoComparator> // Verify that dominance function is used assertTrue( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, ANY, SMALL), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, ANY, NORMAL) ) ); assertFalse( comparator.leftDominanceExist( - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, SMALL_VALUE), - new TestRaptorPath(NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, NO_VALUE, BIG_VALUE) + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, ANY, LARGE), + new TestRaptorPath(ANY, ANY, ANY, ANY, ANY, ANY, NORMAL) ) ); } diff --git a/src/test/java/org/opentripplanner/raptor/util/PathStringBuilderTest.java b/src/test/java/org/opentripplanner/raptor/util/PathStringBuilderTest.java index d347f3848e4..771efe6ab03 100644 --- a/src/test/java/org/opentripplanner/raptor/util/PathStringBuilderTest.java +++ b/src/test/java/org/opentripplanner/raptor/util/PathStringBuilderTest.java @@ -71,7 +71,7 @@ public void summary() { @Test public void summaryGeneralizedCostOnly() { - assertEquals("[C₁0.01]", subject.summary(1).toString()); + assertEquals("[C₁0.01 C₂7]", subject.summary(1, 7).toString()); } @Test diff --git a/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitPriorityGroup32nTest.java b/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitGroupPriority32nTest.java similarity index 51% rename from src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitPriorityGroup32nTest.java rename to src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitGroupPriority32nTest.java index 85083a3ee6a..2713a190dbf 100644 --- a/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitPriorityGroup32nTest.java +++ b/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/cost/grouppriority/TransitGroupPriority32nTest.java @@ -6,9 +6,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Test; -import org.opentripplanner.raptor.api.request.RaptorTransitPriorityGroupCalculator; +import org.opentripplanner.raptor.api.request.RaptorTransitGroupCalculator; -class TransitPriorityGroup32nTest { +class TransitGroupPriority32nTest { private static final int GROUP_INDEX_0 = 0; private static final int GROUP_INDEX_1 = 1; @@ -16,35 +16,35 @@ class TransitPriorityGroup32nTest { private static final int GROUP_INDEX_30 = 30; private static final int GROUP_INDEX_31 = 31; - private static final int GROUP_0 = TransitPriorityGroup32n.groupId(GROUP_INDEX_0); - private static final int GROUP_1 = TransitPriorityGroup32n.groupId(GROUP_INDEX_1); - private static final int GROUP_2 = TransitPriorityGroup32n.groupId(GROUP_INDEX_2); - private static final int GROUP_30 = TransitPriorityGroup32n.groupId(GROUP_INDEX_30); - private static final int GROUP_31 = TransitPriorityGroup32n.groupId(GROUP_INDEX_31); - private static final RaptorTransitPriorityGroupCalculator subjct = TransitPriorityGroup32n.priorityCalculator(); + private static final int GROUP_0 = TransitGroupPriority32n.groupId(GROUP_INDEX_0); + private static final int GROUP_1 = TransitGroupPriority32n.groupId(GROUP_INDEX_1); + private static final int GROUP_2 = TransitGroupPriority32n.groupId(GROUP_INDEX_2); + private static final int GROUP_30 = TransitGroupPriority32n.groupId(GROUP_INDEX_30); + private static final int GROUP_31 = TransitGroupPriority32n.groupId(GROUP_INDEX_31); + private static final RaptorTransitGroupCalculator subjct = TransitGroupPriority32n.priorityCalculator(); @Test void groupId() { - assertEqualsHex(0x00_00_00_00, TransitPriorityGroup32n.groupId(0)); - assertEqualsHex(0x00_00_00_01, TransitPriorityGroup32n.groupId(1)); - assertEqualsHex(0x00_00_00_02, TransitPriorityGroup32n.groupId(2)); - assertEqualsHex(0x00_00_00_04, TransitPriorityGroup32n.groupId(3)); - assertEqualsHex(0x40_00_00_00, TransitPriorityGroup32n.groupId(31)); - assertEqualsHex(0x80_00_00_00, TransitPriorityGroup32n.groupId(32)); + assertEqualsHex(0x00_00_00_00, TransitGroupPriority32n.groupId(0)); + assertEqualsHex(0x00_00_00_01, TransitGroupPriority32n.groupId(1)); + assertEqualsHex(0x00_00_00_02, TransitGroupPriority32n.groupId(2)); + assertEqualsHex(0x00_00_00_04, TransitGroupPriority32n.groupId(3)); + assertEqualsHex(0x40_00_00_00, TransitGroupPriority32n.groupId(31)); + assertEqualsHex(0x80_00_00_00, TransitGroupPriority32n.groupId(32)); - assertThrows(IllegalArgumentException.class, () -> TransitPriorityGroup32n.groupId(-1)); - assertThrows(IllegalArgumentException.class, () -> TransitPriorityGroup32n.groupId(33)); + assertThrows(IllegalArgumentException.class, () -> TransitGroupPriority32n.groupId(-1)); + assertThrows(IllegalArgumentException.class, () -> TransitGroupPriority32n.groupId(33)); } @Test - void mergeTransitPriorityGroupIds() { - assertEqualsHex(GROUP_0, subjct.mergeTransitPriorityGroupIds(GROUP_0, GROUP_0)); - assertEqualsHex(GROUP_1, subjct.mergeTransitPriorityGroupIds(GROUP_1, GROUP_1)); - assertEqualsHex(GROUP_0 | GROUP_1, subjct.mergeTransitPriorityGroupIds(GROUP_0, GROUP_1)); - assertEqualsHex(GROUP_30 | GROUP_31, subjct.mergeTransitPriorityGroupIds(GROUP_30, GROUP_31)); + void mergeTransitGroupPriorityIds() { + assertEqualsHex(GROUP_0, subjct.mergeGroupIds(GROUP_0, GROUP_0)); + assertEqualsHex(GROUP_1, subjct.mergeGroupIds(GROUP_1, GROUP_1)); + assertEqualsHex(GROUP_0 | GROUP_1, subjct.mergeGroupIds(GROUP_0, GROUP_1)); + assertEqualsHex(GROUP_30 | GROUP_31, subjct.mergeGroupIds(GROUP_30, GROUP_31)); assertEqualsHex( GROUP_0 | GROUP_1 | GROUP_2 | GROUP_30 | GROUP_31, - subjct.mergeTransitPriorityGroupIds(GROUP_0 | GROUP_1 | GROUP_2 | GROUP_30, GROUP_31) + subjct.mergeGroupIds(GROUP_0 | GROUP_1 | GROUP_2 | GROUP_30, GROUP_31) ); } diff --git a/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfiguratorTest.java b/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfiguratorTest.java index 4cd2b65e8c3..cc4bb09f01e 100644 --- a/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfiguratorTest.java +++ b/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupConfiguratorTest.java @@ -7,81 +7,122 @@ import java.util.List; import org.junit.jupiter.api.Test; -import org.opentripplanner.routing.api.request.request.filter.TransitPriorityGroupSelect; +import org.opentripplanner.routing.api.request.request.filter.TransitGroupSelect; import org.opentripplanner.transit.model.basic.TransitMode; import org.opentripplanner.transit.model.network.RoutingTripPattern; +import org.opentripplanner.transit.model.site.RegularStop; class PriorityGroupConfiguratorTest { - private final TestRouteData routeA = TestRouteData.of( + private static final String AGENCY_A1 = "A1"; + private static final String AGENCY_A2 = "A2"; + private static final String AGENCY_A3 = "A3"; + + private static final int EXP_GROUP_ID_BASE = 1; + private static final int EXP_GROUP_1 = 2; + private static final int EXP_GROUP_2 = 4; + private static final int EXP_GROUP_3 = 8; + + private final TestRouteData routeR1 = route( "R1", TransitMode.RAIL, + AGENCY_A1, List.of(STOP_A, STOP_B), "10:00 10:10" ); - private final TestRouteData routeB = TestRouteData.of( + + private final TestRouteData routeB2 = route( "B2", TransitMode.BUS, + AGENCY_A2, List.of(STOP_B, STOP_D), "10:15 10:40" ); - private final TestRouteData routeC = TestRouteData.of( + private final TestRouteData routeR3 = route( "R3", TransitMode.RAIL, + AGENCY_A3, List.of(STOP_A, STOP_B), "10:00 10:10" ); - private final TestRouteData routeD = TestRouteData.of( - "R3", + private final TestRouteData routeF3 = route( + "F3", TransitMode.FERRY, + AGENCY_A3, + List.of(STOP_A, STOP_B), + "10:00 10:10" + ); + private final TestRouteData routeB3 = route( + "B3", + TransitMode.BUS, + AGENCY_A3, List.of(STOP_A, STOP_B), "10:00 10:10" ); - private final RoutingTripPattern railA = routeA.getTripPattern().getRoutingTripPattern(); - private final RoutingTripPattern busB = routeB.getTripPattern().getRoutingTripPattern(); - private final RoutingTripPattern railC = routeC.getTripPattern().getRoutingTripPattern(); - private final RoutingTripPattern ferryC = routeD.getTripPattern().getRoutingTripPattern(); + private final RoutingTripPattern railR1 = routeR1.getTripPattern().getRoutingTripPattern(); + private final RoutingTripPattern busB2 = routeB2.getTripPattern().getRoutingTripPattern(); + private final RoutingTripPattern railR3 = routeR3.getTripPattern().getRoutingTripPattern(); + private final RoutingTripPattern ferryF3 = routeF3.getTripPattern().getRoutingTripPattern(); + private final RoutingTripPattern busB3 = routeB3.getTripPattern().getRoutingTripPattern(); @Test void emptyConfigurationShouldReturnGroupZero() { var subject = PriorityGroupConfigurator.of(List.of(), List.of()); - assertEquals(0, subject.lookupTransitPriorityGroupId(railA)); - assertEquals(0, subject.lookupTransitPriorityGroupId(busB)); - assertEquals(0, subject.lookupTransitPriorityGroupId(null)); + assertEquals(subject.baseGroupId(), subject.lookupTransitGroupPriorityId(railR1)); + assertEquals(subject.baseGroupId(), subject.lookupTransitGroupPriorityId(busB2)); + assertEquals(subject.baseGroupId(), subject.lookupTransitGroupPriorityId(null)); } @Test - void lookupTransitPriorityGroupIdBySameAgency() { - var subject = PriorityGroupConfigurator.of( - List.of( - TransitPriorityGroupSelect.of().addModes(List.of(TransitMode.BUS)).build(), - TransitPriorityGroupSelect.of().addModes(List.of(TransitMode.RAIL)).build() - ), - List.of() - ); + void lookupTransitGroupIdByAgency() { + var select = TransitGroupSelect + .of() + .addModes(List.of(TransitMode.BUS, TransitMode.RAIL)) + .build(); - assertEquals(0, subject.lookupTransitPriorityGroupId(null)); - assertEquals(0, subject.lookupTransitPriorityGroupId(ferryC)); - assertEquals(1, subject.lookupTransitPriorityGroupId(railA)); - assertEquals(2, subject.lookupTransitPriorityGroupId(busB)); - assertEquals(1, subject.lookupTransitPriorityGroupId(railC)); + // Add matcher `byAgency` for bus and real + var subject = PriorityGroupConfigurator.of(List.of(select), List.of()); + + // Agency groups are indexed (group-id set) at request time + assertEquals(EXP_GROUP_ID_BASE, subject.lookupTransitGroupPriorityId(null)); + assertEquals(EXP_GROUP_1, subject.lookupTransitGroupPriorityId(busB2)); + assertEquals(EXP_GROUP_2, subject.lookupTransitGroupPriorityId(railR3)); + assertEquals(EXP_GROUP_3, subject.lookupTransitGroupPriorityId(railR1)); + assertEquals(EXP_GROUP_2, subject.lookupTransitGroupPriorityId(busB3)); + assertEquals(EXP_GROUP_ID_BASE, subject.lookupTransitGroupPriorityId(ferryF3)); } @Test void lookupTransitPriorityGroupIdByGlobalMode() { + // Global groups are indexed (group-id set) at construction time var subject = PriorityGroupConfigurator.of( List.of(), List.of( - TransitPriorityGroupSelect.of().addModes(List.of(TransitMode.BUS)).build(), - TransitPriorityGroupSelect.of().addModes(List.of(TransitMode.RAIL)).build() + TransitGroupSelect.of().addModes(List.of(TransitMode.BUS)).build(), + TransitGroupSelect.of().addModes(List.of(TransitMode.RAIL)).build() ) ); - assertEquals(0, subject.lookupTransitPriorityGroupId(null)); - assertEquals(0, subject.lookupTransitPriorityGroupId(ferryC)); - assertEquals(2, subject.lookupTransitPriorityGroupId(railA)); - assertEquals(1, subject.lookupTransitPriorityGroupId(busB)); - assertEquals(2, subject.lookupTransitPriorityGroupId(railC)); + assertEquals(EXP_GROUP_ID_BASE, subject.lookupTransitGroupPriorityId(null)); + assertEquals(EXP_GROUP_2, subject.lookupTransitGroupPriorityId(railR1)); + assertEquals(EXP_GROUP_1, subject.lookupTransitGroupPriorityId(busB2)); + assertEquals(EXP_GROUP_2, subject.lookupTransitGroupPriorityId(railR3)); + assertEquals(EXP_GROUP_1, subject.lookupTransitGroupPriorityId(busB3)); + assertEquals(EXP_GROUP_ID_BASE, subject.lookupTransitGroupPriorityId(ferryF3)); + } + + private static TestRouteData route( + String route, + TransitMode mode, + String agency, + List stops, + String times + ) { + return new TestRouteData.Builder(route) + .withMode(mode) + .withAgency(agency) + .withStops(stops) + .build(); } } diff --git a/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcherTest.java b/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcherTest.java index 1205b6b2205..91d0142f9ef 100644 --- a/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcherTest.java +++ b/src/test/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/PriorityGroupMatcherTest.java @@ -6,14 +6,18 @@ import java.util.List; import org.junit.jupiter.api.Test; -import org.opentripplanner.routing.api.request.request.filter.TransitPriorityGroupSelect; +import org.opentripplanner.routing.api.request.request.filter.TransitGroupSelect; import org.opentripplanner.transit.model.basic.TransitMode; import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.network.TripPattern; class PriorityGroupMatcherTest { - private final TestRouteData r1 = TestRouteData.rail("R1").withAgency("A1").build(); + private final TestRouteData r1 = TestRouteData + .rail("R1") + .withSubmode("express") + .withAgency("A1") + .build(); private final TestRouteData b1 = TestRouteData.bus("B2").withAgency("A2").build(); private final TestRouteData f1 = TestRouteData .ferry("F1") @@ -31,7 +35,7 @@ class PriorityGroupMatcherTest { @Test void testMode() { var m = PriorityGroupMatcher.of( - TransitPriorityGroupSelect.of().addModes(List.of(TransitMode.BUS, TransitMode.TRAM)).build() + TransitGroupSelect.of().addModes(List.of(TransitMode.BUS, TransitMode.TRAM)).build() ); assertEquals("Mode(BUS | TRAM)", m.toString()); assertFalse(m.isEmpty()); @@ -42,17 +46,16 @@ void testMode() { @Test void testAgencyIds() { - var matchers = List.of( - PriorityGroupMatcher.of( - TransitPriorityGroupSelect.of().addAgencyIds(List.of(r1agencyId)).build() - ), - PriorityGroupMatcher.of( - TransitPriorityGroupSelect.of().addAgencyIds(List.of(r1agencyId, anyId)).build() - ) + var m1 = PriorityGroupMatcher.of( + TransitGroupSelect.of().addAgencyIds(List.of(r1agencyId)).build() + ); + var m2 = PriorityGroupMatcher.of( + TransitGroupSelect.of().addAgencyIds(List.of(r1agencyId, anyId)).build() ); + var matchers = List.of(m1, m2); - assertEquals("AgencyId(F:A1)", matchers.get(0).toString()); - assertEquals("AgencyId(F:A1 | F:ANY)", matchers.get(1).toString()); + assertEquals("AgencyId(F:A1)", m1.toString()); + assertEquals("AgencyId(F:A1 | F:ANY)", m2.toString()); for (PriorityGroupMatcher m : matchers) { assertFalse(m.isEmpty()); @@ -64,17 +67,16 @@ void testAgencyIds() { @Test void routeIds() { - var matchers = List.of( - PriorityGroupMatcher.of( - TransitPriorityGroupSelect.of().addRouteIds(List.of(r1routeId)).build() - ), - PriorityGroupMatcher.of( - TransitPriorityGroupSelect.of().addRouteIds(List.of(r1routeId, anyId)).build() - ) + var m1 = PriorityGroupMatcher.of( + TransitGroupSelect.of().addRouteIds(List.of(r1routeId)).build() ); + var m2 = PriorityGroupMatcher.of( + TransitGroupSelect.of().addRouteIds(List.of(r1routeId, anyId)).build() + ); + var matchers = List.of(m1, m2); - assertEquals("RouteId(F:R1)", matchers.get(0).toString()); - assertEquals("RouteId(F:R1 | F:ANY)", matchers.get(1).toString()); + assertEquals("RouteId(F:R1)", m1.toString()); + assertEquals("RouteId(F:R1 | F:ANY)", m2.toString()); for (PriorityGroupMatcher m : matchers) { assertFalse(m.isEmpty()); @@ -87,7 +89,7 @@ void routeIds() { @Test void testSubMode() { var subject = PriorityGroupMatcher.of( - TransitPriorityGroupSelect.of().addSubModeRegexp(List.of(".*local.*")).build() + TransitGroupSelect.of().addSubModeRegexp(List.of(".*local.*")).build() ); assertEquals("SubModeRegexp(.*local.*)", subject.toString()); @@ -98,10 +100,32 @@ void testSubMode() { assertFalse(subject.match(bus)); } + @Test + void testAnd() { + var subject = PriorityGroupMatcher.of( + TransitGroupSelect + .of() + .addSubModeRegexp(List.of("express")) + .addRouteIds(List.of(r1routeId)) + .addModes(List.of(TransitMode.RAIL, TransitMode.TRAM)) + .build() + ); + + assertEquals( + "(Mode(RAIL | TRAM) & SubModeRegexp(express) & RouteId(F:R1))", + subject.toString() + ); + + assertFalse(subject.isEmpty()); + assertTrue(subject.match(rail1)); + assertFalse(subject.match(ferry)); + assertFalse(subject.match(bus)); + } + @Test void testToString() { - var m = PriorityGroupMatcher.of( - TransitPriorityGroupSelect + var subject = PriorityGroupMatcher.of( + TransitGroupSelect .of() .addModes(List.of(TransitMode.BUS, TransitMode.TRAM)) .addAgencyIds(List.of(anyId, r1agencyId)) @@ -111,8 +135,8 @@ void testToString() { ); assertEquals( - "(Mode(BUS | TRAM) | SubModeRegexp(.*local.*) | AgencyId(F:A1 | F:ANY) | RouteId(F:R1))", - m.toString() + "(Mode(BUS | TRAM) & SubModeRegexp(.*local.*) & AgencyId(F:A1 | F:ANY) & RouteId(F:R1))", + subject.toString() ); } } diff --git a/src/test/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainServiceTest.java b/src/test/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainServiceTest.java index d05a4090b39..be927046c43 100644 --- a/src/test/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainServiceTest.java +++ b/src/test/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainServiceTest.java @@ -74,6 +74,7 @@ public void testTripWithoutTransfers() { var original = pathBuilder() .access(ITERATION_START_TIME, STOP_B, D1m) .bus(trip1, STOP_C) + .c2(345) .egress(D1m); var subject = subject(transfers, null); diff --git a/src/test/java/org/opentripplanner/routing/api/request/preference/TransitPreferencesTest.java b/src/test/java/org/opentripplanner/routing/api/request/preference/TransitPreferencesTest.java index 811c4a70b29..b77a7d9085b 100644 --- a/src/test/java/org/opentripplanner/routing/api/request/preference/TransitPreferencesTest.java +++ b/src/test/java/org/opentripplanner/routing/api/request/preference/TransitPreferencesTest.java @@ -42,7 +42,7 @@ class TransitPreferencesTest { .setUnpreferredCost(UNPREFERRED_COST) .withBoardSlack(b -> b.withDefault(D45s).with(TransitMode.AIRPLANE, D35m)) .withAlightSlack(b -> b.withDefault(D15s).with(TransitMode.AIRPLANE, D25m)) - .withTransitGroupPriorityGeneralizedCostSlack(TRANSIT_GROUP_PRIORITY_RELAX) + .withRelaxTransitGroupPriority(TRANSIT_GROUP_PRIORITY_RELAX) .setIgnoreRealtimeUpdates(IGNORE_REALTIME_UPDATES) .setIncludePlannedCancellations(INCLUDE_PLANNED_CANCELLATIONS) .setIncludeRealtimeCancellations(INCLUDE_REALTIME_CANCELLATIONS) @@ -77,8 +77,8 @@ void unpreferredCost() { } @Test - void relaxTransitPriorityGroup() { - assertEquals(TRANSIT_GROUP_PRIORITY_RELAX, subject.relaxTransitPriorityGroup()); + void relaxTransitGroupPriority() { + assertEquals(TRANSIT_GROUP_PRIORITY_RELAX, subject.relaxTransitGroupPriority()); } @Test @@ -125,7 +125,7 @@ void testToString() { "reluctanceForMode: {AIRPLANE=2.1}, " + "otherThanPreferredRoutesPenalty: $350, " + "unpreferredCost: 5m + 1.15 t, " + - "relaxTransitPriorityGroup: 5m + 1.50 t, " + + "relaxTransitGroupPriority: 5m + 1.50 t, " + "ignoreRealtimeUpdates, " + "includePlannedCancellations, " + "includeRealtimeCancellations, " +