Otp2 cost transfer by t2gran · Pull Request #3629 · opentripplanner/OpenTripPlanner

t2gran · 2021-09-30T09:20:16Z

Summary

This PR clean up the Optimize transfer, path building and add cost calculation for guaranteed and stay-seated transfers during the Raptor routing. It is the base for adding some of the reminding features for constrained transfer support.

A CONSTRAINED transfer is a transfer with extra constraints. These are imported from GTFS transfers.txt and NeTEx Interchanges. Example of constraints are stay-seated (blocks in GTFS), guaranteed, transfer priority, max-wait-time. This PR only changes the Raptor and post Raptor code, it does not do anything with the import.

This PR also changes OTP to use constrained transfers in the heuristics, previously it was only used in the forward multi-criteria search. To make it work several improvements was needed:

A new trip-time search is added. It is used in the No-Wait Raptor Worker and in logging. The existing trip-time search is still used in the path mapping for multi-criteria search. The reason we want to use both of these, are that the original is finding the correct boarding and alighting relative to a board- or alight- time, while the new one take any time as input and return the closest match. The NoWait Raptor worker is used in heuristics and time-shift the all stop-arrivals. To be able to debug and log these paths in a safe way we can use the new approximate trip-time search. It will always find a trip if it exist, but it is possible to create a case where it returns the wrong one. The previous trip-time search would fail. The original work when the arrival-time or the board-time is known, it is also the most efficient on; Hence used with the multi-criteria and standard Raptor state.
Raptor do not support multiple access/egress paths to the same stop, but the RaptorRoutingRequestTransitData do. The only reason this have not failed is that the Best-time Raptor state is used in the heuristics. The constrained transfers require the standard state to be used to look up previous transit arrivals (previous trip). This fails hard with an exception, saying that multiple-access paths are not supported. I have added support for this by filtering the access and egress paths if the Raptor profile is not multi-criteria. The multi-criteria is unchanged and work with this.

Issue

closes #3478

Unit tests

The test coverage is in general very good for this feature and most classes and modules changed have their own unit test. So fare there is one exception, the new PathBuilder, witch have good indirect coverage, but no proper test on the class. I intend to add unit test on the builder.

I have not done any performance testing on this, but with few constrained transfers the performance change should be minimal. Also, the OTPFeature#TransferConstraints can be turned off - in witch case the performance should be the same as before.

Documentation

All relevant documentation should be up to date, there is very little doc on this in the documentation.

Changelog

✅

- Support for reduced cost in Raptor for constrained transfers - New Path builder, used in Raptor mappers(FWD & RWS), Optimize transfers and in TestPathBuilder.

Unit tests are added on OptimizedPathTail and OptimizedPath.

Clean up how transfer constraint is added to path toStrings. Make utility methods for adding cost w/unit.

There is an error in the logic in OptimizedPathTail line 195 and the OptimizedPath line 90, when adding the priority cost. In both places a cost was added for path legs witch did not represent a transfer. Since the transfer constraint is on the transit-leg BEFORE the optional transfer, this logic is a bit complicated. This commit fixes this, and refactor the priority cost calculation, so the cost is always positive. Previously the transfer priority cost was calculated in the TransferPriority enum, but part of the information is in the TransferConstraint, so it make sense to move some of the logic up to he TransferConstraint.

This commit add the ability to log the stop id and name as part of the Raptor returned path. This is continent when logging and debugging in Raptor.

Co-authored-by: Hannes Junnila <hannes.junnila@gmail.com>

# Conflicts: # src/main/java/org/opentripplanner/routing/algorithm/RoutingWorker.java

We need to check if the previous stop-arrival is an access-stop arrival in StopArrivalsState#previousTransit(int boardStopIndex), not just return the previous arrival.

This parameter do not need to be a multi-criteria request.

Inject the search direction (FORWARD/REVERSE) into the debug logger using life-cycle events instead of relaying on looking at "increasing" arrival-times. There are cases where arrival times in two following arrivals are tha same, and with constrained transfers we might allow negative times.

The trip-times-search is used in many places to find boar- and alight-times for a given RaptorTripSchedule. There are several version supporting forward and reverse Raptor search results, but the current version handle the NO_WAIT_WORKER and the 0 minutes transfers badly. To improve this a new search witch find the colosest trip with respect to an approximate time is added, and the existing methods refactored. New unit tests are added. We will use the new version later.

The Raptor routing engine only support one access/egress leg for each stop. The McRaptor do support this, but not the Standard Raptor or BestTime Raptor. The BestTime will not fail, but pick an arbitrary access/egress while the Standard will fail. The reason way this has not failed in normal operation is that the BestTime is used in the Heuristics. But, for transfers with constraints we need to use the Standard Raptor state instead.

If transfer constraints are not used with heuristics we might not be able to find a path from some locations. This commit enables using constrained transfers with the Standard Raptor State and uses it for the heuristic search when constrained transfers is enabled.

flaktack

We've tested this with staySeated transfers and it worked as expected.

In NeTEx an interchange have an id, in GTFS a transfer do not. We include it here to enable debugging, logging and system integration. Note! OTP do not use the id, and it is just passed through OTP. There is no service in OTP to look up a transfer by the id.

Raptor allow facilitated transfers between two stops to happen even if there is no time to do the transfer walking. So, the optimized transfer service also need to respect this.

abyrd

