Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

bug fix regarding #947 #956

Closed
wants to merge 7 commits into from

2 participants

@benmoovit

a bug fix, regarding #947
that bug allowed to concerted stops in a removed island

@benmoovit benmoovit closed this
@abyrd
Owner

Is there a reason why we are using the logging framework to write out the island pruning and stops alert file? I am going to commit changes to use a normal PrintWriter. @benmoovit, let me know if this breaks anything for you.

Owner

Also, please remove the default Javadoc and add comments to all these classes explaining what they are for.

@abyrd, Hi Andrew I've removed the IntelliJ comments and added some others when needed. sorry for the troubles.
Regards using the logging framework for save output to files, I found it very convenient to use the logging to save files it allow easy modification and control files access. you can change it if you wont it shouldn't break anything for me.
BTW, why do you think it is wrong to use the logging framework to save csv files?

Owner

I don't think it's "wrong" in that it does produce a usable output file, but a good old fprintf does the job without external configuration files etc. The real reason I made the changes was that I was switching logging frameworks so the old logging calls no longer worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 6, 2013
  1. @benmoovit

    stops alerts frist commit

    benmoovit authored
  2. @benmoovit

    Merge branch 'master' into FindStationAtBuildGraph

    benmoovit authored
    Conflicts:
    	opentripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/PruneFloatingIslands.java
Commits on Jan 7, 2013
  1. @benmoovit
Commits on Jan 9, 2013
  1. @benmoovit
Commits on Jan 13, 2013
  1. @benmoovit
  2. @benmoovit
  3. @benmoovit
This page is out of date. Refresh to see the latest.
Showing with 321 additions and 17 deletions.
  1. +35 −0 opentripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/LoggerAppenderProvider.java
  2. +2 −14 opentripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/PruneFloatingIslands.java
  3. +69 −0 opentripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/StopsAlerts.java
  4. +23 −0 ...nner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/AbstractStopTester.java
  5. +83 −0 ...pplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/CountRoadInDis.java
  6. +24 −0 ...tripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/IStopTester.java
  7. +28 −0 ...tripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/TransitType.java
  8. +40 −0 ...planner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/UnconnectedStop.java
  9. +8 −3 opentripplanner-graph-builder/src/main/resources/log4j.properties
  10. +1 −0  opentripplanner-routing/src/main/java/org/opentripplanner/common/StreetUtils.java
  11. +5 −0 opentripplanner-routing/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java
  12. +3 −0  opentripplanner-routing/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java
