From a618a01df457393c93e0f5c3565c93385bf57822 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Thu, 30 Jan 2025 14:06:26 +0200 Subject: [PATCH 1/3] Remove default accessegress penalty for car modes that use transit. --- .../api/request/preference/AccessEgressPreferences.java | 4 +++- .../org/opentripplanner/apis/transmodel/schema.graphql | 2 +- .../routing/api/request/preference/StreetPreferencesTest.java | 1 - doc/user/RouteRequest.md | 1 - 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java index 4a8c342275f..9c2f45f4e05 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java @@ -171,7 +171,9 @@ private static TimeAndCostPenaltyForEnum createDefaultCarPenalty() { // Add penalty to all car variants with access and/or egress. var carPenalty = TimeAndCostPenalty.of(TimePenalty.of(ofMinutes(20), 2f), 1.5); for (var it : StreetMode.values()) { - if (it.includesDriving() && (it.accessAllowed() || it.egressAllowed())) { + if ( + it.includesDriving() && (it.accessAllowed() || it.egressAllowed()) && !it.transferAllowed() + ) { penaltyBuilder.with(it, carPenalty); } } diff --git a/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql b/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql index 611f6e38156..0c107f1e5be 100644 --- a/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql +++ b/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql @@ -780,7 +780,7 @@ type QueryType { "Input type for executing a travel search for a trip between two locations. Returns trip patterns describing suggested alternatives for the trip." trip( "Time and cost penalty on access/egress modes." - accessEgressPenalty: [PenaltyForStreetMode!] = [{streetMode : car, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_park, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_pickup, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_rental, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : flexible, timePenalty : "10m + 1.30 t", costFactor : 1.3}], + accessEgressPenalty: [PenaltyForStreetMode!] = [{streetMode : car_park, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_pickup, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_rental, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : flexible, timePenalty : "10m + 1.30 t", costFactor : 1.3}], "The alightSlack is the minimum extra time after exiting a public transport vehicle. This is the default value used, if not overridden by the 'alightSlackList'." alightSlackDefault: Int = 0, "List of alightSlack for a given set of modes. Defaults: []" diff --git a/application/src/test/java/org/opentripplanner/routing/api/request/preference/StreetPreferencesTest.java b/application/src/test/java/org/opentripplanner/routing/api/request/preference/StreetPreferencesTest.java index 2f2bdd9a5b5..faf7fec9a3e 100644 --- a/application/src/test/java/org/opentripplanner/routing/api/request/preference/StreetPreferencesTest.java +++ b/application/src/test/java/org/opentripplanner/routing/api/request/preference/StreetPreferencesTest.java @@ -113,7 +113,6 @@ void testToString() { "elevator: ElevatorPreferences{boardTime: 2m}, " + "intersectionTraversalModel: CONSTANT, " + "accessEgress: AccessEgressPreferences{penalty: TimeAndCostPenaltyForEnum{" + - "CAR: (timePenalty: 20m + 2.0 t, costFactor: 1.50), " + "CAR_TO_PARK: " + CAR_TO_PARK_PENALTY + ", " + diff --git a/doc/user/RouteRequest.md b/doc/user/RouteRequest.md index ea7ced375ab..4b54c943dc1 100644 --- a/doc/user/RouteRequest.md +++ b/doc/user/RouteRequest.md @@ -460,7 +460,6 @@ performance will be better. The default values are -- `car` = (timePenalty: 20m + 2.0 t, costFactor: 1.50) - `car-to-park` = (timePenalty: 20m + 2.0 t, costFactor: 1.50) - `car-pickup` = (timePenalty: 20m + 2.0 t, costFactor: 1.50) - `car-rental` = (timePenalty: 20m + 2.0 t, costFactor: 1.50) From 561999253a4220f2d48419a8af4bedc3f2581d56 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Fri, 31 Jan 2025 12:52:07 +0200 Subject: [PATCH 2/3] Change if statement and comment. --- .../api/request/preference/AccessEgressPreferences.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java index 9c2f45f4e05..06189402130 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java @@ -168,11 +168,15 @@ private static TimeAndCostPenaltyForEnum createDefaultCarPenalty() { var flexDefaultPenalty = TimeAndCostPenalty.of(TimePenalty.of(ofMinutes(10), 1.3f), 1.3); penaltyBuilder.with(StreetMode.FLEXIBLE, flexDefaultPenalty); - // Add penalty to all car variants with access and/or egress. var carPenalty = TimeAndCostPenalty.of(TimePenalty.of(ofMinutes(20), 2f), 1.5); for (var it : StreetMode.values()) { + // Apply car-penalty to all car modes allowed in access/egress only. Exclude car modes(CAR) used + // in direct street routing and car modes used when you bring the car with you onto transit. This is + // a bit limited and will not work if we combine car access/egress modes like CAR_TO_PARK with CAR + // in the same search. This is currently not possible, but if we enable this in the future this logic must be + // looked at again. if ( - it.includesDriving() && (it.accessAllowed() || it.egressAllowed()) && !it.transferAllowed() + it.includesDriving() && (it.accessAllowed() || it.egressAllowed()) && it != StreetMode.CAR ) { penaltyBuilder.with(it, carPenalty); } From 53eeb8a6767262a1414f090ca8b499c1f8a67104 Mon Sep 17 00:00:00 2001 From: Ville Pihlava Date: Tue, 4 Feb 2025 15:21:03 +0200 Subject: [PATCH 3/3] Change comment. --- .../request/preference/AccessEgressPreferences.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java index 06189402130..46419924fa8 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java @@ -170,11 +170,11 @@ private static TimeAndCostPenaltyForEnum createDefaultCarPenalty() { var carPenalty = TimeAndCostPenalty.of(TimePenalty.of(ofMinutes(20), 2f), 1.5); for (var it : StreetMode.values()) { - // Apply car-penalty to all car modes allowed in access/egress only. Exclude car modes(CAR) used - // in direct street routing and car modes used when you bring the car with you onto transit. This is - // a bit limited and will not work if we combine car access/egress modes like CAR_TO_PARK with CAR - // in the same search. This is currently not possible, but if we enable this in the future this logic must be - // looked at again. + // Apply car-penalty to all car modes used in access/egress. Car modes(CAR) used in direct street + // routing and car modes used when you bring the car with you onto transit should be excluded. The + // penalty should also be applied to modes used in access, egress AND direct (CAR_TO_PARK and + // CAR_RENTAL). This is not ideal, since we get an unfair comparison in the itinerary filters. We will + // live with this for now, but might revisit it later. if ( it.includesDriving() && (it.accessAllowed() || it.egressAllowed()) && it != StreetMode.CAR ) {