Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to configure linear cost function for transit vs walk filter #5495

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add option to configure linear cost function for removeTransitWithHig…
…herCostThanBestOnWalkOnly filter
bartosz committed Nov 9, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 2b510d4559f531b98954e2ddf6b6a5b24711cbeb
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@
import org.opentripplanner.ext.accessibilityscore.AccessibilityScoreFilter;
import org.opentripplanner.ext.fares.FaresFilter;
import org.opentripplanner.framework.lang.Sandbox;
import org.opentripplanner.framework.model.Cost;
import org.opentripplanner.model.plan.Itinerary;
import org.opentripplanner.model.plan.SortOrder;
import org.opentripplanner.routing.algorithm.filterchain.api.TransitGeneralizedCostFilterParams;
@@ -60,6 +61,7 @@ public class ItineraryListFilterChainBuilder {
private int maxNumberOfItineraries = NOT_SET;
private ListSection maxNumberOfItinerariesCrop = ListSection.TAIL;
private CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly;
private CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly;
private boolean removeWalkAllTheWayResults;
private boolean sameFirstOrLastTripFilter;
private TransitGeneralizedCostFilterParams transitGeneralizedCostFilterParams;
@@ -206,6 +208,27 @@ public ItineraryListFilterChainBuilder withRemoveTransitWithHigherCostThanBestOn
return this;
}

/**
* The direct street search(walk) is not pruning the transit search, so in some
* cases we get "silly" transit itineraries that is marginally better on travel-duration compared
* with an on-street-all-the-way itinerary. Use this method to filter worse enough itineraries.
* <p>
* The filter removes all itineraries with a generalized-cost that is higher than the best
* on-street-all-the-way itinerary.
* <p>
* This filter only have an effect, if an on-street-all-the-way(WALK) itinerary
* exist.
*
* This have same effect as {@link ItineraryListFilterChainBuilder#withRemoveTransitWithHigherCostThanBestOnStreetOnly(CostLinearFunction)}
* but only for WALK itineraries
*/
public ItineraryListFilterChainBuilder withRemoveTransitWithHigherCostThanBestOnWalkOnly(
CostLinearFunction value
) {
this.removeTransitWithHigherCostThanBestOnStreetOnly = value;
return this;
}

