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

Fix arrive by filtering for on-street/flex itineraries #6050

Merged
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
c35c98b
Make it explicit which itineraries are timeWindowAware
leonardehrenfried Sep 17, 2024
9b161fb
Implement filtering of search window aware itineraries
leonardehrenfried Sep 17, 2024
8d8566e
Add flex time windows in test builder
leonardehrenfried Sep 17, 2024
9ba36ed
Flesh out tests
leonardehrenfried Sep 17, 2024
e706076
Fix spelling in paging README
leonardehrenfried Sep 18, 2024
cf5266e
Add Javadoc for DirectStreetRouter, FilterTransitWhenStreetModeIsEmpty
leonardehrenfried Sep 18, 2024
f61cf33
Add documentation why applying the page cursor unsets the direct mode
leonardehrenfried Sep 18, 2024
a851e50
Remove outdated deprecation comment
leonardehrenfried Sep 18, 2024
1a5a388
Make test more comprehensive
leonardehrenfried Sep 18, 2024
318ea2f
Add filter just for direct flex itineraries
leonardehrenfried Sep 19, 2024
d37c51c
Add test for FlexWindowFilter
leonardehrenfried Sep 20, 2024
4c11b2a
Merge remote-tracking branch 'upstream/dev-2.x' into arrive-by-filtering
leonardehrenfried Sep 24, 2024
0538587
Extract factory methods for creating itineraries
leonardehrenfried Sep 24, 2024
c2a5310
Apply suggestions from code review
leonardehrenfried Sep 24, 2024
a6098d7
Rename isFlexAndWalkOnly to isDirectFlex
leonardehrenfried Sep 24, 2024
15faf80
Update Javadoc
leonardehrenfried Sep 24, 2024
f9c3add
Rename method
leonardehrenfried Sep 25, 2024
a6d11a5
Merge remote-tracking branch 'upstream/dev-2.x' into arrive-by-filtering
leonardehrenfried Sep 25, 2024
07d1c5c
Add setting for turning direct flex filter off
leonardehrenfried Sep 25, 2024
d1961b4
Add test for switching filter off
leonardehrenfried Sep 25, 2024
744017f
Improve documentation
leonardehrenfried Sep 25, 2024
b64c5da
Update src/main/java/org/opentripplanner/routing/algorithm/filterchai…
leonardehrenfried Sep 27, 2024
aca7ccd
Update src/main/java/org/opentripplanner/routing/algorithm/raptoradap…
leonardehrenfried Sep 27, 2024
442840b
Apply suggestions from code review
leonardehrenfried Sep 27, 2024
aa153b7
Check if itinerary contains flex
leonardehrenfried Sep 30, 2024
ac53bc0
Fix time notation in test
leonardehrenfried Sep 30, 2024
ef22007
Merge remote-tracking branch 'upstream/dev-2.x' into arrive-by-filtering
leonardehrenfried Sep 30, 2024
9ec11be
Fine-tune documentation
leonardehrenfried Sep 30, 2024
023c693
Merge remote-tracking branch 'upstream/dev-2.x' into arrive-by-filtering
leonardehrenfried Sep 30, 2024
fca7ba9
Filter flex resuls also by sort order
leonardehrenfried Oct 10, 2024
9e293e8
Update src/main/java/org/opentripplanner/routing/algorithm/filterchai…
leonardehrenfried Oct 10, 2024
483a04f
Add comment
leonardehrenfried Oct 10, 2024
aa7334e
Rename config parameter
leonardehrenfried Oct 10, 2024
4557498
Change logic of arrive by filters
leonardehrenfried Oct 10, 2024
4a519e4
Fix formatting
leonardehrenfried Oct 10, 2024
3169cef
Fix documentation
leonardehrenfried Oct 11, 2024
ce36b34
Update src/main/java/org/opentripplanner/routing/algorithm/filterchai…
leonardehrenfried Oct 11, 2024
c35532c
Apply suggestions from code review
leonardehrenfried Oct 11, 2024
9cb2f4e
Update documentation
leonardehrenfried Oct 11, 2024
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
17 changes: 17 additions & 0 deletions doc/user/RouteRequest.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe
|    [accessibilityScore](#rd_if_accessibilityScore) | `boolean` | An experimental feature contributed by IBI which adds a sandbox accessibility *score* between 0 and 1 for each leg and itinerary. | *Optional* | `false` | 2.2 |
|    [bikeRentalDistanceRatio](#rd_if_bikeRentalDistanceRatio) | `double` | Filter routes that consist of bike-rental and walking by the minimum fraction of the bike-rental leg using _distance_. | *Optional* | `0.0` | 2.1 |
|    [debug](#rd_if_debug) | `enum` | Enable this to attach a system notice to itineraries instead of removing them. This is very convenient when tuning the itinerary-filter-chain. | *Optional* | `"off"` | 2.0 |
|    [filterDirectFlexBySearchWindow](#rd_if_filterDirectFlexBySearchWindow) | `boolean` | Filter direct flex results by the search window. | *Optional* | `true` | 2.7 |
|    [filterItinerariesWithSameFirstOrLastTrip](#rd_if_filterItinerariesWithSameFirstOrLastTrip) | `boolean` | If more than one itinerary begins or ends with same trip, filter out one of those itineraries so that only one remains. | *Optional* | `false` | 2.2 |
|    groupSimilarityKeepOne | `double` | Pick ONE itinerary from each group after putting itineraries that are 85% similar together. | *Optional* | `0.85` | 2.1 |
|    groupSimilarityKeepThree | `double` | Reduce the number of itineraries to three itineraries by reducing each group of itineraries grouped by 68% similarity. | *Optional* | `0.68` | 2.1 |
Expand Down Expand Up @@ -731,6 +732,22 @@ convenient when tuning the itinerary-filter-chain.
moving to the next page.


<h3 id="rd_if_filterDirectFlexBySearchWindow">filterDirectFlexBySearchWindow</h3>

**Since version:** `2.7` ∙ **Type:** `boolean` ∙ **Cardinality:** `Optional` ∙ **Default value:** `true`
**Path:** /routingDefaults/itineraryFilters

Filter direct flex results by the search window.

When direct flex is mixed with a transit search in the same request, then the direct flex results are filtered by the
search window of the transit results.

Depart-at searches are filtered by latest-arrival-time and arrive-by searches are filtered
by earliest-departure-time.

Use this configuration to turn this feature off.


<h3 id="rd_if_filterItinerariesWithSameFirstOrLastTrip">filterItinerariesWithSameFirstOrLastTrip</h3>

**Since version:** `2.2` ∙ **Type:** `boolean` ∙ **Cardinality:** `Optional` ∙ **Default value:** `false`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.opentripplanner.model.plan.Itinerary.createScheduledTransitItinerary;
import static org.opentripplanner.transit.model._data.TransitModelForTest.id;

import java.time.OffsetDateTime;
Expand Down Expand Up @@ -63,42 +64,48 @@ static void SetUp() {

@Test
void testGetEmissionsForItinerary() {
Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS)));
Itinerary i = createScheduledTransitItinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS)));
decorateWithEmission.decorate(i);
assertEquals(new Grams(2223.902), i.getEmissionsPerPerson().getCo2());
}