View
35 ...ipplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/LoggerAppenderProvider.java
@@ -0,0 +1,35 @@
+package org.opentripplanner.graph_builder.impl;
+
+import org.apache.log4j.FileAppender;
+import org.apache.log4j.Logger;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.RollingFileAppender;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: Ben
+ * Date: 02/01/13
+ * Time: 15:25
+ * To change this template use File | Settings | File Templates.
+ */
+public class LoggerAppenderProvider {
+
+ private static org.slf4j.Logger _log = LoggerFactory.getLogger(PruneFloatingIslands.class);
+
+ public static String createCsvFile4LoggerCat(String fileName, String categoryName) {
+ if (fileName.isEmpty()) return null;
+ try{
+ PatternLayout layout = new PatternLayout("%m\n");
+ FileAppender appender = new RollingFileAppender(layout, fileName, false);
+ Logger.getLogger(categoryName).addAppender(appender);
+ }catch (IOException ioe){
+ _log.warn("could not create file appender for " + fileName + " duo to: " + ioe.getMessage());
+ return null;
+ }
+ return categoryName;
+ }
+
+}
View
16 ...tripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/PruneFloatingIslands.java
@@ -57,7 +57,8 @@
@Override
public void buildGraph(Graph graph, HashMap<Class<?>, Object> extra) {
_log.warn("Pruning isolated islands ...");
- StreetUtils.pruneFloatingIslands(graph, maxIslandSize, islandWithStopMaxSize, createLogger());
+ StreetUtils.pruneFloatingIslands(graph, maxIslandSize, islandWithStopMaxSize,
+ LoggerAppenderProvider.createCsvFile4LoggerCat(islandLogFile, "islands"));
if(transitToStreetNetwork == null){
_log.warn("Could not reconnect stop, TransitToStreetNetworkGraphBuilder was not provided");
}else{
@@ -72,17 +73,4 @@ public void checkInputs() {
//no inputs
}
- private String createLogger() {
- if (islandLogFile.isEmpty()) return null;
- try{
- PatternLayout layout = new PatternLayout("%m\n");
- FileAppender appender = new RollingFileAppender(layout, islandLogFile, false);
- Logger.getLogger("islands").addAppender(appender);
- }catch (IOException ioe){
- _log.warn("could not create file appender for " + islandLogFile + " duo to: " + ioe.getMessage());
- return null;
- }
- return "islands";
- }
-
}
View
69 opentripplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/StopsAlerts.java
@@ -0,0 +1,69 @@
+/* This program is free software: you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public License
+ as published by the Free Software Foundation, either version 3 of
+ the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+package org.opentripplanner.graph_builder.impl;
+
+import lombok.Setter;
+import org.opentripplanner.common.IterableLibrary;
+import org.opentripplanner.graph_builder.impl.stopsAlerts.IStopTester;
+import org.opentripplanner.graph_builder.services.GraphBuilder;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.routing.services.StreetVertexIndexService;
+import org.opentripplanner.routing.vertextype.TransitStop;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.*;
+
+public class StopsAlerts implements GraphBuilder {
+
+ @Setter
+ List<IStopTester> stopTesters = new ArrayList<IStopTester>();
+
+ @Setter
+ String logFile = "";
+
+ @Override
+ public void buildGraph(Graph graph, HashMap<Class<?>, Object> extra) {
+ Logger stopsLog = LoggerFactory.getLogger(LoggerAppenderProvider.createCsvFile4LoggerCat(logFile,"stops"));
+ stopsLog.info(String.format("%s,%s,%s,%s","stopId","lon","lat","types"));
+ for (TransitStop ts : IterableLibrary.filter(graph.getVertices(), TransitStop.class)) {
+ StringBuilder types = new StringBuilder();
+ for(IStopTester stopTester:stopTesters){
+ if(stopTester.fulfillDemands(ts,graph)){
+ if(types.length() > 0) types.append(";");
+ types.append(stopTester.getType());
+ }
+ }
+ if(types.length() > 0) {
+ stopsLog.info(String.format("%s,%f,%f,%s",ts.getIndex() ,ts.getCoordinate().x,ts.getCoordinate().y,types.toString()));
+ }
+ }
+ }
+
+ @Override
+ public List<String> provides() {
+ return Collections.emptyList();
+ }
+
+ @Override
+ public List<String> getPrerequisites() {
+ return Arrays.asList("transit","streets");
+ }
+
+ @Override
+ public void checkInputs() {
+ if(logFile.isEmpty())
+ throw new RuntimeException("missing log file name");
+ }
+}
View
23 ...r-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/AbstractStopTester.java
@@ -0,0 +1,23 @@
+package org.opentripplanner.graph_builder.impl.stopsAlerts;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.routing.vertextype.TransitStop;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: Ben
+ * Date: 02/01/13
+ * Time: 17:22
+ * To change this template use File | Settings | File Templates.
+ */
+public abstract class AbstractStopTester implements IStopTester{
+
+ @Getter @Setter
+ String type;
+
+ @Override
+ abstract public boolean fulfillDemands(TransitStop ts, Graph graph);
+
+}
View
83 ...anner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/CountRoadInDis.java
@@ -0,0 +1,83 @@
+package org.opentripplanner.graph_builder.impl.stopsAlerts;
+
+import com.vividsolutions.jts.geom.Envelope;
+import com.vividsolutions.jts.geom.GeometryFactory;
+import com.vividsolutions.jts.geom.Point;
+import com.vividsolutions.jts.operation.distance.DistanceOp;
+import lombok.Setter;
+import org.opentripplanner.common.geometry.DistanceLibrary;
+import org.opentripplanner.common.geometry.SphericalDistanceLibrary;
+import org.opentripplanner.routing.core.TraverseMode;
+import org.opentripplanner.routing.edgetype.StreetEdge;
+import org.opentripplanner.routing.edgetype.StreetTraversalPermission;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.routing.impl.StreetVertexIndexServiceImpl;
+import org.opentripplanner.routing.services.StreetVertexIndexService;
+import org.opentripplanner.routing.vertextype.TransitStop;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collection;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: Ben
+ * Date: 06/01/13
+ * Time: 14:01
+ * To change this template use File | Settings | File Templates.
+ */
+public class CountRoadInDis extends AbstractStopTester {
+
+ Logger _log = LoggerFactory.getLogger(CountRoadInDis.class);
+
+ @Setter
+ double distance; //distance in meters
+
+ @Setter
+ int numberOfStreets;
+
+ @Setter
+ StreetTraversalPermission allowedPermission;
+
+ GeometryFactory geometryFactory = new GeometryFactory();
+
+ @Override
+ public boolean fulfillDemands(TransitStop ts, Graph graph) {
+ if(graph.streetIndex == null){
+ graph.streetIndex = new StreetVertexIndexServiceImpl(graph);
+ _log.debug("street index built.");
+ }
+ StreetVertexIndexService streetIndexService = graph.streetIndex;
+ DistanceLibrary distanceLibrary;
+ if(streetIndexService instanceof StreetVertexIndexServiceImpl) {
+ distanceLibrary = ((StreetVertexIndexServiceImpl)streetIndexService).getDistanceLibrary();
+ }else{
+ distanceLibrary = SphericalDistanceLibrary.getInstance();
+ }
+ Envelope env = new Envelope(ts.getCoordinate());
+ double rInMeters;
+ if(distanceLibrary instanceof SphericalDistanceLibrary){
+ rInMeters = ((SphericalDistanceLibrary)distanceLibrary).RADIUS_OF_EARTH_IN_M;
+ }else{
+ rInMeters = 6371.01 * 1000;
+ }
+ double degForOneMeter = (Math.PI/(180*rInMeters));
+ double disInDeg = degForOneMeter * distance;
+ env.expandBy(disInDeg);
+ Collection<StreetEdge> streetEdges = streetIndexService.getEdgesForEnvelope(env);
+
+ int counter = 0;
+ for(StreetEdge streetEdge: streetEdges){
+ //this is completely inaccurate!!! but may be good enough for now.
+ double dis = DistanceOp.distance(streetEdge.getGeometry(),geometryFactory.createPoint(ts.getCoordinate()));
+ double disMeters = (dis*Math.PI/180) * rInMeters;
+ if (disMeters <= distance && streetEdge.getPermission().allows(allowedPermission)){
+ counter++;
+ }
+ }
+
+ if(counter >= numberOfStreets)
+ return true;
+ return false;
+ }
+}
View
24 ...pplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/IStopTester.java
@@ -0,0 +1,24 @@
+/* This program is free software: you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public License
+ as published by the Free Software Foundation, either version 3 of
+ the License, or (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+package org.opentripplanner.graph_builder.impl.stopsAlerts;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.routing.vertextype.TransitStop;
+
+public interface IStopTester {
+ boolean fulfillDemands(TransitStop ts, Graph graph);
+ String getType();
+}
View
28 ...pplanner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/TransitType.java
@@ -0,0 +1,28 @@
+package org.opentripplanner.graph_builder.impl.stopsAlerts;
+
+import lombok.Setter;
+import org.opentripplanner.routing.core.TraverseMode;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.routing.vertextype.TransitStop;
+
+import java.util.Set;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: Ben
+ * Date: 06/01/13
+ * Time: 11:47
+ * To change this template use File | Settings | File Templates.
+ */
+public class TransitType extends AbstractStopTester {
+
+ @Setter
+ TraverseMode transitType;
+
+ @Override
+ public boolean fulfillDemands(TransitStop ts, Graph graph) {
+ if (ts.getModes().contains(transitType))
+ return true;
+ return false;
+ }
+}
View
40 ...nner-graph-builder/src/main/java/org/opentripplanner/graph_builder/impl/stopsAlerts/UnconnectedStop.java
@@ -0,0 +1,40 @@
+package org.opentripplanner.graph_builder.impl.stopsAlerts;
+
+import org.opentripplanner.routing.edgetype.StreetTransitLink;
+import org.opentripplanner.routing.graph.Edge;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.routing.vertextype.TransitStop;
+
+import java.util.List;
+
+/**
+ * Created with IntelliJ IDEA.
+ * User: Ben
+ * Date: 02/01/13
+ * Time: 17:32
+ * To change this template use File | Settings | File Templates.
+ */
+public class UnconnectedStop extends AbstractStopTester {
+
+ @Override
+ public boolean fulfillDemands(TransitStop ts, Graph graph) {
+ List<Edge> outgoingStreets = ts.getOutgoingStreetEdges();
+ boolean hasStreetLink = false;
+ for(Edge e:ts.getIncoming()){
+ if(e instanceof StreetTransitLink){
+ hasStreetLink = true;
+ break;
+ }
+ }
+ if(!hasStreetLink){
+ //TODO: see what if there is incoming and not outgoing
+ for(Edge e:ts.getOutgoing()){
+ if(e instanceof StreetTransitLink){
+ hasStreetLink = true;
+ break;
+ }
+ }
+ }
+ return !(hasStreetLink || (outgoingStreets.size() > 0));
+ }
+}
View
11 opentripplanner-graph-builder/src/main/resources/log4j.properties
@@ -24,11 +24,16 @@ log4j.appender.stdout.layout.ConversionPattern=%d{ISO8601} %-5p [%F:%L] : %m%n
log4j.category.org.opentripplanner.graph_builder=DEBUG
log4j.category.org.opentripplanner.routing=DEBUG
-#This category is use to save the island of the graph by using PruneFloatingIslands model
+#This category is used to save the island of the graph by using PruneFloatingIslands model
log4j.category.islands=DEBUG
log4j.additivity.islands=false
-log4j.category.pointsInIsland=DEBUG
-log4j.additivity.pointsInIsland=false
+#This category is used to save ambiguous stops, see Stops Alerts
+log4j.category.stops=DEBUG
+log4j.additivity.stops=false
+
+
+
+
View
1  opentripplanner-routing/src/main/java/org/opentripplanner/common/StreetUtils.java
@@ -165,6 +165,7 @@ private static void depedestrianizeOrRemove(Graph graph, Subgraph island) {
Collection<Edge> outgoing = new ArrayList<Edge>(v.getOutgoing());
for (Edge e : outgoing) {
if (e instanceof StreetTransitLink) {
+ e.detach();
}
}
}
View
5 opentripplanner-routing/src/main/java/org/opentripplanner/routing/impl/StreetVertexIndexServiceImpl.java
@@ -282,6 +282,11 @@ public Vertex getClosestVertex(final Coordinate coordinate, String name,
}
@Override
+ public Collection<StreetEdge> getEdgesForEnvelope(Envelope envelope){
+ return edgeTree.query(envelope);
+ }
+
+ @Override
@SuppressWarnings("unchecked")
public CandidateEdgeBundle getClosestEdges(LocationObservation location,
TraversalRequirements reqs, List<Edge> extraEdges, Collection<Edge> preferredEdges,
View
3  opentripplanner-routing/src/main/java/org/opentripplanner/routing/services/StreetVertexIndexService.java
@@ -20,6 +20,7 @@
import org.opentripplanner.routing.core.LocationObservation;
import org.opentripplanner.routing.core.RoutingRequest;
import org.opentripplanner.routing.core.TraversalRequirements;
+import org.opentripplanner.routing.edgetype.StreetEdge;
import org.opentripplanner.routing.graph.Edge;
import org.opentripplanner.routing.graph.Vertex;
import org.opentripplanner.routing.impl.CandidateEdgeBundle;
@@ -66,6 +67,8 @@ public Vertex getClosestVertex(final Coordinate location, String name,
*/
public Collection<Vertex> getVerticesForEnvelope(Envelope envelope);
+ public Collection<StreetEdge> getEdgesForEnvelope(Envelope envelope);
+
/**
* Get the closest edges to this location are traversable given these
* preferences.
Something went wrong with that request. Please try again.