I have done a high-level review, and this generally looks good. I have not tested on input data with guaranteed transfers (so it's good to have testing commentary from @flaktack). It's also reassuring to see thorough use of testing and assertions. I just made one suggestion on the changelog.

t2gran · 2021-10-21T14:33:18Z

I will force this trough, since we have two approvals (dismissed by the last merged.)

t2gran added 7 commits September 30, 2021 11:04

feat: Cost on constrained transfers

0747ded

- Support for reduced cost in Raptor for constrained transfers - New Path builder, used in Raptor mappers(FWD & RWS), Optimize transfers and in TestPathBuilder.

fix: Class cast exception in OptimizedPathTail

97b4a69

Unit tests are added on OptimizedPathTail and OptimizedPath.

refactor: Cleanup toStrings transfer constraints

dce7b17

Clean up how transfer constraint is added to path toStrings. Make utility methods for adding cost w/unit.

feat: Add stop id and name to Raptor logging

924b1f4

This commit add the ability to log the stop id and name as part of the Raptor returned path. This is continent when logging and debugging in Raptor.

Add Changelog entry

51893b0

regression: Add back Leg#interlinedWithPreviousLeg

727e2fe

t2gran added !New Feature A functional feature targeting the end user. Entur Test This is currently being tested at Entur +NeTEx This issue is related to the Netex model/import. labels Sep 30, 2021

t2gran added this to the 2.1 milestone Sep 30, 2021

t2gran requested a review from a team as a code owner September 30, 2021 09:20

hannesj reviewed Oct 6, 2021

View reviewed changes

Apply suggestions from code review

66e0b66

Co-authored-by: Hannes Junnila <hannes.junnila@gmail.com>

t2gran force-pushed the otp2_cost_transfer branch from f77e1c2 to 66e0b66 Compare October 7, 2021 14:43

t2gran added 2 commits October 7, 2021 16:44

Merge branch 'dev-2.x' into otp2_cost_transfer

4589756

Merge remote-tracking branch 'otp/dev-2.x' into otp2_cost_transfer

24131e2

# Conflicts: # src/main/java/org/opentripplanner/routing/algorithm/RoutingWorker.java

t2gran mentioned this pull request Oct 7, 2021

Use transfer constraints in raptor #3643

Closed

hannesj previously approved these changes Oct 8, 2021

View reviewed changes

t2gran requested a review from abyrd October 8, 2021 10:37

fix: NullPointerException in ConstrainedBoardingSearch

e92c427

We need to check if the previous stop-arrival is an access-stop arrival in StopArrivalsState#previousTransit(int boardStopIndex), not just return the previous arrival.

t2gran dismissed hannesj’s stale review via e92c427 October 8, 2021 12:06

t2gran added 8 commits October 8, 2021 16:34

refactor: Add TransferConstraint builder

7c054f0

refactor: Small clean-ups

05ad380

refactor: Rename mcRequest -> request

30cee34

This parameter do not need to be a multi-criteria request.

doc: Raptor doc update

7d6315d

feet: Use approximate trip search with no-wait worker

0dead6f

t2gran mentioned this pull request Oct 8, 2021

Missing trips with short searchWindow #3593

Closed

flaktack previously approved these changes Oct 12, 2021

View reviewed changes

hannesj previously approved these changes Oct 12, 2021

View reviewed changes

t2gran dismissed stale reviews from hannesj and flaktack via 1d41c2f October 12, 2021 16:25

t2gran added 2 commits October 13, 2021 13:52

fix: Allow 0 min transfers with walking in Tx Optimization

6aef0a9

Raptor allow facilitated transfers between two stops to happen even if there is no time to do the transfer walking. So, the optimized transfer service also need to respect this.

t2gran force-pushed the otp2_cost_transfer branch from 1d41c2f to 6aef0a9 Compare October 13, 2021 11:57

t2gran mentioned this pull request Oct 15, 2021

Add cost to maximize wait-time and avoid back-travel in optimize transfers #3654

Merged

t2gran added 2 commits October 16, 2021 01:49

Merge branch 'dev-2.x' into otp2_cost_transfer

ee30da0

Merge remote-tracking branch 'otp/dev-2.x' into otp2_cost_transfer

d8482e9

hannesj previously approved these changes Oct 20, 2021

View reviewed changes

abyrd previously approved these changes Oct 21, 2021

View reviewed changes

Comment thread docs/Changelog.md

Merge branch 'dev-2.x' into otp2_cost_transfer

0b2d6c9

t2gran dismissed stale reviews from abyrd and hannesj via 0b2d6c9 October 21, 2021 13:06

Merge branch 'dev-2.x' into otp2_cost_transfer

ad87dfc

t2gran merged commit 6279075 into opentripplanner:dev-2.x Oct 21, 2021

t2gran deleted the otp2_cost_transfer branch October 21, 2021 14:33

Navigation Menu

Uh oh!

Otp2 cost transfer - #3629

Otp2 cost transfer#3629
t2gran merged 26 commits into
opentripplanner:dev-2.xfrom
entur:otp2_cost_transfer

t2gran commented Sep 30, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flaktack left a comment

Uh oh!

abyrd left a comment

Uh oh!

Uh oh!

t2gran commented Oct 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

t2gran commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issue

Unit tests

Documentation

Changelog

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flaktack left a comment

Choose a reason for hiding this comment

Uh oh!

abyrd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

t2gran commented Oct 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

t2gran commented Sep 30, 2021 •

edited

Loading