From 92cbaed363036ee0c2c21f207baf99b0e1efd8e9 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Thu, 6 Aug 2015 20:26:44 +0200 Subject: [PATCH] report which repeated stops were removed, fixes #2087 --- .../annotation/RepeatedStops.java | 10 ++- .../factory/GTFSPatternHopFactory.java | 76 +++++++++++-------- 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/opentripplanner/graph_builder/annotation/RepeatedStops.java b/src/main/java/org/opentripplanner/graph_builder/annotation/RepeatedStops.java index 3d7e90b7112..9c51c92f843 100644 --- a/src/main/java/org/opentripplanner/graph_builder/annotation/RepeatedStops.java +++ b/src/main/java/org/opentripplanner/graph_builder/annotation/RepeatedStops.java @@ -13,23 +13,27 @@ the License, or (at your option) any later version. package org.opentripplanner.graph_builder.annotation; +import gnu.trove.list.TIntList; import org.onebusaway.gtfs.model.Trip; public class RepeatedStops extends GraphBuilderAnnotation { private static final long serialVersionUID = 1L; - public static final String FMT = "Trip %s visits stops repeatedly. Removed duplicates."; + public static final String FMT = "Trip %s visits stops repeatedly. Removed duplicates at stop sequence numbers %s."; public final Trip trip; + + public final TIntList removedStopSequences; - public RepeatedStops(Trip trip){ + public RepeatedStops(Trip trip, TIntList removedStopSequences){ this.trip = trip; + this.removedStopSequences = removedStopSequences; } @Override public String getMessage() { - return String.format(FMT, trip); + return String.format(FMT, trip.getId(), removedStopSequences); } } diff --git a/src/main/java/org/opentripplanner/routing/edgetype/factory/GTFSPatternHopFactory.java b/src/main/java/org/opentripplanner/routing/edgetype/factory/GTFSPatternHopFactory.java index c7b02801232..d22ca32d1e5 100644 --- a/src/main/java/org/opentripplanner/routing/edgetype/factory/GTFSPatternHopFactory.java +++ b/src/main/java/org/opentripplanner/routing/edgetype/factory/GTFSPatternHopFactory.java @@ -13,17 +13,21 @@ the License, or (at your option) any later version. package org.opentripplanner.routing.edgetype.factory; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; - +import com.beust.jcommander.internal.Maps; +import com.google.common.base.Strings; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.ListMultimap; +import com.google.common.collect.Multimap; +import com.vividsolutions.jts.geom.Coordinate; +import com.vividsolutions.jts.geom.CoordinateSequence; +import com.vividsolutions.jts.geom.Geometry; +import com.vividsolutions.jts.geom.GeometryFactory; +import com.vividsolutions.jts.geom.LineString; +import com.vividsolutions.jts.linearref.LinearLocation; +import com.vividsolutions.jts.linearref.LocationIndexedLine; +import gnu.trove.list.TIntList; +import gnu.trove.list.array.TIntArrayList; import org.apache.commons.math3.util.FastMath; import org.onebusaway.gtfs.model.Agency; import org.onebusaway.gtfs.model.AgencyAndId; @@ -41,7 +45,18 @@ the License, or (at your option) any later version. import org.opentripplanner.common.geometry.PackedCoordinateSequence; import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.model.P2; -import org.opentripplanner.graph_builder.annotation.*; +import org.opentripplanner.graph_builder.annotation.BogusShapeDistanceTraveled; +import org.opentripplanner.graph_builder.annotation.BogusShapeGeometry; +import org.opentripplanner.graph_builder.annotation.BogusShapeGeometryCaught; +import org.opentripplanner.graph_builder.annotation.HopSpeedFast; +import org.opentripplanner.graph_builder.annotation.HopSpeedSlow; +import org.opentripplanner.graph_builder.annotation.HopZeroTime; +import org.opentripplanner.graph_builder.annotation.NegativeDwellTime; +import org.opentripplanner.graph_builder.annotation.NegativeHopTime; +import org.opentripplanner.graph_builder.annotation.NonStationParentStation; +import org.opentripplanner.graph_builder.annotation.RepeatedStops; +import org.opentripplanner.graph_builder.annotation.TripDegenerate; +import org.opentripplanner.graph_builder.annotation.TripUndefinedService; import org.opentripplanner.gtfs.GtfsContext; import org.opentripplanner.gtfs.GtfsLibrary; import org.opentripplanner.model.StopPattern; @@ -76,19 +91,16 @@ the License, or (at your option) any later version. import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.beust.jcommander.internal.Maps; -import com.google.common.base.Strings; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ListMultimap; -import com.google.common.collect.Multimap; -import com.google.common.collect.HashMultimap; -import com.vividsolutions.jts.geom.Coordinate; -import com.vividsolutions.jts.geom.CoordinateSequence; -import com.vividsolutions.jts.geom.Geometry; -import com.vividsolutions.jts.geom.GeometryFactory; -import com.vividsolutions.jts.geom.LineString; -import com.vividsolutions.jts.linearref.LinearLocation; -import com.vividsolutions.jts.linearref.LocationIndexedLine; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; // Filtering out (removing) stoptimes from a trip forces us to either have two copies of that list, // or do all the steps within one loop over trips. It would be clearer if there were multiple loops over the trips. @@ -367,8 +379,9 @@ public void run(Graph graph) { List stopTimes = new ArrayList(_dao.getStopTimesForTrip(trip)); /* GTFS stop times frequently contain duplicate, missing, or incorrect entries. Repair them. */ - if (removeRepeatedStops(stopTimes)) { - LOG.warn(graph.addBuilderAnnotation(new RepeatedStops(trip))); + TIntList removedStopSequences = removeRepeatedStops(stopTimes); + if (!removedStopSequences.isEmpty()) { + LOG.warn(graph.addBuilderAnnotation(new RepeatedStops(trip, removedStopSequences))); } filterStopTimes(stopTimes, graph); interpolateStopTimes(stopTimes); @@ -1287,23 +1300,24 @@ private LinearLocation getSegmentFraction(double[] distances, double distance) { * * @return whether any repeated stops were filtered out. */ - private boolean removeRepeatedStops (List stopTimes) { + private TIntList removeRepeatedStops (List stopTimes) { boolean filtered = false; StopTime prev = null; Iterator it = stopTimes.iterator(); + TIntList stopSequencesRemoved = new TIntArrayList(); while (it.hasNext()) { StopTime st = it.next(); if (prev != null) { if (prev.getStop().equals(st.getStop())) { // OBA gives us unmodifiable lists, but we have copied them. - prev.setDepartureTime(st.getDepartureTime()); + st.setDepartureTime(st.getDepartureTime()); it.remove(); - filtered = true; + stopSequencesRemoved.add(st.getStopSequence()); } } prev = st; } - return filtered; + return stopSequencesRemoved; } public void setFareServiceFactory(FareServiceFactory fareServiceFactory) {