From 1e2adad9e6f6e8f40df424fa80a990cc1462d192 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Sun, 5 Nov 2023 14:10:43 -0500 Subject: [PATCH 01/10] feat(JdbcTableWriter): add stoptime interpolation --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index 86ee2c771..767691227 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -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 { try { JDBCTableReader patternStops = new JDBCTableReader( Table.PATTERN_STOP, @@ -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) { @@ -755,8 +755,9 @@ private int updateStopTimesForPatternStop(ObjectNode patternStop, int previousTr * * TODO? add param Set serviceIdFilters service_id values to filter trips on */ - private int updateStopTimesForPatternStops(List patternStops) throws SQLException { + private int updateStopTimesForPatternStops(List patternStops, boolean interpolateStopTimes) throws SQLException { PatternStop firstPatternStop = patternStops.iterator().next(); + List 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; @@ -789,9 +790,25 @@ private int updateStopTimesForPatternStops(List patternStops) throw for (String tripId : timesForTripIds.keySet()) { // Initialize travel time with previous stop time value. int cumulativeTravelTime = timesForTripIds.get(tripId); + int timepointNumber = 0; + double timepointSpeed = 0; + double previousShapeDistTraveled = 0; + PatternStop currentTimepoint = patternStops.get(0); // First stop must be a timepoint 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){ + timepointNumber++; + previousShapeDistTraveled = currentTimepoint.shape_dist_traveled; + PatternStop nextTimePoint = timepoints.get(timepointNumber); + timepointSpeed = (nextTimePoint.shape_dist_traveled - currentTimepoint.shape_dist_traveled) / nextTimePoint.default_travel_time; + } // 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). From 5f34e048ab92936b98e344e69d8a41c9cf978dc4 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Sun, 5 Nov 2023 14:15:31 -0500 Subject: [PATCH 02/10] refactor(JdbcTableWriter): update tests --- .../java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index 62f2a8d57..3f80b5ce8 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -839,7 +839,7 @@ public void canCreateUpdateAndDeleteFrequencyTripForFrequencyPattern() throws IO } /** - * Checks that {@link JdbcTableWriter#normalizeStopTimesForPattern(int, int)} can normalize stop times to a pattern's + * Checks that {@link JdbcTableWriter#normalizeStopTimesForPattern(int, int, boolean)} can normalize stop times to a pattern's * default travel times. */ @Test @@ -875,7 +875,7 @@ public void canNormalizePatternStopTimes() throws IOException, SQLException, Inv LOG.info("Updated pattern output: {}", updatedPatternOutput); // Normalize stop times. JdbcTableWriter updateTripWriter = createTestTableWriter(tripsTable); - updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0); + updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0, false); // Read pattern stops from database and check that the arrivals/departures have been updated. JDBCTableReader stopTimesTable = new JDBCTableReader(Table.STOP_TIMES, testDataSource, From b585c9df6e944b927859e7f6948dc5c2ab1080ff Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Mon, 13 Nov 2023 17:18:54 -0500 Subject: [PATCH 03/10] refactor(JDBCTableWriter): add tests, respond to comments --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 16 +-- .../com/conveyal/gtfs/dto/PatternStopDTO.java | 8 ++ .../gtfs/loader/JDBCTableWriterTest.java | 108 +++++++++++++----- 3 files changed, 96 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index 767691227..753c98c07 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -789,19 +789,21 @@ private int updateStopTimesForPatternStops(List patternStops, boole 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 timepointNumber = 0; - double timepointSpeed = 0; - double previousShapeDistTraveled = 0; + int cumulativeTravelTime = timesForTripIds.get(tripId), timepointNumber = 0; + double timepointSpeed = 0, previousShapeDistTraveled = 0; PatternStop currentTimepoint = patternStops.get(0); // First stop must be a timepoint + 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){ - timepointNumber++; + if (isTimepoint && interpolateStopTimes && timepointNumber++ < timepoints.size()-1){ previousShapeDistTraveled = currentTimepoint.shape_dist_traveled; PatternStop nextTimePoint = timepoints.get(timepointNumber); - timepointSpeed = (nextTimePoint.shape_dist_traveled - currentTimepoint.shape_dist_traveled) / nextTimePoint.default_travel_time; + 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; } // 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; diff --git a/src/test/java/com/conveyal/gtfs/dto/PatternStopDTO.java b/src/test/java/com/conveyal/gtfs/dto/PatternStopDTO.java index e089041d4..cba32143b 100644 --- a/src/test/java/com/conveyal/gtfs/dto/PatternStopDTO.java +++ b/src/test/java/com/conveyal/gtfs/dto/PatternStopDTO.java @@ -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; + } } diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index 3f80b5ce8..d7ef12b27 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -64,9 +64,13 @@ 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 String patternId = "123456"; 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"; @@ -93,6 +97,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()} **/ @@ -838,29 +843,17 @@ public void canCreateUpdateAndDeleteFrequencyTripForFrequencyPattern() throws IO )); } - /** - * 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 { - // 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) 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); @@ -869,20 +862,77 @@ 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, false); + 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 { + // 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 + 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); + // Read pattern stops from database and check that the arrivals/departures have been updated. JDBCTableReader 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; + + PatternStopDTO[] patternStops = new PatternStopDTO[]{ + new PatternStopDTO(patternId, firstStopId, 0), + new PatternStopDTO(patternId, lastStopId, 1) + }; + + String createdTripId = normalizeStopsForPattern(patternStops, 1, false, initialTravelTime, updatedTravelTime, startTime); + JDBCTableReader 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++; @@ -990,7 +1040,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; From 36ccb0952b40f70eaa632dbf17a7532c67479ea5 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Mon, 13 Nov 2023 17:29:49 -0500 Subject: [PATCH 04/10] refactor(JDBCTableWriter): don't share pattern IDs --- .../com/conveyal/gtfs/loader/JDBCTableWriterTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index d7ef12b27..1b1d4df7e 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -66,7 +66,6 @@ public class JDBCTableWriterTest { private static String firstStopId = "1"; private static String secondStopId= "1.5"; private static String lastStopId = "2"; - private static String patternId = "123456"; private static double firstStopLat = 34.2222; private static double firstStopLon = -87.333; private static double secondStopLat = 34.2227; @@ -843,7 +842,7 @@ public void canCreateUpdateAndDeleteFrequencyTripForFrequencyPattern() throws IO )); } - private static String normalizeStopsForPattern(PatternStopDTO[] patternStops, int updatedStopSequence, boolean interpolateStopTimes, int initialTravelTime, int updatedTravelTime, int startTime) throws SQLException, InvalidNamespaceException, IOException { + 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; PatternDTO pattern = createRouteAndPattern(newUUID(), @@ -881,6 +880,7 @@ public void canInterpolatePatternStopTimes() throws IOException, SQLException, I 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 @@ -894,7 +894,7 @@ public void canInterpolatePatternStopTimes() throws IOException, SQLException, I 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); + 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 stopTimesTable = new JDBCTableReader(Table.STOP_TIMES, @@ -920,13 +920,14 @@ public void canNormalizePatternStopTimes() throws IOException, SQLException, Inv 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); + String createdTripId = normalizeStopsForPattern(patternStops, 1, false, initialTravelTime, updatedTravelTime, startTime, patternId); JDBCTableReader stopTimesTable = new JDBCTableReader(Table.STOP_TIMES, testDataSource, testNamespace + ".", From 8a1fe2a240a4815e65eae1ec70bc449ebbf06473 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Fri, 17 Nov 2023 14:01:57 -0500 Subject: [PATCH 05/10] refactor(JdbcTableWriter): move interpolation into separate method --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index 753c98c07..a8d607055 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -745,6 +745,33 @@ private int updateStopTimesForPatternStop(ObjectNode patternStop, int previousTr return travelTime + dwellTime; } + private int interpolateTimesFromTimepoints( + PatternStop patternStop, + List 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; + 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 @@ -789,28 +816,19 @@ private int updateStopTimesForPatternStops(List patternStops, boole 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), timepointNumber = 0; - double timepointSpeed = 0, previousShapeDistTraveled = 0; - PatternStop currentTimepoint = patternStops.get(0); // First stop must be a timepoint - if (currentTimepoint.timepoint != 1 && interpolateStopTimes) { - throw new IllegalStateException("First pattern stop must be a timepoint to perform interpolation"); - } + int cumulativeTravelTime = timesForTripIds.get(tripId); + int timepointNumber = 0; + double previousShapeDistTraveled = 0; // Used for calculating timepoint speed for 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){ - previousShapeDistTraveled = currentTimepoint.shape_dist_traveled; - PatternStop nextTimePoint = timepoints.get(timepointNumber); - 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; - } + 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) { - travelTime = (int) Math.round((patternStop.shape_dist_traveled - previousShapeDistTraveled) / timepointSpeed); - previousShapeDistTraveled += patternStop.shape_dist_traveled; + // Override travel time if we're interpolating between timepoints + travelTime = interpolateTimesFromTimepoints(patternStop, timepoints, timepointNumber, previousShapeDistTraveled); } + 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). From 36f3043f11e761ea3a738f61174ec4ab5e363c31 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Mon, 20 Nov 2023 15:59:08 -0500 Subject: [PATCH 06/10] refactor(st interpolation): Update calculation method, respond to comments --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 25 ++++++++++++++++--- .../gtfs/loader/JDBCTableWriterTest.java | 12 +++++++-- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index a8d607055..01d8d2d8c 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -256,6 +256,13 @@ 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); + } /** * For a given pattern id and starting stop sequence (inclusive), normalize all stop times to match the pattern * stops' travel times. @@ -817,6 +824,7 @@ private int updateStopTimesForPatternStops(List patternStops, boole 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) { @@ -832,10 +840,19 @@ private int updateStopTimesForPatternStops(List patternStops, boole 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. + 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(); diff --git a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java index 1b1d4df7e..44dab2b04 100644 --- a/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java +++ b/src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java @@ -842,7 +842,15 @@ public void canCreateUpdateAndDeleteFrequencyTripForFrequencyPattern() throws IO )); } - private static String normalizeStopsForPattern(PatternStopDTO[] patternStops, int updatedStopSequence, boolean interpolateStopTimes, int initialTravelTime, int updatedTravelTime, int startTime, String patternId) throws SQLException, InvalidNamespaceException, IOException { + 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; PatternDTO pattern = createRouteAndPattern(newUUID(), @@ -875,7 +883,7 @@ private static String normalizeStopsForPattern(PatternStopDTO[] patternStops, in * Checks that {@link JdbcTableWriter#normalizeStopTimesForPattern(int, int, boolean)} can interpolate stop times between timepoints. */ @Test - public void canInterpolatePatternStopTimes() throws IOException, SQLException, InvalidNamespaceException { + 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 From dbd09ce019932751cb535be4e475469519e3268a Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Tue, 21 Nov 2023 10:11:42 -0500 Subject: [PATCH 07/10] refactor(st interpolation): respond to PR comments --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index 01d8d2d8c..0a400d0d6 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -263,6 +263,7 @@ public String update(Integer id, String json, boolean autoCommit) throws SQLExce public int normalizeStopTimesForPattern(int id, int beginWithSequence) throws SQLException { return normalizeStopTimesForPattern(id, beginWithSequence, false); } + /** * For a given pattern id and starting stop sequence (inclusive), normalize all stop times to match the pattern * stops' travel times. @@ -752,6 +753,16 @@ private int updateStopTimesForPatternStop(ObjectNode patternStop, int previousTr return travelTime + dwellTime; } + /** + * Updates the non-timepoint stop times between two timepoints using the speed implied by + * the travel time between them. Ignores any existing default_travel_time or default_dwell_time + * entered for the non-timepoint stops. + * @param patternStop + * @param timepoints + * @param timepointNumber + * @param previousShapeDistTraveled + * @return + */ private int interpolateTimesFromTimepoints( PatternStop patternStop, List timepoints, @@ -772,6 +783,11 @@ private int interpolateTimesFromTimepoints( 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."); + } else if ( + nextTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING || + lastTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING + ) { + throw new IllegalStateException("Shape_dist_traveled must be defined in order to perform interpolation."); } double timepointSpeed = (nextTimepoint.shape_dist_traveled - lastTimepoint.shape_dist_traveled) / nextTimepoint.default_travel_time; @@ -832,18 +848,23 @@ private int updateStopTimesForPatternStops(List patternStops, boole 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 - travelTime = interpolateTimesFromTimepoints(patternStop, timepoints, timepointNumber, previousShapeDistTraveled); + if (interpolateStopTimes) { + if (patternStop.shape_dist_traveled == Entity.DOUBLE_MISSING) { + throw new IllegalStateException("Shape_dist_traveled must be defined for all stops in order to perform interpolation"); + } + // Override travel time if we're interpolating between timepoints. + if (!isTimepoint) travelTime = interpolateTimesFromTimepoints(patternStop, timepoints, timepointNumber, previousShapeDistTraveled); + previousShapeDistTraveled += patternStop.shape_dist_traveled; } - 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). 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. + // Dwell times are ignored right now as they do not fit the typical use case for interpolation. + // They may be incorporated by accounting for all dwell times in intermediate stops when calculating + // the timepoint speed. cumulativeInterpolatedTime += travelTime; updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeInterpolatedTime); updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeInterpolatedTime); From da9cc6bb90e43969215ccf0c8e85a32bbe29063e Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Tue, 21 Nov 2023 11:40:42 -0500 Subject: [PATCH 08/10] refactor(st interpolation): update illegalState text --- .../conveyal/gtfs/loader/JdbcTableWriter.java | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index 0a400d0d6..c017b571e 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -764,30 +764,24 @@ private int updateStopTimesForPatternStop(ObjectNode patternStop, int previousTr * @return */ private int interpolateTimesFromTimepoints( - PatternStop patternStop, - List timepoints, - Integer timepointNumber, - double previousShapeDistTraveled + PatternStop patternStop, + List 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"); + if (timepointNumber == 0 || timepoints.size() == 1 || timepointNumber >= timepoints.size()) { + throw new IllegalStateException("Issue in pattern stops which prevents interpolation (e.g. less than 2 timepoints)"); } 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."); - } else if ( + if ( + nextTimepoint == null || + nextTimepoint.default_travel_time == Entity.INT_MISSING || nextTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING || lastTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING ) { - throw new IllegalStateException("Shape_dist_traveled must be defined in order to perform interpolation."); + throw new IllegalStateException("Error with stop time interpolation: timepoint or shape_dist_traveled is null"); } double timepointSpeed = (nextTimepoint.shape_dist_traveled - lastTimepoint.shape_dist_traveled) / nextTimepoint.default_travel_time; From 5fd655d3b09cc756df5e355e24d83346d1cbc845 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Tue, 21 Nov 2023 11:45:52 -0500 Subject: [PATCH 09/10] refactor(st interpolation): update formatting --- src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index c017b571e..f29d07b44 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -785,8 +785,7 @@ private int interpolateTimesFromTimepoints( } double timepointSpeed = (nextTimepoint.shape_dist_traveled - lastTimepoint.shape_dist_traveled) / nextTimepoint.default_travel_time; - int travelTime = (int) Math.round((patternStop.shape_dist_traveled - previousShapeDistTraveled) / timepointSpeed); - return travelTime; + return (int) Math.round((patternStop.shape_dist_traveled - previousShapeDistTraveled) / timepointSpeed); } /** From a331d7475138bb3ac23dd25960313f69d07c38b7 Mon Sep 17 00:00:00 2001 From: "philip.cline" Date: Tue, 21 Nov 2023 11:46:37 -0500 Subject: [PATCH 10/10] refactor(st interpolation): updating method description --- src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java index f29d07b44..c2fb3758f 100644 --- a/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java +++ b/src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java @@ -757,11 +757,6 @@ private int updateStopTimesForPatternStop(ObjectNode patternStop, int previousTr * Updates the non-timepoint stop times between two timepoints using the speed implied by * the travel time between them. Ignores any existing default_travel_time or default_dwell_time * entered for the non-timepoint stops. - * @param patternStop - * @param timepoints - * @param timepointNumber - * @param previousShapeDistTraveled - * @return */ private int interpolateTimesFromTimepoints( PatternStop patternStop,