Repair Interpolation

.gitignore

@@ -9,4 +9,6 @@
 .idea/
 target/
 
-lambda$*.json
+lambda$*.json
+
+.DS_Store

src/main/java/com/conveyal/gtfs/loader/JdbcTableWriter.java

@@ -257,21 +257,14 @@ 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.
      *
      * @return number of stop times updated
      */
-    public int normalizeStopTimesForPattern(int id, int beginWithSequence, boolean interpolateStopTimes) throws SQLException {
+    public int normalizeStopTimesForPattern(int id, int beginWithSequence, boolean interpolateStopTimes, boolean ignoreNonBlankStopTimes) throws SQLException {
         try {
             JDBCTableReader<PatternStop> patternStops = new JDBCTableReader(
                 Table.PATTERN_STOP,
@@ -288,7 +281,7 @@ public int normalizeStopTimesForPattern(int id, int beginWithSequence, boolean i
                     patternStopsToNormalize.add(patternStop);
                 }
             }
-            int stopTimesUpdated = updateStopTimesForPatternStops(patternStopsToNormalize, interpolateStopTimes);
+            int stopTimesUpdated = updateStopTimesForPatternStops(patternStopsToNormalize, interpolateStopTimes, ignoreNonBlankStopTimes);
             connection.commit();
             return stopTimesUpdated;
         } catch (Exception e) {
@@ -788,27 +781,47 @@ private int interpolateTimesFromTimepoints(
         PatternStop patternStop,
         List<PatternStop> timepoints,
         Integer timepointNumber,
-        double previousShapeDistTraveled
+        PatternStop prevPatternStop
     ) {
         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)");
+            throw new IllegalStateException("Issue in pattern stops which prevents interpolation (e.g. less than 2 stoptimes to interpolate between)");
         }
         PatternStop nextTimepoint = timepoints.get(timepointNumber);
-        PatternStop lastTimepoint = timepoints.get(timepointNumber-1);
+        PatternStop prevTimepoint = timepoints.get(timepointNumber - 1);
 
         if (
             nextTimepoint == null ||
             nextTimepoint.default_travel_time == Entity.INT_MISSING ||
             nextTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING ||
-            lastTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING
+            prevTimepoint.shape_dist_traveled == Entity.DOUBLE_MISSING ||
+            (prevPatternStop != null && prevPatternStop.shape_dist_traveled == Entity.DOUBLE_MISSING)
         ) {
             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;
-        return (int) Math.round((patternStop.shape_dist_traveled - previousShapeDistTraveled) / timepointSpeed);
+        double prevSdt = prevPatternStop == null ? 0 : prevPatternStop.shape_dist_traveled;
+
+        double timepointSpeed = (nextTimepoint.shape_dist_traveled - prevTimepoint.shape_dist_traveled) / nextTimepoint.default_travel_time;
+        return (int) Math.round((patternStop.shape_dist_traveled - prevSdt) / timepointSpeed);
     }
 
+    private boolean checkIfArrivalTimeExists(int stopSequence, String trip_id) throws SQLException {
+        String sql = String.format("select arrival_time from %s.stop_times where stop_sequence = ? and trip_id = ?", tablePrefix);
+        PreparedStatement statement = connection.prepareStatement(sql);
+        statement.setInt(1, stopSequence);
+        statement.setString(2, trip_id);
+        try (ResultSet rs = statement.executeQuery()) {
+            if (!rs.next()) {
+                return false;
+            }
+            int arrivalTime = rs.getInt(1);
+            if (rs.wasNull()) {
+                return false;
+            }
+            return arrivalTime > 0;
+        }
+
+    }
     /**
      * 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
@@ -819,7 +832,11 @@ private int interpolateTimesFromTimepoints(
      *
      * TODO? add param Set<String> serviceIdFilters service_id values to filter trips on
      */
-    private int updateStopTimesForPatternStops(List<PatternStop> patternStops, boolean interpolateStopTimes) throws SQLException {
+    private int updateStopTimesForPatternStops(
+            List<PatternStop> patternStops,
+            boolean interpolateStopTimes,
+            boolean ignoreNonBlankStopTimes
+    ) throws SQLException {
         PatternStop firstPatternStop = patternStops.iterator().next();
         List<PatternStop> timepoints = patternStops.stream().filter(ps -> ps.timepoint == 1).collect(Collectors.toList());
         int firstStopSequence = firstPatternStop.stop_sequence;
@@ -854,43 +871,64 @@ private int updateStopTimesForPatternStops(List<PatternStop> patternStops, boole
         for (String tripId : timesForTripIds.keySet()) {
             // Initialize travel time with previous stop time value.
             int cumulativeTravelTime = timesForTripIds.get(tripId);
-            int cumulativeInterpolatedTime = cumulativeTravelTime;
+            int timepointCumulativeTravelTime = cumulativeTravelTime;
             int timepointNumber = 0;
-            double previousShapeDistTraveled = 0; // Used for calculating timepoint speed for interpolation
+            PatternStop prevPatternStop = null;
             for (PatternStop patternStop : patternStops) {
                 boolean isTimepoint = patternStop.timepoint == 1;
                 if (isTimepoint) timepointNumber++;
+                boolean hasStopTime = !ignoreNonBlankStopTimes || checkIfArrivalTimeExists(patternStop.stop_sequence, tripId);
                 // 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 (interpolateStopTimes) {
+                if (interpolateStopTimes || ignoreNonBlankStopTimes) {
                     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;
+                    // Override travel time if we're interpolating between timepoints, or generating blank stop times
+                    if ((!ignoreNonBlankStopTimes && !isTimepoint) || (ignoreNonBlankStopTimes && !hasStopTime && patternStop.stop_sequence != 0)) {
+                        travelTime = interpolateTimesFromTimepoints(patternStop, timepoints, timepointNumber, prevPatternStop);
+                    }
                 }
-                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.
-                    // 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);
-                } else {
-                    cumulativeTravelTime += travelTime;
-                    updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime);
-                    cumulativeTravelTime += dwellTime;
-                    updateStopTimeStatement.setInt(oneBasedIndex++, cumulativeTravelTime);
+                // When interpolating, assume that *all* non-timepoint data is incorrect/missing
+                // The travel and dwell times defined for stops between timepoints will be ignored.
+                int dwellTime = patternStop.default_dwell_time == Entity.INT_MISSING || (interpolateStopTimes && !isTimepoint) ?
+                        0 : patternStop.default_dwell_time;
+
+                // In non-blank stop time mode, only update if our stop time is blank
+                if (ignoreNonBlankStopTimes) {
+                    if (hasStopTime) {
+                        cumulativeTravelTime += travelTime + dwellTime;
+                        prevPatternStop = patternStop;
+                        continue;
+                    }
+
+                    updateStopTimeStatement.setInt(1, cumulativeTravelTime + travelTime);
+                    updateStopTimeStatement.setInt(2, cumulativeTravelTime + travelTime);
+                    updateStopTimeStatement.setString(3, tripId);
+                    updateStopTimeStatement.setInt(4, patternStop.stop_sequence);
+                    stopTimesTracker.addBatch();
                 }
-                updateStopTimeStatement.setString(oneBasedIndex++, tripId);
-                updateStopTimeStatement.setInt(oneBasedIndex++, patternStop.stop_sequence);
+
+
+                // In interpolation mode, don't write changes to db for timepoints
+                // We don't write the travel time, since it is interpolated
+                if (interpolateStopTimes && isTimepoint) {
+                    timepointCumulativeTravelTime += travelTime + dwellTime;
+                    // Reset the cumulativeTravelTime to the schedule truth
+                    cumulativeTravelTime = timepointCumulativeTravelTime;
+                    prevPatternStop = patternStop;
+                    continue;
+                }
+                // Increase travel time by current pattern stop's travel and dwell times (and set values for update).
+                cumulativeTravelTime += travelTime;
+                updateStopTimeStatement.setInt(1, cumulativeTravelTime);
+                cumulativeTravelTime += dwellTime;
+                updateStopTimeStatement.setInt(2, cumulativeTravelTime);
+                updateStopTimeStatement.setString(3, tripId);
+                updateStopTimeStatement.setInt(4, patternStop.stop_sequence);
                 stopTimesTracker.addBatch();
+
+                prevPatternStop = patternStop;
             }
         }
         return stopTimesTracker.executeRemaining();
@@ -1621,7 +1659,7 @@ private String[] parseExceptionListField(int id, String namespace, Table table,
         return parsedString.replaceAll("[{}]", "").split("[,]", 0);
     }
 
-    private String getResultSetString(int column, ResultSet resultSet) throws java.sql.SQLException {
+    private String getResultSetString(int column, ResultSet resultSet) throws SQLException {
         String resultSetString = resultSet.getString(column);
         return resultSetString == null ? "" : resultSetString;
     }

src/test/java/com/conveyal/gtfs/loader/JDBCTableWriterTest.java

@@ -42,6 +42,7 @@
 import static com.conveyal.gtfs.TestUtils.getResourceFileName;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -62,6 +63,7 @@ public class JDBCTableWriterTest {
     private static DataSource testDataSource;
     private static String testNamespace;
     private static String testGtfsGLSnapshotNamespace;
+    private static String testGtfsBlankStoptimesSnapshotNamespace;
     private static String simpleServiceId = "1";
     private static String firstStopId = "1";
     private static String secondStopId= "1.5";
@@ -109,6 +111,16 @@ public static void setUpClass() throws SQLException, IOException, InvalidNamespa
         JdbcGtfsSnapshotter snapshotter = new JdbcGtfsSnapshotter(testGtfsGLNamespace, testDataSource, false);
         SnapshotResult snapshotResult = snapshotter.copyTables();
         testGtfsGLSnapshotNamespace = snapshotResult.uniqueIdentifier;
+
+        // Load fake-agency-blank-stoptimes feed for use with blank stoptimes insertion test
+        FeedLoadResult blankStoptimesFeedLoadResult = load(TestUtils.zipFolderFiles("fake-agency-blank-stoptimes", true), testDataSource);
+        String testGtfsBlankStoptimesNamespace = blankStoptimesFeedLoadResult.uniqueIdentifier;
+        // validate feed to create additional tables
+        validate(testGtfsBlankStoptimesNamespace, testDataSource);
+        // load into editor via snapshot
+        JdbcGtfsSnapshotter blankStoptimesSnapshotter = new JdbcGtfsSnapshotter(testGtfsBlankStoptimesNamespace, testDataSource, false);
+        SnapshotResult blankStoptimesSnapshotResult = blankStoptimesSnapshotter.copyTables();
+        testGtfsBlankStoptimesSnapshotNamespace = blankStoptimesSnapshotResult.uniqueIdentifier;
     }
 
     @AfterAll
@@ -874,7 +886,7 @@ private static String normalizeStopsForPattern(
         LOG.info("Updated pattern output: {}", updatedPatternOutput);
         // Normalize stop times.
         JdbcTableWriter updateTripWriter = createTestTableWriter(tripsTable);
-        updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0, interpolateStopTimes);
+        updateTripWriter.normalizeStopTimesForPattern(pattern.id, 0, interpolateStopTimes, false);
 
         return createdTrip.trip_id;
     }
@@ -950,6 +962,25 @@ public void canNormalizePatternStopTimes() throws IOException, SQLException, Inv
         assertThat(index, equalTo(patternStops.length));
     }
 
+    @Test
+    public void canInterpolateMissingStoptimes() throws IOException, SQLException, InvalidNamespaceException {
+        JdbcTableWriter tableWriter = new JdbcTableWriter(Table.PATTERN_STOP, testDataSource, testGtfsBlankStoptimesSnapshotNamespace);
+        tableWriter.normalizeStopTimesForPattern(1, 0, false, true);
+
+        JDBCTableReader<StopTime> stopTimesTable = new JDBCTableReader(Table.STOP_TIMES,
+                testDataSource,
+                testGtfsBlankStoptimesSnapshotNamespace + ".",
+                EntityPopulator.STOP_TIME);
+        int index = 0;
+        for (StopTime stopTime : stopTimesTable.getOrdered("1720")) {
+            LOG.info("stop times i={} arrival={} departure={}", index, stopTime.arrival_time, stopTime.departure_time);
+            // These outcomes will be nonsensical, but correct given the nonsensical shape-dist-traveled values
+            assertThat(stopTime.arrival_time, greaterThan(0));
+            assertThat(stopTime.departure_time, greaterThan(0));
+            index++;
+        }
+    }
+
     /**
      * This test makes sure that updated the service_id will properly update affected referenced entities properly.
      * This test case was initially developed to prove that https://github.com/conveyal/gtfs-lib/issues/203 is

src/test/resources/fake-agency-blank-stoptimes/agency.txt

@@ -0,0 +1,2 @@
+agency_id,agency_name,agency_url,agency_phone,agency_timezone,agency_lang
+1,"Tri Delta Transit","http://trideltatransit.com","(925) 754-4040","America/Los_Angeles","en"

src/test/resources/fake-agency-blank-stoptimes/calendar.txt

@@ -0,0 +1,2 @@
+service_id,monday,tuesday,wednesday,thursday,friday,saturday,sunday,start_date,end_date
+1,1,1,1,1,1,1,1,20210828,20221231

src/test/resources/fake-agency-blank-stoptimes/fare_attributes.txt

@@ -0,0 +1,2 @@
+fare_id,price,currency_type,payment_method,transfers,transfer_duration,agency_id
+"1",2.00,USD,0,0,,1

src/test/resources/fake-agency-blank-stoptimes/fare_rules.txt

@@ -0,0 +1,2 @@
+fare_id,route_id
+"1","300"

src/test/resources/fake-agency-blank-stoptimes/routes.txt

@@ -0,0 +1,2 @@
+route_id,agency_id,route_short_name,route_long_name,route_type,route_color,route_text_color,route_url
+"300",1,"300","Brentwood Park & Ride / Antioch BART",3,800080,FFFFFF,"http://12.155.17.20/RTT/Public/Schedule.aspx?RouteNo=300"

src/test/resources/fake-agency-blank-stoptimes/shapes.txt

@@ -0,0 +1,3 @@
+"shape_id","shape_pt_lon","shape_pt_lat","shape_pt_sequence","shape_dist_traveled"
+"27","-121.780946","37.996068","1","0"
+"27","-121.69877","37.933812","2","200"

src/test/resources/fake-agency-blank-stoptimes/stop_times.txt

@@ -0,0 +1,7 @@
+"trip_id","arrival_time","departure_time","stop_id","stop_sequence","stop_headsign","timepoint","shape_dist_traveled"
+"1720","17:46:00","17:46:00","722","0","","1","0"
+"1720","17:59:00","17:59:00","33","2","","1","50"
+"1720","","","900","3","","","150"
+"1720","18:02:01","18:02:01","443","4","","0","200"
+"1720","18:20:06","18:20:06","421","5","","0","250"
+"1720","18:24:00","18:24:00","610","6","","1","300"

src/test/resources/fake-agency-blank-stoptimes/stops.txt

@@ -0,0 +1,7 @@
+"stop_id","stop_code","stop_name","stop_lat","stop_lon","stop_url","zone_id"
+"33","813644","Main St & Empire Ave","37.997839","-121.730492","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=33",""
+"421","810930","Brentwood Blvd & Village Dr","37.941524","-121.696341","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=421",""
+"443","811043","Main St & Norcross Ln","37.997716","-121.716376","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=443",""
+"610","811425","Brentwood Park n Ride","37.933838","-121.698755","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=610",""
+"722","817754","Antioch BART","37.995572","-121.778137","http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=722",""
+"900","817755","test statin","37.995578","-121.778139","",""

src/test/resources/fake-agency-blank-stoptimes/trips.txt

@@ -0,0 +1,2 @@
+route_id,service_id,trip_id,trip_headsign,direction_id,block_id,shape_id
+"300",1,1720,"Eastbound Brentwood Park & Ride",0,167,27