diff --git a/src/main/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessor.java b/src/main/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessor.java index 515d5e836f4..b377c4f0635 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessor.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessor.java @@ -3,6 +3,9 @@ import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Multimap; +import java.time.LocalDate; +import java.time.temporal.ChronoUnit; +import java.util.BitSet; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -14,6 +17,7 @@ import org.opentripplanner.graph_builder.issues.InterliningTeleport; import org.opentripplanner.gtfs.mapping.StaySeatedNotAllowed; import org.opentripplanner.model.Timetable; +import org.opentripplanner.model.calendar.CalendarServiceData; import org.opentripplanner.model.transfer.ConstrainedTransfer; import org.opentripplanner.model.transfer.DefaultTransferService; import org.opentripplanner.model.transfer.TransferConstraint; @@ -33,17 +37,27 @@ public class InterlineProcessor { private final int maxInterlineDistance; private final DataImportIssueStore issueStore; private final List staySeatedNotAllowed; + private final LocalDate transitServiceStart; + private final int daysInTransitService; + private final CalendarServiceData calendarServiceData; + private final Map daysOfServices = new HashMap<>(); public InterlineProcessor( DefaultTransferService transferService, List staySeatedNotAllowed, int maxInterlineDistance, - DataImportIssueStore issueStore + DataImportIssueStore issueStore, + CalendarServiceData calendarServiceData ) { this.transferService = transferService; this.staySeatedNotAllowed = staySeatedNotAllowed; this.maxInterlineDistance = maxInterlineDistance > 0 ? maxInterlineDistance : 200; this.issueStore = issueStore; + this.transitServiceStart = calendarServiceData.getFirstDate().get(); + this.daysInTransitService = + (int) ChronoUnit.DAYS.between(transitServiceStart, calendarServiceData.getLastDate().get()) + + 1; + this.calendarServiceData = calendarServiceData; } public List run(Collection tripPatterns) { @@ -105,8 +119,8 @@ private Multimap getInterlinedTrips( /* Record which Pattern each interlined TripTimes belongs to. */ Map patternForTripTimes = new HashMap<>(); - /* TripTimes grouped by the block ID and service ID of their trips. Must be a ListMultimap to allow sorting. */ - ListMultimap tripTimesForBlock = ArrayListMultimap.create(); + /* TripTimes grouped by the block ID of their trips. Must be a ListMultimap to allow sorting. */ + ListMultimap tripTimesForBlockId = ArrayListMultimap.create(); LOG.info("Finding interlining trips based on block IDs."); for (TripPattern pattern : tripPatterns) { @@ -115,7 +129,7 @@ private Multimap getInterlinedTrips( for (TripTimes tripTimes : timetable.getTripTimes()) { Trip trip = tripTimes.getTrip(); if (StringUtils.hasValue(trip.getGtfsBlockId())) { - tripTimesForBlock.put(BlockIdAndServiceId.ofTrip(trip), tripTimes); + tripTimesForBlockId.put(trip.getGtfsBlockId(), tripTimes); // For space efficiency, only record times that are part of a block. patternForTripTimes.put(tripTimes, pattern); } @@ -125,52 +139,39 @@ private Multimap getInterlinedTrips( // Associate pairs of TripPatterns with lists of trips that continue from one pattern to the other. Multimap interlines = ArrayListMultimap.create(); - // Sort trips within each block by first departure time, then iterate over trips in this block and service, - // linking them. Has no effect on single-trip blocks. - SERVICE_BLOCK:for (BlockIdAndServiceId block : tripTimesForBlock.keySet()) { - List blockTripTimes = tripTimesForBlock.get(block); + // Sort trips within each block by first departure time, then iterate over trips in this block, + // linking them. One from trip can have multiple interline transfers if trip which interlines + // with the from trip doesn't operate on every service date of the from trip. + for (String blockId : tripTimesForBlockId.keySet()) { + List blockTripTimes = tripTimesForBlockId.get(blockId); Collections.sort(blockTripTimes); - TripTimes prev = null; - for (TripTimes curr : blockTripTimes) { - if (prev != null) { - if (prev.getDepartureTime(prev.getNumStops() - 1) > curr.getArrivalTime(0)) { - LOG.error( - "Trip times within block {} are not increasing on service {} after trip {}.", - block.blockId(), - block.serviceId(), - prev.getTrip().getId() - ); - continue SERVICE_BLOCK; + for (int i = 0; i < blockTripTimes.size(); i++) { + var fromTripTimes = blockTripTimes.get(i); + var fromServiceId = fromTripTimes.getTrip().getServiceId(); + BitSet uncoveredDays = getDaysForService(fromServiceId, true); + for (int j = i + 1; j < blockTripTimes.size(); j++) { + var toTripTimes = blockTripTimes.get(j); + var toServiceId = toTripTimes.getTrip().getServiceId(); + if ( + toServiceId.equals(fromServiceId) && + createInterline(fromTripTimes, toTripTimes, blockId, patternForTripTimes, interlines) + ) { + break; } - TripPattern prevPattern = patternForTripTimes.get(prev); - TripPattern currPattern = patternForTripTimes.get(curr); - var fromStop = prevPattern.lastStop(); - var toStop = currPattern.firstStop(); - double teleportationDistance = SphericalDistanceLibrary.fastDistance( - fromStop.getLat(), - fromStop.getLon(), - toStop.getLat(), - toStop.getLon() + BitSet daysForToTripTimes = getDaysForService( + toTripTimes.getTrip().getServiceId(), + false ); - if (teleportationDistance > maxInterlineDistance) { - issueStore.add( - new InterliningTeleport( - prev.getTrip(), - block.blockId(), - (int) teleportationDistance, - fromStop, - toStop - ) - ); - // Only skip this particular interline edge; there may be other valid ones in the block. - } else { - interlines.put( - new TripPatternPair(prevPattern, currPattern), - new TripPair(prev.getTrip(), curr.getTrip()) - ); + if ( + uncoveredDays.intersects(daysForToTripTimes) && + createInterline(fromTripTimes, toTripTimes, blockId, patternForTripTimes, interlines) + ) { + uncoveredDays.andNot(daysForToTripTimes); + if (uncoveredDays.isEmpty()) { + break; + } } } - prev = curr; } } @@ -178,13 +179,82 @@ private Multimap getInterlinedTrips( } /** - * This compound key object is used when grouping interlining trips together by (serviceId, - * blockId). + * Validates that trip times are continuous and that the transfer stop(s) are not too far away + * from each other. Then creates interline between the trips. + * + * @return true if interline has been created or if there is an issue preventing an interline + * creation for certain service dates. */ - private record BlockIdAndServiceId(String blockId, FeedScopedId serviceId) { - static BlockIdAndServiceId ofTrip(Trip trip) { - return new BlockIdAndServiceId(trip.getGtfsBlockId(), trip.getServiceId()); + private boolean createInterline( + TripTimes fromTripTimes, + TripTimes toTripTimes, + String blockId, + Map patternForTripTimes, + Multimap interlines + ) { + if ( + fromTripTimes.getDepartureTime(fromTripTimes.getNumStops() - 1) > + toTripTimes.getArrivalTime(0) + ) { + LOG.error( + "Trip times within block {} are not increasing on after trip {}.", + blockId, + fromTripTimes.getTrip().getId() + ); + return true; + } + TripPattern fromPattern = patternForTripTimes.get(fromTripTimes); + TripPattern toPattern = patternForTripTimes.get(toTripTimes); + var fromStop = fromPattern.lastStop(); + var toStop = toPattern.firstStop(); + double teleportationDistance = SphericalDistanceLibrary.fastDistance( + fromStop.getLat(), + fromStop.getLon(), + toStop.getLat(), + toStop.getLon() + ); + if (teleportationDistance > maxInterlineDistance) { + issueStore.add( + new InterliningTeleport( + fromTripTimes.getTrip(), + blockId, + (int) teleportationDistance, + fromStop, + toStop + ) + ); + // Only skip this particular interline edge; there may be other valid ones in the block for the + // from trip. + return false; + } else { + interlines.put( + new TripPatternPair(fromPattern, toPattern), + new TripPair(fromTripTimes.getTrip(), toTripTimes.getTrip()) + ); + return true; + } + } + + /** + * @param mutable This should be set as true if the {@link BitSet} will be modified as otherwise + * we don't have to create a copy of a cached entity. + * @return {@link BitSet} which index starts at the first overall date of the services and the + * last index is the last date. + */ + private BitSet getDaysForService(FeedScopedId serviceId, boolean mutable) { + BitSet daysForService = this.daysOfServices.get(serviceId); + if (daysForService == null) { + daysForService = new BitSet(daysInTransitService); + var serviceDates = calendarServiceData.getServiceDatesForServiceId(serviceId); + if (serviceDates != null) { + for (LocalDate serviceDate : serviceDates) { + int daysBetween = (int) ChronoUnit.DAYS.between(transitServiceStart, serviceDate); + daysForService.set(daysBetween); + } + } + daysOfServices.put(serviceId, daysForService); } + return mutable ? (BitSet) daysForService.clone() : daysForService; } private record TripPatternPair(TripPattern from, TripPattern to) {} diff --git a/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsModule.java b/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsModule.java index a772672fcff..14602d6a705 100644 --- a/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsModule.java +++ b/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsModule.java @@ -180,7 +180,8 @@ public void buildGraph() { transitModel.getTransferService(), builder.getStaySeatedNotAllowed(), gtfsBundle.maxInterlineDistance(), - issueStore + issueStore, + calendarServiceData ) .run(otpTransitService.getTripPatterns()); } diff --git a/src/main/java/org/opentripplanner/model/calendar/CalendarServiceData.java b/src/main/java/org/opentripplanner/model/calendar/CalendarServiceData.java index 2f98027a10a..8b91da7f9cc 100644 --- a/src/main/java/org/opentripplanner/model/calendar/CalendarServiceData.java +++ b/src/main/java/org/opentripplanner/model/calendar/CalendarServiceData.java @@ -9,6 +9,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.opentripplanner.framework.time.ServiceDateUtils; import org.opentripplanner.transit.model.framework.FeedScopedId; @@ -72,6 +73,24 @@ public void add(CalendarServiceData other) { } } + public Optional getFirstDate() { + return serviceDatesByServiceId + .values() + .stream() + .filter(list -> !list.isEmpty()) + .map(list -> list.get(0)) + .min(LocalDate::compareTo); + } + + public Optional getLastDate() { + return serviceDatesByServiceId + .values() + .stream() + .filter(list -> !list.isEmpty()) + .map(list -> list.get(list.size() - 1)) + .max(LocalDate::compareTo); + } + /* private methods */ private static List sortedImmutableList(Collection c) { diff --git a/src/test/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessorTest.java b/src/test/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessorTest.java index 4229ea4b287..c2dba539586 100644 --- a/src/test/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessorTest.java +++ b/src/test/java/org/opentripplanner/graph_builder/module/interlining/InterlineProcessorTest.java @@ -4,12 +4,19 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.transit.model._data.TransitModelForTest.stopTime; +import java.time.LocalDate; +import java.time.Month; import java.util.List; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; import org.opentripplanner.gtfs.mapping.StaySeatedNotAllowed; +import org.opentripplanner.model.calendar.CalendarServiceData; import org.opentripplanner.model.plan.PlanTestConstants; import org.opentripplanner.model.transfer.DefaultTransferService; +import org.opentripplanner.test.support.VariableSource; import org.opentripplanner.transit.model._data.TransitModelForTest; import org.opentripplanner.transit.model.framework.Deduplicator; import org.opentripplanner.transit.model.framework.FeedScopedId; @@ -20,27 +27,95 @@ class InterlineProcessorTest implements PlanTestConstants { List patterns = List.of( - tripPattern("trip-1", "block-1"), - tripPattern("trip-2", "block-1"), - tripPattern("trip-2", "block-3") + tripPattern("trip-1", "block-1", "service-1"), + tripPattern("trip-2", "block-1", "service-1"), + tripPattern("trip-3", "block-1", "service-2"), + tripPattern("trip-4", "block-1", "service-3"), + tripPattern("trip-5", "block-2", "service-4") ); - @Test - void run() { + static Stream interlineTestCases = Stream.of( + Arguments.of( + List.of( + new FeedScopedId("1", "service-1"), + new FeedScopedId("1", "service-2"), + new FeedScopedId("1", "service-3"), + new FeedScopedId("1", "service-4") + ), + List.of( + List.of(LocalDate.of(2023, Month.JANUARY, 1), LocalDate.of(2023, Month.JANUARY, 5)), + List.of(LocalDate.of(2023, Month.JANUARY, 1)), + List.of(LocalDate.of(2023, Month.JANUARY, 1)), + List.of(LocalDate.of(2023, Month.JANUARY, 1)) + ), + "[ConstrainedTransfer{from: TripTP{F:trip-2, stopPos 2}, to: TripTP{F:trip-3, stopPos 0}, " + + "constraint: {staySeated}}, ConstrainedTransfer{from: TripTP{F:trip-1, stopPos 2}, " + + "to: TripTP{F:trip-2, stopPos 0}, constraint: {staySeated}}, " + + "ConstrainedTransfer{from: TripTP{F:trip-3, stopPos 2}, to: TripTP{F:trip-4, stopPos 0}, constraint: {staySeated}}]" + ), + Arguments.of( + List.of( + new FeedScopedId("1", "service-1"), + new FeedScopedId("1", "service-2"), + new FeedScopedId("1", "service-3"), + new FeedScopedId("1", "service-4") + ), + List.of( + List.of(LocalDate.of(2023, Month.JANUARY, 1), LocalDate.of(2023, Month.JANUARY, 5)), + List.of(LocalDate.of(2023, Month.JANUARY, 1)), + List.of(LocalDate.of(2023, Month.JANUARY, 5)), + List.of(LocalDate.of(2023, Month.JANUARY, 1)) + ), + "[ConstrainedTransfer{from: TripTP{F:trip-2, stopPos 2}, to: TripTP{F:trip-3, stopPos 0}, " + + "constraint: {staySeated}}, ConstrainedTransfer{from: TripTP{F:trip-1, stopPos 2}, " + + "to: TripTP{F:trip-2, stopPos 0}, constraint: {staySeated}}, " + + "ConstrainedTransfer{from: TripTP{F:trip-2, stopPos 2}, to: TripTP{F:trip-4, stopPos 0}, constraint: {staySeated}}]" + ), + // No common days between services + Arguments.of( + List.of( + new FeedScopedId("1", "service-1"), + new FeedScopedId("1", "service-2"), + new FeedScopedId("1", "service-3"), + new FeedScopedId("1", "service-4") + ), + List.of( + List.of(LocalDate.of(2023, Month.JANUARY, 1), LocalDate.of(2023, Month.JANUARY, 5)), + List.of(LocalDate.of(2023, Month.JANUARY, 2)), + List.of(LocalDate.of(2023, Month.JANUARY, 3)), + List.of(LocalDate.of(2023, Month.JANUARY, 1)) + ), + "[ConstrainedTransfer{from: TripTP{F:trip-1, stopPos 2}, to: TripTP{F:trip-2, stopPos 0}, constraint: {staySeated}}]" + ) + ); + + @ParameterizedTest(name = "{0} services with {1} dates should generate transfers: {2}") + @VariableSource("interlineTestCases") + void testInterline( + List serviceIds, + List> serviceDates, + String transfers + ) { var transferService = new DefaultTransferService(); + var calendarServiceData = new CalendarServiceData(); + for (int i = 0; i < serviceIds.size(); i++) { + calendarServiceData.putServiceDatesForServiceId(serviceIds.get(i), serviceDates.get(i)); + } var processor = new InterlineProcessor( transferService, List.of(), 100, - DataImportIssueStore.NOOP + DataImportIssueStore.NOOP, + calendarServiceData ); var createdTransfers = processor.run(patterns); - assertEquals(1, createdTransfers.size()); assertEquals(transferService.listAll(), createdTransfers); createdTransfers.forEach(t -> assertTrue(t.getTransferConstraint().isStaySeated())); + + assertEquals(transfers, createdTransfers.toString()); } @Test @@ -52,11 +127,18 @@ void staySeatedNotAllowed() { var notAllowed = new StaySeatedNotAllowed(fromTrip, toTrip); + var calendarService = new CalendarServiceData(); + calendarService.putServiceDatesForServiceId( + new FeedScopedId("1", "service-1"), + List.of(LocalDate.of(2023, Month.JANUARY, 1)) + ); + var processor = new InterlineProcessor( transferService, List.of(notAllowed), 100, - DataImportIssueStore.NOOP + DataImportIssueStore.NOOP, + calendarService ); var createdTransfers = processor.run(patterns); @@ -65,11 +147,11 @@ void staySeatedNotAllowed() { assertEquals(transferService.listAll(), createdTransfers); } - private static TripPattern tripPattern(String tripId, String blockId) { + private static TripPattern tripPattern(String tripId, String blockId, String serviceId) { var trip = TransitModelForTest .trip(tripId) .withGtfsBlockId(blockId) - .withServiceId(new FeedScopedId("1", "1")) + .withServiceId(new FeedScopedId("1", serviceId)) .build(); var stopTimes = List.of(stopTime(trip, 0), stopTime(trip, 1), stopTime(trip, 2));