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
68 changes: 61 additions & 7 deletions src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,20 @@ public String update(Integer id, String json, boolean autoCommit) throws SQLExce
}
}

/**
* Deprecated method to normalize stop times before stop time interpolation. Defaults to
* false for interpolation.
*/
public int normalizeStopTimesForPattern(int id, int beginWithSequence) throws SQLException {
return normalizeStopTimesForPattern(id, beginWithSequence, false);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit. New line after closing bracket.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in dbd09ce

/**
* For a given pattern id and starting stop sequence (inclusive), normalize all stop times to match the pattern
* stops' travel times.
*
* @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 +286,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 @@ -745,6 +752,33 @@ private int updateStopTimesForPatternStop(ObjectNode patternStop, int previousTr
return travelTime + dwellTime;
}

private int interpolateTimesFromTimepoints(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Method comment to describe what this is doing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added in dbd09ce

PatternStop patternStop,
List<PatternStop> timepoints,
Integer timepointNumber,
double previousShapeDistTraveled
) {
if (timepointNumber == 0) {
throw new IllegalStateException("First pattern stop must be a timepoint to perform interpolation");
} else if (timepoints.size() == 1) {
throw new IllegalStateException("Pattern must have more than one timepoint to perform interpolation");
} else if (timepointNumber >= timepoints.size()) {
throw new IllegalStateException("Last stop must be a timepoint to perform interpolation");
}
PatternStop nextTimepoint = timepoints.get(timepointNumber);
PatternStop lastTimepoint = timepoints.get(timepointNumber-1);

if (nextTimepoint == null) {
throw new IllegalStateException("Stop time interpolation is not possible with null timepoints.");
} else if (nextTimepoint.default_travel_time == Entity.INT_MISSING) {
throw new IllegalStateException("All timepoints must have a default travel time specified.");
}

double timepointSpeed = (nextTimepoint.shape_dist_traveled - lastTimepoint.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.

If shape_dist_traveled is not defined and set to Double.MIN_VALUE, isn't that going to cause issues here?

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 point, I added checks for every pattern stop and both timepoints in dbd09ce

int travelTime = (int) Math.round((patternStop.shape_dist_traveled - previousShapeDistTraveled) / timepointSpeed);
return travelTime;
}

/**
* Normalizes all stop times' arrivals and departures for an ordered set of pattern stops. This set can be the full
* set of stops for a pattern or just a subset. Typical usage for this method would be to overwrite the arrival and
Expand All @@ -755,8 +789,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 @@ -789,16 +824,35 @@ private int updateStopTimesForPatternStops(List<PatternStop> patternStops) throw
for (String tripId : timesForTripIds.keySet()) {
// Initialize travel time with previous stop time value.
int cumulativeTravelTime = timesForTripIds.get(tripId);
int cumulativeInterpolatedTime = cumulativeTravelTime;
int timepointNumber = 0;
double previousShapeDistTraveled = 0; // Used for calculating timepoint speed for interpolation
for (PatternStop patternStop : patternStops) {
boolean isTimepoint = patternStop.timepoint == 1;
if (isTimepoint) timepointNumber++;
// 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) {
// Override travel time if we're interpolating between timepoints
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit. Full stop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added in dbd09ce

travelTime = interpolateTimesFromTimepoints(patternStop, timepoints, timepointNumber, previousShapeDistTraveled);
}
previousShapeDistTraveled += patternStop.shape_dist_traveled;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If any shape_dist_traveled is set to Double.MIN_VALUE, will that cause an issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

likewise yes, I added a check here too dbd09ce

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).
cumulativeTravelTime += travelTime;
updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime);
cumulativeTravelTime += dwellTime;
updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime);
if (!isTimepoint && interpolateStopTimes) {
// We don't want to increment the true cumulative travel time because that adjusts the timepoint
// times later in the pattern.
// TODO? We ignore dwell times in interpolation calculations right now.
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODOs have a habit of not being done. Can dwell times be incorporated easily now? If not, explain why they are ignored and remove the TODO.

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 added an explanation of why this is left out, and an explanation of how it could be implemented. Right now there's not much reason to since it doesn't fit the use case which is driving this feature.

cumulativeInterpolatedTime += travelTime;
updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeInterpolatedTime);
updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeInterpolatedTime);
} else {
cumulativeTravelTime += travelTime;
updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime);
cumulativeTravelTime += dwellTime;
updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime);
}
updateStopTimeStatement.setString(oneBasedIndex++, tripId);
updateStopTimeStatement.setInt(oneBasedIndex++, patternStop.stop_sequence);
stopTimesTracker.addBatch();
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;
}
}
117 changes: 88 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,25 @@ 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 {
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 +869,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
private void canInterpolatePatternStopTimes() throws IOException, SQLException, InvalidNamespaceException {
// 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 +1049,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