Skip to content
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

Jump to bottom

ReverseSearch field is now on StreetRouter #296

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

ReverseSearch field is now on StreetRouter #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 3 additions & 9 deletions src/main/java/com/conveyal/r5/api/ProfileResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,14 @@ public void addTransitPath(Map<LegMode, StreetRouter> accessRouter,
if (accessPathIndex < 0) {
//Here accessRouter needs to have this access mode since stopModeAccessMap is filled from accessRouter
StreetRouter streetRouter = accessRouter.get(accessMode);
//FIXME: Must we really update this on every streetrouter?
streetRouter.profileRequest.reverseSearch = false;
StreetRouter.State state = streetRouter.getStateAtVertex(startVertexStopIndex);
if (state != null) {
StreetPath streetPath;
if ((accessMode == LegMode.CAR_PARK || accessMode == LegMode.BICYCLE_RENT) && streetRouter.previousRouter != null) {
streetPath = new StreetPath(state, streetRouter, accessMode,
transportNetwork);
} else {
streetPath = new StreetPath(state, transportNetwork, false);
streetPath = new StreetPath(state, transportNetwork, streetRouter.reverseSearch); //reverse search is false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and on new L125 it's not clear to me why we're replacing hard-coded booleans with variables, accompanied with comments that state the values of those variables are constants.

}
StreetSegment streetSegment = new StreetSegment(streetPath, accessMode, transportNetwork.streetLayer);
profileOption.addAccess(streetSegment, accessMode, startVertexStopIndex);
Expand All @@ -122,11 +120,9 @@ public void addTransitPath(Map<LegMode, StreetRouter> accessRouter,
if (egressPathIndex < 0) {
//Here egressRouter needs to have this egress mode since stopModeEgressMap is filled from egressRouter
StreetRouter streetRouter = egressRouter.get(egressMode);
//FIXME: Must we really update this on every streetrouter?
streetRouter.profileRequest.reverseSearch = true;
StreetRouter.State state = streetRouter.getStateAtVertex(endVertexStopIndex);
if (state != null) {
StreetPath streetPath = new StreetPath(state, transportNetwork, true);
StreetPath streetPath = new StreetPath(state, transportNetwork, streetRouter.reverseSearch); // reverse search is true
StreetSegment streetSegment = new StreetSegment(streetPath, egressMode, transportNetwork.streetLayer);
profileOption.addEgress(streetSegment, egressMode, endVertexStopIndex);
//This should never happen since stopModeEgressMap is filled from reached stops in egressRouter
Expand Down Expand Up @@ -180,10 +176,9 @@ public void generateStreetTransfers(TransportNetwork transportNetwork, ProfileRe
Map<Integer, List<Transfer>> transfersWithSameStart = transferToOption.keySet().stream()
.collect(Collectors.groupingBy(Transfer::getAlightStop));
//LOG.info("Filling middle paths");
boolean prevReverseSearch = request.reverseSearch;
request.reverseSearch = false;
for (Map.Entry<Integer, List<Transfer>> entry: transfersWithSameStart.entrySet()) {
StreetRouter streetRouter = new StreetRouter(transportNetwork.streetLayer);
//reverse search is false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again here upon first reading I don't understand the comments asserting that certain values are true or false.

streetRouter.streetMode = StreetMode.WALK;
streetRouter.profileRequest = request;
//TODO: make configurable distanceLimitMeters in middle
Expand All @@ -207,7 +202,6 @@ public void generateStreetTransfers(TransportNetwork transportNetwork, ProfileRe
}
}
}
request.reverseSearch = prevReverseSearch;
}

/** Recompute stats for all options, should be done once all options have been added */
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,10 @@ private static void run(TransportNetwork transportNetwork) {
}

ProfileRequest profileRequest = new ProfileRequest();
Boolean reverseSearch = request.queryMap("reverse").booleanValue();
if (reverseSearch != null && reverseSearch) {
profileRequest.reverseSearch = true;
Boolean reverseSearchB = request.queryMap("reverse").booleanValue();
boolean reverseSearch = false;
if (reverseSearchB != null && reverseSearchB) {
reverseSearch = true;
}
profileRequest.zoneId = transportNetwork.getTimeZone();
profileRequest.fromLat = fromLat;
Expand Down Expand Up @@ -410,7 +411,7 @@ private static void run(TransportNetwork transportNetwork) {
featureCollection.put("type", "FeatureCollection");
List<GeoJsonFeature> features = new ArrayList<>();

fillFeature(transportNetwork, lastState, features, profileRequest.reverseSearch);
fillFeature(transportNetwork, lastState, features, reverseSearch);
featureCollection.put("features", features);
content.put("data", featureCollection);
} else {
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/com/conveyal/r5/point_to_point/builder/PointToPointQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,9 @@ public ProfileResponse getPlan(ProfileRequest request) {
private Map<LegMode, StreetRouter> findEgressPaths(ProfileRequest request) {
Map<LegMode, StreetRouter> egressRouter = new HashMap<>();
//For egress
//TODO: this must be reverse search
request.reverseSearch = true;
for(LegMode mode: request.egressModes) {
StreetRouter streetRouter = new StreetRouter(transportNetwork.streetLayer);
streetRouter.reverseSearch = true;
streetRouter.transitStopSearch = true;
streetRouter.quantityToMinimize = StreetRouter.State.RoutingVariable.DURATION_SECONDS;
if (egressUnsupportedModes.contains(mode)) {
Expand Down Expand Up @@ -232,10 +231,10 @@ private Map<LegMode, StreetRouter> findEgressPaths(ProfileRequest request) {
* @param option
*/
private void findDirectPaths(ProfileRequest request, ProfileOption option) {
request.reverseSearch = false;
//For direct modes
for(LegMode mode: request.directModes) {
StreetRouter streetRouter = new StreetRouter(transportNetwork.streetLayer);
//reverse search is false
StreetPath streetPath;
streetRouter.profileRequest = request;
if (mode == LegMode.BICYCLE_RENT) {
Expand Down Expand Up @@ -289,10 +288,10 @@ private void findDirectPaths(ProfileRequest request, ProfileOption option) {
* @param request
*/
private HashMap<LegMode, StreetRouter> findAccessPaths(ProfileRequest request) {
request.reverseSearch = false;
// Routes all access modes
HashMap<LegMode, StreetRouter> accessRouter = new HashMap<>();
for(LegMode mode: request.accessModes) {
//reverse search is false
StreetRouter streetRouter = new StreetRouter(transportNetwork.streetLayer);
streetRouter.profileRequest = request;
if (mode == LegMode.CAR_PARK) {
Expand Down
7 changes: 0 additions & 7 deletions src/main/java/com/conveyal/r5/profile/ProfileRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,6 @@ public class ProfileRequest implements Serializable, Cloneable {
/** Whether this is a depart-after or arrive-by search */
private SearchType searchType;

/**
* If true current search is reverse search AKA we are looking for a path from destination to origin in reverse
* It differs from searchType because it is used as egress search
*/
@JsonInclude(JsonInclude.Include.NON_DEFAULT)
public boolean reverseSearch = false;

/**
* Maximum fare for constraining monetary cost of paths during search in Analysis.
* If nonnegative, fares will be used in routing.
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/com/conveyal/r5/streets/EdgeStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,12 +552,12 @@ public float calculateSpeed(ProfileRequest options, StreetMode traverseStreetMod
return options.getSpeedForMode(traverseStreetMode);
}

public StreetRouter.State traverse (StreetRouter.State s0, StreetMode streetMode, ProfileRequest req,
TurnCostCalculator turnCostCalculator) {
public StreetRouter.State traverse(StreetRouter.State s0, StreetMode streetMode,
ProfileRequest req, TurnCostCalculator turnCostCalculator, boolean reverseSearch) {

// The vertex we'll be at after the traversal
int vertex;
if (req.reverseSearch) {
if (reverseSearch) {
vertex = getFromVertex();
} else {
vertex = getToVertex();
Expand All @@ -568,14 +568,14 @@ public StreetRouter.State traverse (StreetRouter.State s0, StreetMode streetMode
float time = (float) (getLengthM() / speedms);
float weight = 0;

if (!canTurnFrom(s0, s1, req.reverseSearch)) return null;
if (!canTurnFrom(s0, s1, reverseSearch)) return null;

// clear out turn restrictions if they're empty
if (s1.turnRestrictions != null && s1.turnRestrictions.isEmpty()) s1.turnRestrictions = null;

// figure out which turn res

startTurnRestriction(s0.streetMode, req.reverseSearch, s1);
startTurnRestriction(s0.streetMode, reverseSearch, s1);

//We allow two links in a row if this is a first state (negative back edge or no backState
//Since at least P+R stations are connected to graph with only LINK edges and otherwise search doesn't work
Expand Down Expand Up @@ -644,7 +644,7 @@ public StreetRouter.State traverse (StreetRouter.State s0, StreetMode streetMode
// Negative backEdge means this state is not the result of traversing an edge (it's the start of a search).
int turnCost = 0;
if (s0.backEdge >= 0) {
if (req.reverseSearch) {
if (reverseSearch) {
turnCost = turnCostCalculator.computeTurnCost(getEdgeIndex(), s0.backEdge, streetMode);
} else {
turnCost = turnCostCalculator.computeTurnCost(s0.backEdge, getEdgeIndex(), streetMode);
Expand Down
25 changes: 15 additions & 10 deletions src/main/java/com/conveyal/r5/streets/StreetRouter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public class StreetRouter {
*/
private TurnCostCalculator turnCostCalculator;

/**
* If true current search is reverse search AKA we are looking for a path from destination to origin in reverse
*/
public boolean reverseSearch = false;

// These are used for scaling coordinates in approximate distance calculations.
// The lon value must be properly scaled to underestimate distances in the region where we're routing.
private static final double MM_PER_UNIT_LAT_FIXED =
Expand Down Expand Up @@ -313,15 +318,15 @@ public boolean setOrigin (double lat, double lon) {
// FIXME we're setting weight but not time and distance on these states above!

// FIXME Below is reversing the vertices, but then aren't the weights, times, distances wrong? Why are we even doing this?
if (profileRequest.reverseSearch) {
if (reverseSearch) {
startState0.vertex = split.vertex1;
startState1.vertex = split.vertex0;
}

// If there is a turn restriction on this edge, we need to indicate that we are beginning the search in a
// turn restriction.
streetLayer.edgeStore.startTurnRestriction(streetMode, profileRequest.reverseSearch, startState0);
streetLayer.edgeStore.startTurnRestriction(streetMode, profileRequest.reverseSearch, startState1);
streetLayer.edgeStore.startTurnRestriction(streetMode, reverseSearch, startState0);
streetLayer.edgeStore.startTurnRestriction(streetMode, reverseSearch, startState1);

// These initial states are not recorded as bestStates, they will be added when they come out of the queue.
// FIXME but wait - we are putting them in the bestStates for some reason.
Expand Down Expand Up @@ -550,15 +555,15 @@ public void route () {
}

TIntList edgeList;
if (profileRequest.reverseSearch) {
if (reverseSearch) {
edgeList = streetLayer.incomingEdges.get(s0.vertex);
} else {
edgeList = streetLayer.outgoingEdges.get(s0.vertex);
}
// explore edges leaving this vertex
edgeList.forEach(eidx -> {
edge.seek(eidx);
State s1 = edge.traverse(s0, streetMode, profileRequest, turnCostCalculator);
State s1 = edge.traverse(s0, streetMode, profileRequest, turnCostCalculator, reverseSearch);
if (s1 != null && s1.distance <= distanceLimitMm && s1.getDurationSeconds() < tmpTimeLimitSeconds) {
if (!isDominated(s1)) {
// Calculate the heuristic (which involves a square root) only when the state is retained.
Expand Down Expand Up @@ -676,7 +681,7 @@ public State getStateAtVertex (int vertexIndex) {
State ret = null;

TIntList edgeList;
if (profileRequest.reverseSearch) {
if (reverseSearch) {
edgeList = streetLayer.outgoingEdges.get(vertexIndex);
} else {
edgeList = streetLayer.incomingEdges.get(vertexIndex);
Expand Down Expand Up @@ -718,7 +723,7 @@ public State getState(Split split) {
EdgeStore.Edge e = streetLayer.edgeStore.getCursor(split.edge);

TIntList edgeList;
if (profileRequest.reverseSearch) {
if (reverseSearch) {
edgeList = streetLayer.outgoingEdges.get(split.vertex1);
} else {
edgeList = streetLayer.incomingEdges.get(split.vertex0);
Expand All @@ -728,7 +733,7 @@ public State getState(Split split) {
Collection<State> states = bestStatesAtEdge.get(it.next());
// NB this needs a state to copy turn restrictions into. We then don't use that state, which is fine because
// we don't need the turn restrictions any more because we're at the end of the search
states.stream().filter(s -> e.canTurnFrom(s, new State(-1, split.edge, s), profileRequest.reverseSearch))
states.stream().filter(s -> e.canTurnFrom(s, new State(-1, split.edge, s), reverseSearch))
.map(s -> {
State ret = new State(-1, split.edge, s);
ret.streetMode = s.streetMode;
Expand All @@ -750,15 +755,15 @@ public State getState(Split split) {
// advance to back edge
e.advance();

if (profileRequest.reverseSearch) {
if (reverseSearch) {
edgeList = streetLayer.outgoingEdges.get(split.vertex0);
} else {
edgeList = streetLayer.incomingEdges.get(split.vertex1);
}

for (TIntIterator it = edgeList.iterator(); it.hasNext();) {
Collection<State> states = bestStatesAtEdge.get(it.next());
states.stream().filter(s -> e.canTurnFrom(s, new State(-1, split.edge + 1, s), profileRequest.reverseSearch))
states.stream().filter(s -> e.canTurnFrom(s, new State(-1, split.edge + 1, s), reverseSearch))
.map(s -> {
State ret = new State(-1, split.edge + 1, s);
ret.streetMode = s.streetMode;
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/com/conveyal/r5/streets/ReverseRoutingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void testReverseRouting() throws Exception {
ProfileRequest profileRequest = new ProfileRequest();
StreetRouter streetRouter = new StreetRouter(streetLayer);
streetRouter.profileRequest = profileRequest;
profileRequest.reverseSearch = true;
streetRouter.reverseSearch = true;
streetRouter.streetMode = StreetMode.BICYCLE;
VertexStore.Vertex AVertex = streetLayer.vertexStore.getCursor(A);
VertexStore.Vertex EVertex = streetLayer.vertexStore.getCursor(E);
Expand All @@ -108,7 +108,7 @@ public void testReverseRouting() throws Exception {

StreetRouter.State lastState = streetRouter.getStateAtVertex(A); //streetRouter.getDestinationSplit());
assertNotNull(lastState);
StreetPath streetPath = new StreetPath(lastState, transportNetwork, profileRequest.reverseSearch);
StreetPath streetPath = new StreetPath(lastState, transportNetwork, streetRouter.reverseSearch);

List<Integer> correctEdgeIdx = Arrays.asList( 0, 4, 6, 3 );
List<Integer> correctDuration = Arrays.asList(3,6,9,13);
Expand Down