Skip to content
Merged
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ public interface TripSchedule extends DefaultTripSchedule {
*/
TripPattern getOriginalTripPattern();

/**
* Return the index used to look up this trip in the
* {@link org.opentripplanner.raptor.spi.RaptorTimeTable#getTripSchedule(int)}.
*/
int tripScheduleIndex();

/**
* Return {@code true} if this trip is not based on a fixed schedule, but instead a frequency
* based scheduled trip. The {@link #frequencyHeadwayInSeconds()} is only defined for such trips.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public FrequencyBoardOrAlightEvent(
/* RaptorTripScheduleBoardOrAlightEvent implementation */

@Override
public int tripIndex() {
public int tripScheduleIndex() {
return tripTimes.getDepartureTime(0) + offset;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.opentripplanner.raptor.spi.RaptorStopNameResolver;
import org.opentripplanner.raptor.spi.RaptorTransfer;
import org.opentripplanner.raptor.spi.RaptorTransitDataProvider;
import org.opentripplanner.raptor.spi.RaptorTripScheduleReference;
import org.opentripplanner.raptor.util.BitSetIterator;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.DefaultSlackProvider;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.RaptorTransitData;
Expand Down Expand Up @@ -260,4 +261,9 @@ public RaptorConstrainedBoardingSearch<TripSchedule> transferConstraintsReverseS
}
return new ConstrainedBoardingSearch(false, toStopTransfers, fromStopTransfers);
}

@Override
public RaptorTripScheduleReference tripScheduleReference(TripSchedule trip) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be here when it only depends on the trip?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand, can you explain?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing from the RaptorRoutingRequestTransitData class or instance is actually needed to do the work. Only the trip itself, that's why I question the location of the method. For example, might it make more sense to put it in the TripSchedule itself?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will be on vacation next week, so I approve this for now so that you are not blocked. But I would like you to think of a good justification for where this method should be and maybe change it's location in one of your next PRs. (If there is a good reason for it being here, that might be fine, I would just like that choice to have been made consciously)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now I understand. This i s the implementation of the RaptorTransitDataProvider. So the reason for this is the following:

In the Raptor SPI we try to avoid enforcing dependencies onto the model, especially from the leafs(ScheduledTripSchedule) and up (Route/TripPattern). We want to keep the ScheduledTripSchedule as small and compact as possible for the best possible performance. The bigger it is the more memory we need to scan - this apply to the logic in the hot loop. But in this case we need to look ut the route using the ScheduledTripSchedule utside the hot loop - then it make sense to just have an index for this.

When designing the Raptor SPI/API we design for want we think is an open less restrictive model - we do not look to much on the implementation. This should actually apply to all api design. Having an API with less restrictions make it easier to change the implementation in the future. An I think we should remove the reference to Route/TripPattern to make the memory footprint smaller.

Does this make sense?

return new RaptorTripScheduleReference(trip.pattern().patternIndex(), trip.tripScheduleIndex());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,6 @@ public int priorityGroupId() {
return priorityGroupId;
}

public int transitReluctanceFactorIndex() {
return tripPattern.transitReluctanceFactorIndex();
}

@Override
public String debugInfo() {
return tripPattern.debugInfo();
Expand Down Expand Up @@ -222,6 +218,10 @@ public int numberOfTripSchedules() {
return numberOfTripSchedules;
}

public int transitReluctanceFactorIndex() {
return tripPattern.transitReluctanceFactorIndex();
}

public Route route() {
return tripPattern.route();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public T trip() {
}

@Override
public int tripIndex() {
public int tripScheduleIndex() {
return candidateTripIndex;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public T trip() {
}

@Override
public int tripIndex() {
public int tripScheduleIndex() {
return candidateTripIndex;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ public TripPattern getOriginalTripPattern() {
return pattern.getTripPattern().getPattern();
}

@Override
public int tripScheduleIndex() {
return tripIndexForDates;
}

@Override
public LocalDate getServiceDate() {
if (tripTimes == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public ConstrainedTransferBoarding(
}

@Override
public int tripIndex() {
public int tripScheduleIndex() {
return tripIndex;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,10 @@ public interface RaptorTestConstants {
default String stopIndexToName(int index) {
return Character.toString('A' + index - 1);
}

static RuntimeException createDeprecatedUnsupportedFeatureException() {
return new UnsupportedOperationException(
"This class is deprecated, we are not adding more features."
);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.opentripplanner.raptorlegacy._data.transit;

import static org.opentripplanner.raptorlegacy._data.RaptorTestConstants.createDeprecatedUnsupportedFeatureException;

import java.util.ArrayList;
import java.util.BitSet;
import java.util.HashSet;
Expand All @@ -19,6 +21,7 @@
import org.opentripplanner.raptor.spi.RaptorTransfer;
import org.opentripplanner.raptor.spi.RaptorTransitDataProvider;
import org.opentripplanner.raptor.spi.RaptorTripPattern;
import org.opentripplanner.raptor.spi.RaptorTripScheduleReference;
import org.opentripplanner.raptor.util.BitSetIterator;
import org.opentripplanner.raptorlegacy._data.RaptorTestConstants;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.CostCalculatorFactory;
Expand Down Expand Up @@ -183,6 +186,11 @@ public RaptorConstrainedBoardingSearch<TestTripSchedule> transferConstraintsReve
return getRoute(routeIndex).transferConstraintsReverseSearch();
}

@Override
public RaptorTripScheduleReference tripScheduleReference(TestTripSchedule trip) {
throw createDeprecatedUnsupportedFeatureException();
}

public TestRoute getRoute(int index) {
return routes.get(index);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.raptorlegacy._data.transit;

import static org.opentripplanner.core.model.accessibility.Accessibility.NO_INFORMATION;
import static org.opentripplanner.raptorlegacy._data.RaptorTestConstants.createDeprecatedUnsupportedFeatureException;

import java.time.LocalDate;
import java.util.Arrays;
Expand Down Expand Up @@ -125,6 +126,11 @@ public TripPattern getOriginalTripPattern() {
return this.originalPattern;
}

@Override
public int tripScheduleIndex() {
throw createDeprecatedUnsupportedFeatureException();
}

@SuppressWarnings("UnusedReturnValue")
public static class Builder {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ TripAssert assertTripFound() {
}

TripAssert withIndex(int expectedTripIndex) {
assertEquals(expectedTripIndex, result.tripIndex(), "Trip index");
assertEquals(expectedTripIndex, result.tripScheduleIndex(), "Trip index");
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public void noTripsToAlightInEmptyPattern() {
}

@Test
public void findTripWithGivenTripIndexLowerBound() {
public void findTripWithGivenTripScheduleIndexLowerBound() {
// Given a pattern with the following trips: A, B
withTrips(tripA, tripB);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void noTripsToBoardInEmptyPattern() {
}

@Test
public void findTripWithGivenTripIndexUpperBound() {
public void findTripWithGivenTripScheduleIndexUpperBound() {
// Given a pattern with the following trips: A, B
int TRIP_INDEX_A = 0;
int TRIP_INDEX_B = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ private void assertBoarding(
assertEquals(expectedConstraint, boarding.transferConstraint());
assertEquals(stopIndex, boarding.boardStopIndex());
assertEquals(targetStopPos, boarding.stopPositionInPattern());
assertEquals(expectedTripIndex, boarding.tripIndex());
assertEquals(expectedTripIndex, boarding.tripScheduleIndex());
} else {
assertTrue(boarding.empty());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
// Run all test-cases on the given date
"testDate": "2009-12-17",
"feedId": "1",
"graph": "not-used-but-requiered.obj",
"graph": "not-used-but-required.obj",
"ignoreStreetResults": true
}
Original file line number Diff line number Diff line change
@@ -1,28 +1,16 @@
package org.opentripplanner.raptor.api.model;

/**
* RaptorOnBoardAccess allows you to board a specific trip at a given stop. The trip is
* identified by the route index and trip schedule index. A typical use-case for this is when you
* want to start a trip on-board, meaning that one is already on the vehicle when the path starts.
* The returned paths will start with a zero duration access and a boarding at the given stop.
* This class allows you to board a specific trip at a given stop. The trip is identified by the
* route index and trip schedule index. A typical use-case for this is when you want to start a
* trip on-board, meaning that one is already on the vehicle when the path starts. The returned
* paths will start with a zero duration access and a boarding at the given stop.
*/
public interface RaptorOnBoardAccess extends RaptorAccessEgress {
public interface RaptorStartOnBoardAccess extends RaptorAccessEgress {
/**
* The index of the boarded route
* Return the trip boarding this access is required to use.
*/
int routeIndex();

/**
* The index of the boarded trip within the route
*/
int tripScheduleIndex();

/**
* The stop position in the route pattern for the board stop. It must refer to the same stop as
* the {@link #stop()} method. The stop position is required because the stop can be visited twice
* in case of a circular stop pattern.
*/
int stopPositionInPattern();
RaptorTripScheduleStopPosition tripBoarding();

/**
* The stop index corresponding to {@link #stopPositionInPattern()}.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.opentripplanner.raptor.api.model;

import java.util.Objects;
import org.opentripplanner.utils.tostring.ToStringBuilder;

/**
* This class contains information to identify a given stop in a stop pattern for a given trip
* schedule in a route. This can for example be used to identify where a bording or alighting
* happens.
*/
public final class RaptorTripScheduleStopPosition {

private final int routeIndex;
private final int tripScheduleIndex;
private final int stopPositionInPattern;

public RaptorTripScheduleStopPosition(
int routeIndex,
int tripScheduleIndex,
int stopPositionInPattern
) {
this.routeIndex = routeIndex;
this.tripScheduleIndex = tripScheduleIndex;
this.stopPositionInPattern = stopPositionInPattern;
}

/// The index of the boarded route
public int routeIndex() {
return routeIndex;
}

/// The index of the boarded trip within the route
public int tripScheduleIndex() {
return tripScheduleIndex;
}

/// The stop position in the route stop-pattern. Knowing the stop index is not enough to identify
/// a stop, a stop may occour multiple times in a stop pattern, in other words a circular stop
/// pattern visits some of the same stops multiple times.
public int stopPositionInPattern() {
return stopPositionInPattern;
}

@Override
public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) {
return false;
}
RaptorTripScheduleStopPosition that = (RaptorTripScheduleStopPosition) o;
return (
routeIndex == that.routeIndex &&
tripScheduleIndex == that.tripScheduleIndex &&
stopPositionInPattern == that.stopPositionInPattern
);
}

@Override
public int hashCode() {
return Objects.hash(routeIndex, tripScheduleIndex, stopPositionInPattern);
}

@Override
public String toString() {
// The field labels are shortened to improve reading - should be easy to get in the context
return ToStringBuilder.of(RaptorTripScheduleStopPosition.class)
.addNum("route", routeIndex)
.addNum("tripSchedule", tripScheduleIndex)
.addNum("stopPosition", stopPositionInPattern)
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import java.util.function.IntFunction;
import javax.annotation.Nullable;
import org.opentripplanner.raptor.api.model.RaptorTripScheduleStopPosition;
import org.opentripplanner.raptor.spi.RaptorConstants;
import org.opentripplanner.raptor.spi.RaptorCostCalculator;
import org.opentripplanner.raptor.spi.RaptorTransfer;
Expand Down Expand Up @@ -146,7 +147,7 @@ default EgressPathView egressPath() {

boolean arrivedOnBoard();

default TripScheduleStopPosition subsequentBoardingConstraint() {
default RaptorTripScheduleStopPosition subsequentBoardingConstraint() {
throw new UnsupportedOperationException();
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ private Optional<PathAndTripIndex<T>> findFirstPathInSearchWindow(
}

if (path.startTime() < latestDepartureTime) {
return Optional.of(new PathAndTripIndex<T>(path, boardEvent.tripIndex()));
return Optional.of(new PathAndTripIndex<T>(path, boardEvent.tripScheduleIndex()));
}
return Optional.empty();
}
Expand Down
Loading
Loading