@Test
void testGetEmissionsForCarRoute() {
Itinerary i = new Itinerary(List.of(STREET_LEG));
Itinerary i = createScheduledTransitItinerary(List.of(STREET_LEG));
decorateWithEmission.decorate(i);
assertEquals(new Grams(28.0864), i.getEmissionsPerPerson().getCo2());
}

@Test
void testNoEmissionsForFeedWithoutEmissionsConfigured() {
Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED)));
Itinerary i = createScheduledTransitItinerary(
List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED))
);
decorateWithEmission.decorate(i);
assertNull(i.getEmissionsPerPerson());
}

@Test
void testZeroEmissionsForItineraryWithZeroEmissions() {
Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_ZERO_EMISSIONS)));
Itinerary i = createScheduledTransitItinerary(
List.of(createTransitLeg(ROUTE_WITH_ZERO_EMISSIONS))
);
decorateWithEmission.decorate(i);
assertEquals(new Grams(0.0), i.getEmissionsPerPerson().getCo2());
}

@Test
void testGetEmissionsForCombinedRoute() {
Itinerary i = new Itinerary(List.of(createTransitLeg(ROUTE_WITH_EMISSIONS), STREET_LEG));
Itinerary i = createScheduledTransitItinerary(
List.of(createTransitLeg(ROUTE_WITH_EMISSIONS), STREET_LEG)
);
decorateWithEmission.decorate(i);
assertEquals(new Grams(2251.9884), i.getEmissionsPerPerson().getCo2());
}

