Skip to content

Fix point-origin costs in reverse street searches and keep the origin cost in State.reverse() - #1027

Open
rafapereirabr wants to merge 1 commit into
conveyal:devfrom
rafapereirabr:fix/reverse-point-origin-costs
Open

rafapereirabr wants to merge 1 commit into
conveyal:devfrom
rafapereirabr:fix/reverse-point-origin-costs

Conversation

@rafapereirabr

Copy link
Copy Markdown

Problem

Reverse street searches started from a coordinate (reverseSearch = true + setOrigin(lat, lon), used for egress in PointToPointQuery and ProfileResponse) get wrong costs near the origin point. The vertex closer to the point looks more expensive than the farther one. Paths rebuilt from the search with new StreetPath(state, network, true) report less time and distance than the search itself used, because they leave out the partial street segment and the off-street walk to the point. On real networks this shortens egress legs and can change which stops are chosen: on a Porto Alegre sample data used in r5r, egress walks in r5r ended up to 1.27 km short of the destination and itineraries were up to 25 min faster than the same trips in travel_time_matrix().

Cause

  1. In StreetRouter.setOrigin(lat, lon), the two start states are costed for a forward search: the state at vertex1 gets distance1 and the state at vertex0 gets distance0. When reverseSearch is true, the method swaps the vertices but not the costs (see the existing FIXME). The state at vertex0 therefore carries the length of the other part of the edge.
  2. State.reverse() starts from a zero-cost copy and adds only the differences between consecutive states. The cost already held by the first state (partial edge plus off-street walk) is never added.

Fix

  1. In setOrigin(lat, lon), each start state gets the distance of the part of its own back edge that the traveler actually covers: distance0 along the forward edge from vertex0, distance1 along the backward edge from vertex1, at that edge's speed. Forward searches, vertices, back edges and turn restrictions are unchanged. The FIXME is replaced by a comment.
  2. State.reverse() adds the first state's duration and distance to the reversed path. Nothing changes when that state has no cost, e.g. after setOrigin(vertex).

Only reverse searches are affected. Regional and single-point analyses do not use them, so their results do not change. The partial-edge speed model and StreetPath geometry are unchanged.

Tests

New ReversePointOriginTest: each vertex gets the cost of its own part of the split edge, and a reversed StreetPath keeps the duration and distance of its state. Both fail on dev without this change. All 35 tests in com.conveyal.r5.streets pass. A local Windows run of the full suite gave 8 failures, all from temporary files Windows could not delete in non-routing tests (FlexNetworkBuildTest, SceneBuilderTest, OSMTest).

Found while working on r5r's detailed itineraries (ipea/r5r#607, by @e-kotov), where both issues are currently worked around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant