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 Stop Time Interpolation #399

Merged
merged 10 commits into from
Nov 21, 2023
27 changes: 23 additions & 4 deletions src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ public String update(Integer id, String json, boolean autoCommit) throws SQLExce
*
* @return number of stop times updated
*/
public int normalizeStopTimesForPattern(int id, int beginWithSequence) throws SQLException {
public int normalizeStopTimesForPattern(int id, int beginWithSequence, boolean interpolateStopTimes) throws SQLException {
br648 marked this conversation as resolved.
Show resolved Hide resolved
try {
JDBCTableReader<PatternStop> patternStops = new JDBCTableReader(
Table.PATTERN_STOP,
Expand All @@ -279,7 +279,7 @@ public int normalizeStopTimesForPattern(int id, int beginWithSequence) throws SQ
patternStopsToNormalize.add(patternStop);
}
}
int stopTimesUpdated = updateStopTimesForPatternStops(patternStopsToNormalize);
int stopTimesUpdated = updateStopTimesForPatternStops(patternStopsToNormalize, interpolateStopTimes);
connection.commit();
return stopTimesUpdated;
} catch (Exception e) {
Expand Down Expand Up @@ -755,8 +755,9 @@ private int updateStopTimesForPatternStop(ObjectNode patternStop, int previousTr
*
* TODO? add param Set<String> serviceIdFilters service_id values to filter trips on
*/
private int updateStopTimesForPatternStops(List<PatternStop> patternStops) throws SQLException {
private int updateStopTimesForPatternStops(List<PatternStop> patternStops, boolean interpolateStopTimes) throws SQLException {
PatternStop firstPatternStop = patternStops.iterator().next();

Choose a reason for hiding this comment

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

(Unlikely edge case) Say I accidentally make a pattern with zero stops and click Normalize stop times it throws a NoSuchElemenException and a long error.
image
Is there a way to check for an empty list and to return a cleaner error message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is best addressed by preventing the behaviour client side, although I'll defer to other reviewers if they'd like some additional checks

List<PatternStop> timepoints = patternStops.stream().filter(ps -> ps.timepoint == 1).collect(Collectors.toList());
int firstStopSequence = firstPatternStop.stop_sequence;
// Prepare SQL query to determine the time that should form the basis for adding the travel time values.
int previousStopSequence = firstStopSequence > 0 ? firstStopSequence - 1 : 0;
Expand Down Expand Up @@ -788,10 +789,28 @@ private int updateStopTimesForPatternStops(List<PatternStop> patternStops) throw
final BatchTracker stopTimesTracker = new BatchTracker("stop_times", updateStopTimeStatement);
for (String tripId : timesForTripIds.keySet()) {
// Initialize travel time with previous stop time value.
int cumulativeTravelTime = timesForTripIds.get(tripId);
int cumulativeTravelTime = timesForTripIds.get(tripId), timepointNumber = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Declare timepointNumber on a separate line e.g. int timepointNumber = 0;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 36f3043

double timepointSpeed = 0, previousShapeDistTraveled = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Declare previousShapeDistTraveled on a separate line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 36f3043

PatternStop currentTimepoint = patternStops.get(0); // First stop must be a timepoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add a check here to verify that the item exists and is a timepoint

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call, added in b585c9d

if (currentTimepoint.timepoint != 1 && interpolateStopTimes) {
throw new IllegalStateException("First pattern stop must be a timepoint to perform interpolation");
}
for (PatternStop patternStop : patternStops) {
boolean isTimepoint = patternStop.timepoint == 1;
// If we have reached a timepoint (which is not the last), we calculate the speed between it and the next timepoint.
if (isTimepoint && interpolateStopTimes && timepointNumber++ < timepoints.size()-1){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of injecting the new code into this for loop, look to create a new interpolation method which will carry out the required tasks and return the updated travelTime value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated to use a new method in 36f3043

previousShapeDistTraveled = currentTimepoint.shape_dist_traveled;
PatternStop nextTimePoint = timepoints.get(timepointNumber);
Copy link
Collaborator

Choose a reason for hiding this comment

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

make sure this isn't null

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in b585c9d

if (nextTimePoint == null) throw new IllegalStateException("Stop time interpolation is not possible with null timepoints.");
timepointSpeed = nextTimePoint.default_travel_time == Entity.INT_MISSING ? 0
: (nextTimePoint.shape_dist_traveled - currentTimepoint.shape_dist_traveled) / nextTimePoint.default_travel_time;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit. Line up the ? and : makes this easier to read.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 36f3043

}
// Gather travel/dwell time for pattern stop (being sure to check for missing values).
int travelTime = patternStop.default_travel_time == Entity.INT_MISSING ? 0 : patternStop.default_travel_time;
if (!isTimepoint && interpolateStopTimes) {
travelTime = (int) Math.round((patternStop.shape_dist_traveled - previousShapeDistTraveled) / timepointSpeed);
previousShapeDistTraveled += patternStop.shape_dist_traveled;
}
int dwellTime = patternStop.default_dwell_time == Entity.INT_MISSING ? 0 : patternStop.default_dwell_time;
int oneBasedIndex = 1;
// Increase travel time by current pattern stop's travel and dwell times (and set values for update).
Expand Down
8 changes: 8 additions & 0 deletions src/test/java/com/conveyal/gtfs/dto/PatternStopDTO.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,12 @@ public PatternStopDTO (String patternId, String stopId, int stopSequence) {
stop_id = stopId;
stop_sequence = stopSequence;
}

public PatternStopDTO (String patternId, String stopId, int stopSequence, int timepointValue, double shape_dist_traveledValue) {
timepoint = timepointValue;
pattern_id = patternId;
stop_id = stopId;
stop_sequence = stopSequence;
shape_dist_traveled = shape_dist_traveledValue;
}
}
109 changes: 80 additions & 29 deletions src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,12 @@ public class JDBCTableWriterTest {
private static String testGtfsGLSnapshotNamespace;
private static String simpleServiceId = "1";
private static String firstStopId = "1";
private static String secondStopId= "1.5";
private static String lastStopId = "2";
private static double firstStopLat = 34.2222;
private static double firstStopLon = -87.333;
private static double secondStopLat = 34.2227;
private static double secondStopLon = -87.3335;
private static double lastStopLat = 34.2233;
private static double lastStopLon = -87.334;
private static String sharedShapeId = "shared_shape_id";
Expand All @@ -93,6 +96,7 @@ public static void setUpClass() throws SQLException, IOException, InvalidNamespa
// Create a service calendar and two stops, both of which are necessary to perform pattern and trip tests.
createWeekdayCalendar(simpleServiceId, "20180103", "20180104");
createSimpleStop(firstStopId, "First Stop", firstStopLat, firstStopLon);
createSimpleStop(secondStopId, "Second Stop", secondStopLat, secondStopLon);
createSimpleStop(lastStopId, "Last Stop", lastStopLat, lastStopLon);

/** Load the following real-life GTFS for use with {@link JDBCTableWriterTest#canUpdateServiceId()} **/
Expand Down Expand Up @@ -838,29 +842,17 @@ public void canCreateUpdateAndDeleteFrequencyTripForFrequencyPattern() throws IO
));
}

/**
* Checks that {@link JdbcTableWriter#normalizeStopTimesForPattern(int, int)} can normalize stop times to a pattern's
* default travel times.
*/
@Test
public void canNormalizePatternStopTimes() throws IOException, SQLException, InvalidNamespaceException {
// Store Table and Class values for use in test.
private static String normalizeStopsForPattern(PatternStopDTO[] patternStops, int updatedStopSequence, boolean interpolateStopTimes, int initialTravelTime, int updatedTravelTime, int startTime, String patternId) throws SQLException, InvalidNamespaceException, IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reduce the length of this line by putting each param on a new line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 36f3043

final Table tripsTable = Table.TRIPS;
int initialTravelTime = 60; // one minute
int startTime = 6 * 60 * 60; // 6AM
String patternId = "123456";
PatternStopDTO[] patternStops = new PatternStopDTO[]{
new PatternStopDTO(patternId, firstStopId, 0),
new PatternStopDTO(patternId, lastStopId, 1)
};
patternStops[1].default_travel_time = initialTravelTime;

PatternDTO pattern = createRouteAndPattern(newUUID(),
patternId,
"Pattern A",
null,
new ShapePointDTO[]{},
patternStops,
0);
patternId,
"Pattern A",
null,
new ShapePointDTO[]{},
patternStops,
0);

// Create trip with travel times that match pattern stops.
TripDTO tripInput = constructTimetableTrip(pattern.pattern_id, pattern.route_id, startTime, initialTravelTime);
JdbcTableWriter createTripWriter = createTestTableWriter(tripsTable);
Expand All @@ -869,20 +861,79 @@ public void canNormalizePatternStopTimes() throws IOException, SQLException, Inv
TripDTO createdTrip = mapper.readValue(createTripOutput, TripDTO.class);
// Update pattern stop with new travel time.
JdbcTableWriter patternUpdater = createTestTableWriter(Table.PATTERNS);
int updatedTravelTime = 3600; // one hour
pattern.pattern_stops[1].default_travel_time = updatedTravelTime;
pattern.pattern_stops[updatedStopSequence].default_travel_time = updatedTravelTime;
String updatedPatternOutput = patternUpdater.update(pattern.id, mapper.writeValueAsString(pattern), true);
LOG.info("Updated pattern output: {}", updatedPatternOutput);
// Normalize stop times.
JdbcTableWriter updateTripWriter = createTestTableWriter(tripsTable);
updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0);
updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0, interpolateStopTimes);

return createdTrip.trip_id;
}

/**
* Checks that {@link JdbcTableWriter#normalizeStopTimesForPattern(int, int, boolean)} can interpolate stop times between timepoints.
*/
@Test
public void canInterpolatePatternStopTimes() throws IOException, SQLException, InvalidNamespaceException {
br648 marked this conversation as resolved.
Show resolved Hide resolved
// Parameters are shared with canNormalizePatternStopTimes, but maintained for test flexibility.
int startTime = 6 * 60 * 60; // 6AM
int initialTravelTime = 60; // seconds
int updatedTravelTime = 600; // ten minutes
String patternId = "123456-interpolated";
double[] shapeDistTraveledValues = new double[] {0.0, 300.0, 600.0};
double timepointTravelTime = (shapeDistTraveledValues[2] - shapeDistTraveledValues[0]) / updatedTravelTime; // 1 m/s

// Create the array of patterns, set the timepoints properly.
PatternStopDTO[] patternStops = new PatternStopDTO[]{
new PatternStopDTO(patternId, firstStopId, 0, 1, shapeDistTraveledValues[0]),
new PatternStopDTO(patternId, secondStopId, 1, 0, shapeDistTraveledValues[1]),
new PatternStopDTO(patternId, lastStopId, 2, 1, shapeDistTraveledValues[2]),
};

patternStops[2].default_travel_time = initialTravelTime;

// Pass the array of patterns to the body method with param
String createdTripId = normalizeStopsForPattern(patternStops, 2, true, initialTravelTime, updatedTravelTime, startTime, patternId);

// Read pattern stops from database and check that the arrivals/departures have been updated.
JDBCTableReader<StopTime> stopTimesTable = new JDBCTableReader(Table.STOP_TIMES,
testDataSource,
testNamespace + ".",
EntityPopulator.STOP_TIME);
testDataSource,
testNamespace + ".",
EntityPopulator.STOP_TIME);
int index = 0;
for (StopTime stopTime : stopTimesTable.getOrdered(createdTripId)) {
LOG.info("stop times i={} arrival={} departure={}", index, stopTime.arrival_time, stopTime.departure_time);
int calculatedArrivalTime = (int) (startTime + shapeDistTraveledValues[index] * timepointTravelTime);
assertThat(stopTime.arrival_time, equalTo(calculatedArrivalTime));
index++;
}
}

/**
* Checks that {@link JdbcTableWriter#normalizeStopTimesForPattern(int, int, boolean)} can normalize stop times to a pattern's
* default travel times.
*/
@Test
public void canNormalizePatternStopTimes() throws IOException, SQLException, InvalidNamespaceException {
// Parameters are shared with canNormalizePatternStopTimes, but maintained for test flexibility.
int initialTravelTime = 60; // one minute
int startTime = 6 * 60 * 60; // 6AM
int updatedTravelTime = 3600;
String patternId = "123456";

PatternStopDTO[] patternStops = new PatternStopDTO[]{
new PatternStopDTO(patternId, firstStopId, 0),
new PatternStopDTO(patternId, lastStopId, 1)
};

String createdTripId = normalizeStopsForPattern(patternStops, 1, false, initialTravelTime, updatedTravelTime, startTime, patternId);
JDBCTableReader<StopTime> stopTimesTable = new JDBCTableReader(Table.STOP_TIMES,
testDataSource,
testNamespace + ".",
EntityPopulator.STOP_TIME);
int index = 0;
for (StopTime stopTime : stopTimesTable.getOrdered(createdTrip.trip_id)) {
for (StopTime stopTime : stopTimesTable.getOrdered(createdTripId)) {
LOG.info("stop times i={} arrival={} departure={}", index, stopTime.arrival_time, stopTime.departure_time);
assertThat(stopTime.arrival_time, equalTo(startTime + index * updatedTravelTime));
index++;
Expand Down Expand Up @@ -990,7 +1041,7 @@ private TripDTO constructFrequencyTrip(String patternId, String routeId, int sta
/**
* Construct (without writing to the database) a timetable trip.
*/
private TripDTO constructTimetableTrip(String patternId, String routeId, int startTime, int travelTime) {
private static TripDTO constructTimetableTrip(String patternId, String routeId, int startTime, int travelTime) {
TripDTO tripInput = new TripDTO();
tripInput.pattern_id = patternId;
tripInput.route_id = routeId;
Expand Down
Loading