Skip to content

Commit

Permalink
report which repeated stops were removed, fixes #2087
Browse files Browse the repository at this point in the history
  • Loading branch information
@abyrd
abyrd committed Aug 6, 2015
1 parent 3f6ce78 commit 92cbaed
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 34 deletions.
10 changes: 7 additions & 3 deletions src/main/java/org/opentripplanner/graph_builder/annotation/RepeatedStops.java
View file
Expand Up @@ -13,23 +13,27 @@ the License, or (at your option) any later version.


package org.opentripplanner.graph_builder.annotation; package org.opentripplanner.graph_builder.annotation;


import gnu.trove.list.TIntList;
import org.onebusaway.gtfs.model.Trip; import org.onebusaway.gtfs.model.Trip;


public class RepeatedStops extends GraphBuilderAnnotation { public class RepeatedStops extends GraphBuilderAnnotation {


private static final long serialVersionUID = 1L; 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 Trip trip;

public final TIntList removedStopSequences;


public RepeatedStops(Trip trip){ public RepeatedStops(Trip trip, TIntList removedStopSequences){
this.trip = trip; this.trip = trip;
this.removedStopSequences = removedStopSequences;
} }


@Override @Override
public String getMessage() { public String getMessage() {
return String.format(FMT, trip); return String.format(FMT, trip.getId(), removedStopSequences);
} }


} }
76 changes: 45 additions & 31 deletions src/main/java/org/opentripplanner/routing/edgetype/factory/GTFSPatternHopFactory.java
View file
Expand Up @@ -13,17 +13,21 @@ the License, or (at your option) any later version.


package org.opentripplanner.routing.edgetype.factory; package org.opentripplanner.routing.edgetype.factory;


import java.util.ArrayList; import com.beust.jcommander.internal.Maps;
import java.util.Arrays; import com.google.common.base.Strings;
import java.util.Collection; import com.google.common.collect.ArrayListMultimap;
import java.util.Collections; import com.google.common.collect.HashMultimap;
import java.util.Comparator; import com.google.common.collect.ListMultimap;
import java.util.HashMap; import com.google.common.collect.Multimap;
import java.util.Iterator; import com.vividsolutions.jts.geom.Coordinate;
import java.util.LinkedList; import com.vividsolutions.jts.geom.CoordinateSequence;
import java.util.List; import com.vividsolutions.jts.geom.Geometry;
import java.util.Map; 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.apache.commons.math3.util.FastMath;
import org.onebusaway.gtfs.model.Agency; import org.onebusaway.gtfs.model.Agency;
import org.onebusaway.gtfs.model.AgencyAndId; import org.onebusaway.gtfs.model.AgencyAndId;
Expand All @@ -41,7 +45,18 @@ the License, or (at your option) any later version.
import org.opentripplanner.common.geometry.PackedCoordinateSequence; import org.opentripplanner.common.geometry.PackedCoordinateSequence;
import org.opentripplanner.common.geometry.SphericalDistanceLibrary; import org.opentripplanner.common.geometry.SphericalDistanceLibrary;
import org.opentripplanner.common.model.P2; 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.GtfsContext;
import org.opentripplanner.gtfs.GtfsLibrary; import org.opentripplanner.gtfs.GtfsLibrary;
import org.opentripplanner.model.StopPattern; import org.opentripplanner.model.StopPattern;
Expand Down Expand Up @@ -76,19 +91,16 @@ the License, or (at your option) any later version.
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;


import com.beust.jcommander.internal.Maps; import java.util.ArrayList;
import com.google.common.base.Strings; import java.util.Arrays;
import com.google.common.collect.ArrayListMultimap; import java.util.Collection;
import com.google.common.collect.ListMultimap; import java.util.Collections;
import com.google.common.collect.Multimap; import java.util.Comparator;
import com.google.common.collect.HashMultimap; import java.util.HashMap;
import com.vividsolutions.jts.geom.Coordinate; import java.util.Iterator;
import com.vividsolutions.jts.geom.CoordinateSequence; import java.util.LinkedList;
import com.vividsolutions.jts.geom.Geometry; import java.util.List;
import com.vividsolutions.jts.geom.GeometryFactory; import java.util.Map;
import com.vividsolutions.jts.geom.LineString;
import com.vividsolutions.jts.linearref.LinearLocation;
import com.vividsolutions.jts.linearref.LocationIndexedLine;


// Filtering out (removing) stoptimes from a trip forces us to either have two copies of that list, // 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. // or do all the steps within one loop over trips. It would be clearer if there were multiple loops over the trips.
Expand Down Expand Up @@ -367,8 +379,9 @@ public void run(Graph graph) {
List<StopTime> stopTimes = new ArrayList<StopTime>(_dao.getStopTimesForTrip(trip)); List<StopTime> stopTimes = new ArrayList<StopTime>(_dao.getStopTimesForTrip(trip));


/* GTFS stop times frequently contain duplicate, missing, or incorrect entries. Repair them. */ /* GTFS stop times frequently contain duplicate, missing, or incorrect entries. Repair them. */
if (removeRepeatedStops(stopTimes)) { TIntList removedStopSequences = removeRepeatedStops(stopTimes);
LOG.warn(graph.addBuilderAnnotation(new RepeatedStops(trip))); if (!removedStopSequences.isEmpty()) {
LOG.warn(graph.addBuilderAnnotation(new RepeatedStops(trip, removedStopSequences)));
} }
filterStopTimes(stopTimes, graph); filterStopTimes(stopTimes, graph);
interpolateStopTimes(stopTimes); interpolateStopTimes(stopTimes);
Expand Down Expand Up @@ -1287,23 +1300,24 @@ private LinearLocation getSegmentFraction(double[] distances, double distance) {
* *
* @return whether any repeated stops were filtered out. * @return whether any repeated stops were filtered out.
*/ */
private boolean removeRepeatedStops (List<StopTime> stopTimes) { private TIntList removeRepeatedStops (List<StopTime> stopTimes) {
boolean filtered = false; boolean filtered = false;
StopTime prev = null; StopTime prev = null;
Iterator<StopTime> it = stopTimes.iterator(); Iterator<StopTime> it = stopTimes.iterator();
TIntList stopSequencesRemoved = new TIntArrayList();
while (it.hasNext()) { while (it.hasNext()) {
StopTime st = it.next(); StopTime st = it.next();
if (prev != null) { if (prev != null) {
if (prev.getStop().equals(st.getStop())) { if (prev.getStop().equals(st.getStop())) {
// OBA gives us unmodifiable lists, but we have copied them. // OBA gives us unmodifiable lists, but we have copied them.
prev.setDepartureTime(st.getDepartureTime()); st.setDepartureTime(st.getDepartureTime());

This comment has been minimized.

Sign in to view

Copy link
@buma

buma Sep 3, 2015

Contributor

I think this line is a reason for #2113
it.remove(); it.remove();
filtered = true; stopSequencesRemoved.add(st.getStopSequence());
} }
} }
prev = st; prev = st;
} }
return filtered; return stopSequencesRemoved;
} }


public void setFareServiceFactory(FareServiceFactory fareServiceFactory) { public void setFareServiceFactory(FareServiceFactory fareServiceFactory) {
Expand Down

0 comments on commit 92cbaed

Please sign in to comment.