Skip to content

2170 stops linker much slower #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pieterbuzing wants to merge 4 commits into
base: master
Choose a base branch
from
Open

2170 stops linker much slower #1

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
881e510
Give Simple Street linker an iterative search
pieterbuzing May 24, 2016
cd46228
Style and indentation fixes
pieterbuzing May 24, 2016
cf7de06
Improve efficiency of linker by storing lowercase
pieterbuzing May 24, 2016
91692a1
Revert "Improve efficiency of linker by storing lowercase"
pieterbuzing May 25, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 90 additions & 63 deletions src/main/java/org/opentripplanner/graph_builder/linking/SimpleStreetSplitter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,20 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;

/**
* This class links transit stops to streets by splitting the streets (unless the stop is extremely close to the street
* intersection).
*
* <p>
* It is intended to eventually completely replace the existing stop linking code, which had been through so many
* revisions and adaptations to different street and turn representations that it was very glitchy. This new code is
* also intended to be deterministic in linking to streets, independent of the order in which the JVM decides to
* iterate over Maps and even in the presence of points that are exactly halfway between multiple candidate linking
* points.
*
* <p>
* It would be wise to keep this new incarnation of the linking code relatively simple, considering what happened before.
*
* <p>
* See discussion in pull request #1922, follow up issue #1934, and the original issue calling for replacement of
* the stop linker, #1305.
*/
Expand All @@ -61,7 +62,9 @@ public class SimpleStreetSplitter {

public static final int MAX_SEARCH_RADIUS_METERS = 1000;

/** if there are two ways and the distances to them differ by less than this value, we link to both of them */
/**
* if there are two ways and the distances to them differ by less than this value, we link to both of them
*/
public static final double DUPLICATE_WAY_EPSILON_METERS = 0.001;

private Graph graph;
Expand All @@ -73,9 +76,10 @@ public class SimpleStreetSplitter {
/**
* Construct a new SimpleStreetSplitter. Be aware that only one SimpleStreetSplitter should be
* active on a graph at any given time.
*
* @param graph
*/
public SimpleStreetSplitter (Graph graph) {
public SimpleStreetSplitter(Graph graph) {
this.graph = graph;

// build a nice private spatial index, since we're adding and removing edges
Expand All @@ -86,56 +90,78 @@ public SimpleStreetSplitter (Graph graph) {
}
}

/** Link all relevant vertices to the street network */
public void link () {
/**
* Link all relevant vertices to the street network
*/
public void link() {
for (Vertex v : graph.getVertices()) {
if (v instanceof TransitStop || v instanceof BikeRentalStationVertex || v instanceof BikeParkVertex)
if (v instanceof TransitStop || v instanceof BikeRentalStationVertex || v instanceof BikeParkVertex) {
if (!link(v)) {
if (v instanceof TransitStop)
LOG.warn(graph.addBuilderAnnotation(new StopUnlinked((TransitStop) v)));
else if (v instanceof BikeRentalStationVertex)
LOG.warn(graph.addBuilderAnnotation(new BikeRentalStationUnlinked((BikeRentalStationVertex) v)));
else if (v instanceof BikeParkVertex)
LOG.warn(graph.addBuilderAnnotation(new BikeParkUnlinked((BikeParkVertex) v)));
};
}
}
}
}

/** Link this vertex into the graph */
public boolean link (Vertex vertex) {
/**
* Iteratively search the area around a location and find all the close by edges that can be traversed.
* This method tries to collect at least 10 candidates per iteration in order to prevent the loss of
* edges that are approximately at the same distance from the center. This number is arbitrary, though.
* <p>
* Note that the returned list is unsorted.
*
* @param vertex the center vertex
* @return a (possibly empty) list of edges close to the vertex
*/
private List<StreetEdge> getCandidateEdges(Vertex vertex) {
List<StreetEdge> edges;
double maxRadiusDeg = SphericalDistanceLibrary.metersToDegrees(MAX_SEARCH_RADIUS_METERS);
int iteration = 7; // We double the radius at most n times

final TraverseModeSet traverseModeSet = new TraverseModeSet(TraverseMode.WALK);
final Envelope env = new Envelope(vertex.getCoordinate());
double xscale = Math.cos(vertex.getLat() * Math.PI / 180);
double radius = maxRadiusDeg / (1 << (iteration - 1));
env.expandBy(radius / xscale, radius);

do {
//Do the envelope query and filter edges that can be traversed and that are still in the
//graph
edges = idx.query(env).stream().parallel().filter(edge ->
edge.canTraverse(traverseModeSet) &&
edge.getToVertex().getIncoming().contains(edge)
).collect(Collectors.toList());

iteration--;
radius = maxRadiusDeg / (1 << iteration);
env.expandBy(radius / xscale, radius);
} while (edges.size() < 10 && iteration > 0);

return edges;
}

/**
* Link this vertex into the graph
*/
public boolean link(Vertex vertex) {
// find nearby street edges
// TODO: we used to use an expanding-envelope search, which is more efficient in
// dense areas. but first let's see how inefficient this is. I suspect it's not too
// bad and the gains in simplicity are considerable.
final double radiusDeg = SphericalDistanceLibrary.metersToDegrees(MAX_SEARCH_RADIUS_METERS);

Envelope env = new Envelope(vertex.getCoordinate());

// local equirectangular projection
final double xscale = Math.cos(vertex.getLat() * Math.PI / 180);

env.expandBy(radiusDeg / xscale, radiusDeg);

double duplicateDeg = SphericalDistanceLibrary.metersToDegrees(DUPLICATE_WAY_EPSILON_METERS);

// We sort the list of candidate edges by distance to the stop
// This should remove any issues with things coming out of the spatial index in different orders
// Then we link to everything that is within DUPLICATE_WAY_EPSILON_METERS of of the best distance
// so that we capture back edges and duplicate ways.
// TODO all the code below looks like a good candidate for Java 8 streams and lambdas
List<StreetEdge> candidateEdges = new ArrayList<StreetEdge>(
Collections2.filter(idx.query(env), new Predicate<StreetEdge>() {

@Override
public boolean apply(StreetEdge edge) {
// note: not filtering by radius here as distance calculation is expensive
// we do that below.
return edge.canTraverse(new TraverseModeSet(TraverseMode.WALK)) &&
// only link to edges still in the graph.
edge.getToVertex().getIncoming().contains(edge);
}
})
);
List<StreetEdge> candidateEdges = getCandidateEdges(vertex);

// make a map of distances
final TIntDoubleMap distances = new TIntDoubleHashMap();
Expand All @@ -145,19 +171,8 @@ public boolean apply(StreetEdge edge) {
}

// sort the list
Collections.sort(candidateEdges, new Comparator<StreetEdge> () {

@Override
public int compare(StreetEdge o1, StreetEdge o2) {
double diff = distances.get(o1.getId()) - distances.get(o2.getId());
if (diff < 0)
return -1;
if (diff > 0)
return 1;
return 0;
}

});
Collections.sort(candidateEdges, (o1, o2) ->
Double.compare(distances.get(o1.getId()), distances.get(o2.getId())));

// find the closest candidate edges
if (candidateEdges.isEmpty() || distances.get(candidateEdges.get(0).getId()) > radiusDeg)
Expand All @@ -183,8 +198,10 @@ public int compare(StreetEdge o1, StreetEdge o2) {
return true;
}

/** split the edge and link in the transit stop */
private void link (Vertex tstop, StreetEdge edge, double xscale) {
/**
* split the edge and link in the transit stop
*/
private void link(Vertex tstop, StreetEdge edge, double xscale) {
// TODO: we've already built this line string, we should save it
LineString orig = edge.getGeometry();
LineString transformed = equirectangularProject(orig, xscale);
Expand All @@ -207,17 +224,17 @@ else if (ll.getSegmentIndex() == orig.getNumPoints() - 1) {
// nPoints - 2: -1 to correct for index vs count, -1 to account for fencepost problem
else if (ll.getSegmentIndex() == orig.getNumPoints() - 2 && ll.getSegmentFraction() > 1 - 1e-8) {
makeLinkEdges(tstop, (StreetVertex) edge.getToVertex());
}

else {
} else {
// split the edge, get the split vertex
SplitterVertex v0 = split(edge, ll);
makeLinkEdges(tstop, v0);
}
}

/** Split the street edge at the given fraction */
private SplitterVertex split (StreetEdge edge, LinearLocation ll) {
/**
* Split the street edge at the given fraction
*/
private SplitterVertex split(StreetEdge edge, LinearLocation ll) {
LineString geometry = edge.getGeometry();

// create the geometries
Expand All @@ -244,17 +261,21 @@ private SplitterVertex split (StreetEdge edge, LinearLocation ll) {
return v;
}

/** Make the appropriate type of link edges from a vertex */
private void makeLinkEdges (Vertex from, StreetVertex to) {
if (from instanceof TransitStop)
/**
* Make the appropriate type of link edges from a vertex
*/
private void makeLinkEdges(Vertex from, StreetVertex to) {
if (from instanceof TransitStop)
makeTransitLinkEdges((TransitStop) from, to);
else if (from instanceof BikeRentalStationVertex)
makeBikeRentalLinkEdges((BikeRentalStationVertex) from, to);
else if (from instanceof BikeParkVertex)
makeBikeParkEdges((BikeParkVertex) from, to);
}

/** Make bike park edges */
/**
* Make bike park edges
*/
private void makeBikeParkEdges(BikeParkVertex from, StreetVertex to) {
for (StreetBikeParkLink sbpl : Iterables.filter(from.getOutgoing(), StreetBikeParkLink.class)) {
if (sbpl.getToVertex() == to)
Expand All @@ -265,10 +286,10 @@ private void makeBikeParkEdges(BikeParkVertex from, StreetVertex to) {
new StreetBikeParkLink(to, from);
}

/**
/**
* Make street transit link edges, unless they already exist.
*/
private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v) {
private void makeTransitLinkEdges(TransitStop tstop, StreetVertex v) {
// ensure that the requisite edges do not already exist
// this can happen if we link to duplicate ways that have the same start/end vertices.
for (StreetTransitLink e : Iterables.filter(tstop.getOutgoing(), StreetTransitLink.class)) {
Expand All @@ -280,8 +301,10 @@ private void makeTransitLinkEdges (TransitStop tstop, StreetVertex v) {
new StreetTransitLink(v, tstop, tstop.hasWheelchairEntrance());
}

/** Make link edges for bike rental */
private void makeBikeRentalLinkEdges (BikeRentalStationVertex from, StreetVertex to) {
/**
* Make link edges for bike rental
*/
private void makeBikeRentalLinkEdges(BikeRentalStationVertex from, StreetVertex to) {
for (StreetBikeRentalLink sbrl : Iterables.filter(from.getOutgoing(), StreetBikeRentalLink.class)) {
if (sbrl.getToVertex() == to)
return;
Expand All @@ -291,14 +314,18 @@ private void makeBikeRentalLinkEdges (BikeRentalStationVertex from, StreetVertex
new StreetBikeRentalLink(to, from);
}

/** projected distance from stop to edge, in latitude degrees */
private static double distance (Vertex tstop, StreetEdge edge, double xscale) {
/**
* projected distance from stop to edge, in latitude degrees
*/
private static double distance(Vertex tstop, StreetEdge edge, double xscale) {
// use JTS internal tools wherever possible
LineString transformed = equirectangularProject(edge.getGeometry(), xscale);
return transformed.distance(geometryFactory.createPoint(new Coordinate(tstop.getLon() * xscale, tstop.getLat())));
}

/** project this linestring to an equirectangular projection */
/**
* project this linestring to an equirectangular projection
*/
private static LineString equirectangularProject(LineString geometry, double xscale) {
Coordinate[] coords = new Coordinate[geometry.getNumPoints()];

Expand Down