/**
* A transit itinerary with higher generalized-cost than a walk-only itinerary is silly. This filter removes such
* itineraries.
@@ -388,7 +411,15 @@ public ItineraryListFilterChain build() {
}

if (removeTransitIfWalkingIsBetter) {
filters.add(new DeletionFlaggingFilter(new RemoveTransitIfWalkingIsBetterFilter()));
filters.add(
new DeletionFlaggingFilter(
new RemoveTransitIfWalkingIsBetterFilter(
removeTransitWithHigherCostThanBestOnWalkOnly != null
? removeTransitWithHigherCostThanBestOnWalkOnly
: CostLinearFunction.of(Cost.ZERO, 1.0)
)
)
);
}

if (removeWalkAllTheWayResults) {
Original file line number Diff line number Diff line change
@@ -15,6 +15,12 @@
*/
public class RemoveTransitIfWalkingIsBetterFilter implements ItineraryDeletionFlagger {

private final CostLinearFunction costLimitFunction;

public RemoveTransitIfWalkingIsBetterFilter(CostLinearFunction costLimitFunction) {
this.costLimitFunction = costLimitFunction;
}

/**
* Required for {@link org.opentripplanner.routing.algorithm.filterchain.ItineraryListFilterChain},
* to know which filters removed
@@ -38,7 +44,7 @@ public List<Itinerary> flagForRemoval(List<Itinerary> itineraries) {
return List.of();
}

var limit = minWalkCost.getAsInt();
var limit = costLimitFunction.calculate(Cost.costOfSeconds(minWalkCost.getAsInt())).toSeconds();

return itineraries
.stream()
Original file line number Diff line number Diff line change
@@ -70,6 +70,9 @@ public static ItineraryListFilterChain createFilterChain(
.withRemoveTransitWithHigherCostThanBestOnStreetOnly(
params.removeTransitWithHigherCostThanBestOnStreetOnly()
)
.withRemoveTransitWithHigherCostThanBestOnWalkOnly(
params.removeTransitWithHigherCostThanBestOnWalkOnly()
)
.withSameFirstOrLastTripFilter(params.filterItinerariesWithSameFirstOrLastTrip())
.withAccessibilityScore(
params.useAccessibilityScore() && request.wheelchair(),
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@ public final class ItineraryFilterPreferences {
private final boolean removeItinerariesWithSameRoutesAndStops;
private final TransitGeneralizedCostFilterParams transitGeneralizedCostLimit;
private final CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly;
private final CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly;
private final boolean removeTransitIfWalkingIsBetter;

private ItineraryFilterPreferences() {
@@ -52,6 +53,8 @@ private ItineraryFilterPreferences() {
);
this.removeTransitWithHigherCostThanBestOnStreetOnly =
CostLinearFunction.of(Duration.ofMinutes(1), 1.3);
this.removeTransitWithHigherCostThanBestOnWalkOnly =
CostLinearFunction.of(Duration.ofMinutes(0), 1.0);
this.removeTransitIfWalkingIsBetter = false;
}

@@ -73,6 +76,8 @@ private ItineraryFilterPreferences(Builder builder) {
this.transitGeneralizedCostLimit = Objects.requireNonNull(builder.transitGeneralizedCostLimit);
this.removeTransitWithHigherCostThanBestOnStreetOnly =
Objects.requireNonNull(builder.removeTransitWithHigherCostThanBestOnStreetOnly);
this.removeTransitWithHigherCostThanBestOnWalkOnly =
Objects.requireNonNull(builder.removeTransitWithHigherCostThanBestOnWalkOnly);
this.removeTransitIfWalkingIsBetter = builder.removeTransitIfWalkingIsBetter;
}

@@ -136,6 +141,10 @@ public CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly() {
return removeTransitWithHigherCostThanBestOnStreetOnly;
}

public CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly() {
return removeTransitWithHigherCostThanBestOnWalkOnly;
}

public boolean removeTransitIfWalkingIsBetter() {
return removeTransitIfWalkingIsBetter;
}
@@ -183,6 +192,11 @@ public String toString() {
removeTransitWithHigherCostThanBestOnStreetOnly,
DEFAULT.removeTransitWithHigherCostThanBestOnStreetOnly
)
.addObj(
"removeTransitWithHigherCostThanBestOnWalkOnly",
removeTransitWithHigherCostThanBestOnWalkOnly,
DEFAULT.removeTransitWithHigherCostThanBestOnWalkOnly
)
.addBoolIfTrue(
"removeItinerariesWithSameRoutesAndStops",
removeItinerariesWithSameRoutesAndStops
@@ -217,6 +231,10 @@ public boolean equals(Object o) {
removeTransitWithHigherCostThanBestOnStreetOnly,
that.removeTransitWithHigherCostThanBestOnStreetOnly
) &&
Objects.equals(
removeTransitWithHigherCostThanBestOnWalkOnly,
that.removeTransitWithHigherCostThanBestOnWalkOnly
) &&
Objects.equals(transitGeneralizedCostLimit, that.transitGeneralizedCostLimit)
);
}
@@ -237,6 +255,7 @@ public int hashCode() {
removeItinerariesWithSameRoutesAndStops,
transitGeneralizedCostLimit,
removeTransitWithHigherCostThanBestOnStreetOnly,
removeTransitWithHigherCostThanBestOnWalkOnly,
removeTransitIfWalkingIsBetter
);
}
@@ -257,6 +276,7 @@ public static class Builder {
private boolean removeItinerariesWithSameRoutesAndStops;
private TransitGeneralizedCostFilterParams transitGeneralizedCostLimit;
private CostLinearFunction removeTransitWithHigherCostThanBestOnStreetOnly;
private CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly;
private boolean removeTransitIfWalkingIsBetter;

public ItineraryFilterPreferences original() {
@@ -341,6 +361,14 @@ public Builder withRemoveTransitWithHigherCostThanBestOnStreetOnly(
return this;
}

public Builder withRemoveTransitWithHigherCostThanBestOnWalkOnly(
CostLinearFunction removeTransitWithHigherCostThanBestOnWalkOnly
) {
this.removeTransitWithHigherCostThanBestOnWalkOnly =
removeTransitWithHigherCostThanBestOnWalkOnly;
return this;
}

public Builder withRemoveTransitIfWalkingIsBetter(boolean removeTransitIfWalkingIsBetter) {
this.removeTransitIfWalkingIsBetter = removeTransitIfWalkingIsBetter;
return this;
Original file line number Diff line number Diff line change
@@ -174,6 +174,28 @@ public static void mapItineraryFilterParams(
)
.asCostLinearFunction(dft.removeTransitWithHigherCostThanBestOnStreetOnly())
)
.withRemoveTransitWithHigherCostThanBestOnStreetOnly(
c
.of("removeTransitWithHigherCostThanBestOnWalkOnly")
.since(V2_4)
.summary(
"Limit function for generalized-cost computed from street-only itineraries applied to transit itineraries."
)
.description(
"""
The max-limit is applied to itineraries with transit *legs*, and only itineraries
without transit legs are considered when calculating the minimum cost. The smallest
generalized-cost value is used as input to the function. The function is used to calculate a
*max-limit*. The max-limit is then used to filter *transit* itineraries by
*generalized-cost*. Itineraries with a cost higher than the max-limit are dropped from the result
set. Walking is handled with a different logic: if a transit itinerary has higher cost than
a plain walk itinerary, it will be removed even if the cost limit function would keep it.
This function works in the same way as removeTransitWithHigherCostThanBestOnStreetOnly but only for WALK legs.
Default value is 0 + 1.0x.
"""
)
.asCostLinearFunction(dft.removeTransitWithHigherCostThanBestOnWalkOnly())
)
.withBikeRentalDistanceRatio(
c
.of("bikeRentalDistanceRatio")
Original file line number Diff line number Diff line change
@@ -7,8 +7,10 @@
import java.time.Duration;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.opentripplanner.framework.model.Cost;
import org.opentripplanner.model.plan.Itinerary;
import org.opentripplanner.model.plan.PlanTestConstants;
import org.opentripplanner.routing.api.request.framework.CostLinearFunction;

public class RemoveTransitIfWalkingIsBetterTest implements PlanTestConstants {

@@ -21,7 +23,7 @@ public void filterAwayNothingIfNoWalking() {
// When:
List<Itinerary> result = DeletionFlaggerTestHelper.process(
List.of(i1, i2),
new RemoveTransitIfWalkingIsBetterFilter()
new RemoveTransitIfWalkingIsBetterFilter(CostLinearFunction.of(Cost.ZERO, 1.0))
);

// Then:
@@ -44,9 +46,10 @@ public void filterAwayTransitWithLongerWalk() {

List<Itinerary> result = DeletionFlaggerTestHelper.process(
List.of(i1, i2, bicycle, walk),
new RemoveTransitIfWalkingIsBetterFilter()
new RemoveTransitIfWalkingIsBetterFilter(CostLinearFunction.of(Cost.ZERO, 1.0))
);

assertEquals(toStr(List.of(i2, bicycle, walk)), toStr(result));
}
// TODO: 2023-11-09 Write tests for cases other than 0 + 1.0x
}