@Test
void testNoEmissionsForCombinedRouteWithoutTransitEmissions() {
Itinerary i = new Itinerary(
Itinerary i = createScheduledTransitItinerary(
List.of(createTransitLeg(ROUTE_WITHOUT_EMISSIONS_CONFIGURED), STREET_LEG)
);
decorateWithEmission.decorate(i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public static void setUpClass() {
* types.
*/
private static void calculateFare(List<Leg> legs, FareType fareType, Money expectedPrice) {
var itinerary = new Itinerary(legs);
var itinerary = Itinerary.createScheduledTransitItinerary(legs);
var itineraryFares = orcaFareService.calculateFares(itinerary);
assertEquals(
expectedPrice,
Expand Down
38 changes: 36 additions & 2 deletions src/main/java/org/opentripplanner/model/plan/Itinerary.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,14 @@ public class Itinerary implements ItinerarySortKey {
/* other properties */

private final List<SystemNotice> systemNotices = new ArrayList<>();
private final boolean searchWindowAware;
private List<Leg> legs;

private ItineraryFares fare = ItineraryFares.empty();

public Itinerary(List<Leg> legs) {
private Itinerary(List<Leg> legs, boolean searchWindowAware) {
setLegs(legs);
this.searchWindowAware = searchWindowAware;

// Set aggregated data
ItinerariesCalculateLegTotals totals = new ItinerariesCalculateLegTotals(legs);
Expand All @@ -87,6 +89,21 @@ public Itinerary(List<Leg> legs) {
this.setElevationLost(totals.totalElevationLost);
}

/**
* Creates an itinerary that contains scheduled transit which is aware of the search window.
*/
public static Itinerary createScheduledTransitItinerary(List<Leg> legs) {
return new Itinerary(legs, true);
}

/**
* Creates an itinerary that creates only street or flex results which are not aware of the
* time window.
*/
public static Itinerary createDirectItinerary(List<Leg> legs) {
return new Itinerary(legs, false);
}

/**
* Time that the trip departs.
*/
Expand Down Expand Up @@ -162,13 +179,30 @@ public boolean isOnStreetAllTheWay() {
return isStreetOnly();
}

/**
* Returns true if this itinerary has only flex and walking legs.
*/
public boolean isDirectFlex() {
var containsFlex = legs.stream().anyMatch(Leg::isFlexibleTrip);
var flexOrWalkOnly = legs.stream().allMatch(l -> l.isFlexibleTrip() || l.isWalkingLeg());
return containsFlex && flexOrWalkOnly;
}

/** TRUE if at least one leg is a transit leg. */
public boolean hasTransit() {
return legs
.stream()
.anyMatch(l -> l instanceof ScheduledTransitLeg || l instanceof FlexibleTransitLeg);
}

/**
* Returns true if this itinerary was produced by an algorithm that is aware of the search window.
* As of 2024 only the itineraries produced by RAPTOR that do that.
*/
public boolean isSearchWindowAware() {
return searchWindowAware;
}

public Leg firstLeg() {
return getLegs().get(0);
}
Expand Down Expand Up @@ -215,7 +249,7 @@ public Itinerary withTimeShiftToStartAt(ZonedDateTime afterTime) {
.stream()
.map(leg -> leg.withTimeShift(duration))
.collect(Collectors.toList());
var newItin = new Itinerary(timeShiftedLegs);
var newItin = new Itinerary(timeShiftedLegs, searchWindowAware);
newItin.setGeneralizedCost(getGeneralizedCost());
return newItin;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ moving on to the next.

## Terminology

- **search-window (sw)** The search window is the minutes Raptor iterate over and the time-window
- **search-window (sw)** The search window is the minutes Raptor iterates over and the time-window
the itinerary must start within to be included in the result. The search-window may change from a
request to the next page. **sw'** is the search window for the new next/previous page. The search
window may change between requests, so we need to account for it when computing the next/previous
Expand All @@ -28,7 +28,7 @@ moving on to the next.
- **<< previous page** The trip search constructed to retrieve itineraries AFTER the original
search.
- **crop-search-window** If the `maxNumOfItineraries` limit is reached in the
`ItineraryFilterChain`, then one or more itineraries are removed. The filter remove itineraries
`ItineraryFilterChain`, then one or more itineraries are removed. The filter removes itineraries
from the beginning or end of the list depending on the page cursor type (next/previous) and the
sort order(arrival/departure time).

Expand All @@ -55,8 +55,8 @@ _previous-page_ must reverse the itinerary-filtering: `crop itineraries at START
- In this case the `<< Previous page` is the same as in [sort-by-arrival](#sort-by-arrival) and not
shown.
- For the `Next page >>` we must adjust the `edt'`.
- In rare cases we get duplicate itineraries. This happens if the `removed itinerary` depart before,
but arrive after the `duplicate`.
- In rare cases we get duplicate itineraries. This happens if the `removed itinerary` departs before,
but arrives after the `duplicate`.

### sort-by-arrival, crop-search-window & original-prev-page

Expand Down Expand Up @@ -99,7 +99,7 @@ This is the basic `sort-by-departure` (arrive-by search) without removing itiner
In this case the itineraries are dropped from the search results in the `Original Search` and the
`<< Previous page` must be adjusted. We use the first removed itinerary to set both the `edt'` and
the `lat'`. An `optimal itinarary` in the original search is lost (not found) in the previous page
if it departs AFTER the `remoed itinerary` and arrive before - hopefully this is a rare case.
if it departs AFTER the `removed itinerary` and arrives before - hopefully this is a rare case.

The `Next page >>` is the same as the basic case [sort-by-departure](#sort-by-departure).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.opentripplanner.routing.algorithm.filterchain.filters.street.RemoveNonTransitItinerariesBasedOnGeneralizedCost;
import org.opentripplanner.routing.algorithm.filterchain.filters.street.RemoveParkAndRideWithMostlyWalkingFilter;
import org.opentripplanner.routing.algorithm.filterchain.filters.street.RemoveWalkOnlyFilter;
import org.opentripplanner.routing.algorithm.filterchain.filters.system.FlexSearchWindowFilter;
import org.opentripplanner.routing.algorithm.filterchain.filters.system.NumItinerariesFilter;
import org.opentripplanner.routing.algorithm.filterchain.filters.system.OutsideSearchWindowFilter;
import org.opentripplanner.routing.algorithm.filterchain.filters.system.PagingFilter;
Expand Down Expand Up @@ -89,6 +90,7 @@ public class ItineraryListFilterChainBuilder {
private boolean removeTransitIfWalkingIsBetter = true;
private ItinerarySortKey itineraryPageCut;
private boolean transitGroupPriorityUsed = false;
private boolean filterDirectFlexBySearchWindow = true;

/**
* Sandbox filters which decorate the itineraries with extra information.
Expand Down Expand Up @@ -470,6 +472,13 @@ public ItineraryListFilterChain build() {
);
}

if (earliestDepartureTime != null && filterDirectFlexBySearchWindow) {
addRemoveFilter(
filters,
new FlexSearchWindowFilter(earliestDepartureTime, searchWindow, sortOrder)
);
}

// Remove itineraries present in the page retrieved before this page/search.
if (itineraryPageCut != null) {
addRemoveFilter(
Expand Down Expand Up @@ -533,6 +542,11 @@ public ItineraryListFilterChain build() {
return new ItineraryListFilterChain(filters, debugHandler);
}

public ItineraryListFilterChainBuilder withFilterDirectFlexBySearchWindow(boolean b) {
this.filterDirectFlexBySearchWindow = b;
return this;
}

/**
* If enabled, this adds the filter to remove itineraries which have the same stops and routes.
* These are sometimes called "time-shifted duplicates" but since those terms have so many
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package org.opentripplanner.routing.algorithm.filterchain.filters.system;

import java.time.Duration;
import java.time.Instant;
import java.util.function.Predicate;
import org.opentripplanner.model.plan.Itinerary;
import org.opentripplanner.model.plan.SortOrder;
import org.opentripplanner.routing.algorithm.filterchain.framework.spi.RemoveItineraryFlagger;

/**
* The flex router doesn't use the transit router's search-window, but nevertheless using it
* for filtering is useful when combining flex with transit.
* <p>
* The flex router also searches the previous day (arrive by) or the next one (depart after).
* If you didn't filter the flex results by something you could get yesterday's or tomorrow's
* trips where you would not expect it.
*/
public class FlexSearchWindowFilter implements RemoveItineraryFlagger {

public static final String TAG = "flex-outside-search-window";

private final Instant earliestDepartureTime;
private final Instant latestArrivalTime;
private final SortOrder sortOrder;

public FlexSearchWindowFilter(
Instant earliestDepartureTime,
Duration searchWindow,
SortOrder sortOrder
) {
this.earliestDepartureTime = earliestDepartureTime;
this.latestArrivalTime = earliestDepartureTime.plus(searchWindow);
this.sortOrder = sortOrder;
}

@Override
public String name() {
return TAG;
}

@Override
public Predicate<Itinerary> shouldBeFlaggedForRemoval() {
return it -> {
if (it.isDirectFlex()) {
return switch (sortOrder) {
case STREET_AND_DEPARTURE_TIME -> {
// arive by
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
var time = it.startTime().toInstant();
yield time.isBefore(earliestDepartureTime);
}
case STREET_AND_ARRIVAL_TIME -> {
// depart at
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
var time = it.startTime().toInstant();
yield time.isAfter(latestArrivalTime);
}
};
} else {
return false;
}
};
}

@Override
public boolean skipAlreadyFlaggedItineraries() {
return false;
}
}
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
import org.opentripplanner.routing.algorithm.filterchain.framework.spi.RemoveItineraryFlagger;

/**
* This filter will remove all itineraries that are outside the search-window. In some
* cases the access is time-shifted after the end of the search-window. These results
* This filter will remove all itineraries that are both search-window aware and outside the
* search-window. Only those that use transit are search-window aware, street and flex itineraries are not.
* <p>
* In some cases the access is time-shifted after the end of the search-window. These results
* should appear again when paging to the next page. Hence, this filter will remove
* such itineraries. The same is true for when paging to the previous page for arriveBy=true.
* <p>
Expand All @@ -35,8 +37,12 @@ public String name() {
@Override
public Predicate<Itinerary> shouldBeFlaggedForRemoval() {
return it -> {
var time = it.startTime().toInstant();
return time.isBefore(earliestDepartureTime) || !time.isBefore(latestDepartureTime);
if (it.isSearchWindowAware()) {
var time = it.startTime().toInstant();
return time.isBefore(earliestDepartureTime) || !time.isBefore(latestDepartureTime);
} else {
return false;
}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public Itinerary generateItinerary(GraphPath<State, Edge, Vertex> path) {
}
}

Itinerary itinerary = new Itinerary(legs);
Itinerary itinerary = Itinerary.createDirectItinerary(legs);

calculateElevations(itinerary, path.edges);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ else if (pathLeg.isTransferLeg()) {
Itinerary mapped = mapEgressLeg(egressPathLeg);
legs.addAll(mapped == null ? List.of() : mapped.getLegs());

Itinerary itinerary = new Itinerary(legs);
Itinerary itinerary = Itinerary.createScheduledTransitItinerary(legs);

// Map general itinerary fields
itinerary.setArrivedAtDestinationWithRentedVehicle(
Expand Down Expand Up @@ -390,7 +390,7 @@ private List<Leg> mapNonTransitLeg(
}

private Itinerary mapDirectPath(RaptorPath<T> path) {
return new Itinerary(
return Itinerary.createScheduledTransitItinerary(
List.of(
new UnknownTransitPathLeg(
mapPlace(request.from()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public static ItineraryListFilterChain createFilterChain(
.withPageCursorInputSubscriber(pageCursorInputSubscriber)
.withRemoveWalkAllTheWayResults(removeWalkAllTheWayResults)
.withRemoveTransitIfWalkingIsBetter(true)
.withFilterDirectFlexBySearchWindow(params.filterDirectFlexBySearchWindow())
.withDebugEnabled(params.debug());

if (!request.preferences().transit().relaxTransitGroupPriority().isNormal()) {
Expand Down
Loading
Loading