Skip to content

Fix: Account for wait-time in no-wait Raptor strategy #3798

Merged
t2gran merged 7 commits into
opentripplanner:dev-2.xfrom
entur:otp2_fix_heuristics_access_boarding
Jan 7, 2022
Merged

Fix: Account for wait-time in no-wait Raptor strategy #3798
t2gran merged 7 commits into
opentripplanner:dev-2.xfrom
entur:otp2_fix_heuristics_access_boarding

Conversation

@t2gran

@t2gran t2gran commented Jan 3, 2022

Copy link
Copy Markdown
Member

Summary

The no-wait Raptor strategy do not only need to remove wait-time, but also consider the individual path departure times when boarding a trip. If more than one path can board a trip (at two different stops) the trip with the latest departure time should be chosen. This can be achieved by maximizing the trip-shift-time(wait time before boarding). The NoWaitTransitWorker is a single-criteria Raptor search using travel-duration, not arrival-time as its criteria.

This error have probably existed since we stated using Raptor in OTP with the heuristics optimization. This error causes the no-wait-time strategy used to find the shortest travel times to return none optimal paths. In some cases these are "bad" enough to make the following McRaptor search to fail. The heuristics is used to prune the search space, and when arriving at a stop we look if it is possible to reach the destination before any of the existing results. We uses the travel time calculated in the heuristics from the given arrival-stop to the destination to do this.

The most frequently reported error cases by this, are journeys with relative few options towards the end of the path and with multiple egress legs from the same trip to the destination. The wrong egress path is sometimes used to calculate the heuristics, witch again causes the mc-raptor to drop optimal solutions. This can also happen during a transfer between two trips, but because of more wait-time in the path it is not as frequent.

Issue

I could not find an issue for this.

Fixes #3799 (Cleanup Routing strategies naming)

Unit tests

Two new Raptor module tests are added for this. One for the case where this happens with access/egress and one test when this happens when transferring between two trips.

Unit tests

Review this commit by commit is recommended.

Code style

Documentation

✅ JavaDoc is updated, no other doc needs to be updated.

t2gran added 2 commits January 3, 2022 16:09
Also add a new method 'isAfter'. These two methods are used to compare
time, and the forward and reverse calculators should implement them. Unit
tests are added for the new method - it is not used, but we will use it in
the next commit.
The NoWait strategy do not only need to remove wait-time, but also consider
the individual path departure times when boarding a trip. If more tha one
path can board a trip (at two different stops) the trip with the latest
departure time should be chosen. This can be achieved by maximizing the
trip-shift-time(wait time before boarding).
@t2gran t2gran added !Bug Apply to issues describing a bug and PRs witch fixes it. Entur On Entur Roadmap Entur Test This is currently being tested at Entur labels Jan 3, 2022
@t2gran t2gran added this to the 2.1 milestone Jan 3, 2022
@t2gran
t2gran requested a review from hannesj January 3, 2022 17:32
@t2gran
t2gran requested a review from a team as a code owner January 3, 2022 17:32
hannesj
hannesj previously approved these changes Jan 4, 2022
@t2gran
t2gran requested a review from optionsome January 4, 2022 09:37

@optionsome optionsome left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looked good but found some problems in comments.

Comment thread src/test/java/org/opentripplanner/transit/raptor/moduletests/A04_NoWaitTest.java Outdated
Co-authored-by: Joel Lappalainen <lappalj8@gmail.com>
Comment thread src/test/java/org/opentripplanner/transit/raptor/moduletests/A04_NoWaitTest.java Outdated
t2gran added 2 commits January 5, 2022 10:42
This commit rename the Raptor routing strategies and update config and doc related to the renaming.
  - StdTransitWorker rename to ArrivalTimeRoutingStrategy
  - NoWaitTransitWorker rename to MinTravelDurationRoutingStrategy
  - McTransitWorker rename to MultiCriteriaRoutingStrategy
Comment thread src/main/java/org/opentripplanner/transit/raptor/package.md
* Same as {@link #STANDARD}, but skip extra wait time - only board slack.
* <p/>
* Computes best/min travel duration and number of transfers.
* Used by Raptor to find the shortest travel duration ignoring wait-time. It also finds

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should explain a bit more what ignoring wait-time means in practice. If I've not misunderstood, the search window is different compared to STANDARD.

@hannesj hannesj Jan 7, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The search window is the same. It will just record travel time, without taking waiting time into account.

@t2gran t2gran Jan 7, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is actually a check that you run the MinTravelDurationStrategy with one iteration, if set a searchWindow > 1m then it fails. See VerifyRequestIsValid

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The searchWindow is a request parameter and controls the number of RangeRaptor iterations depending on the iteration size (In practice 1 minute). All 3 strategies are plugged into the RangeRaptorWorker so in theory they all can be used with a searchWindow > 0m, but as I have now added doc for here and in the JavaDoc it does not make sense for the MinTravelDurationStrategy, hence there is a check on it witch will throw an exception. The Raptor API is a programming API and the check prevent any programmers from using it incorrect.

* The purpose of this class is to implement the "Standard" specific functionality of the worker
* with NO WAIT TIME between transfer and transit, except the boardSlack.
* <p/>
* The purpose of this class is to implement a routing strategy for finding the best minimum travel

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, maybe add something about the effect on the search window vs standard.

Comment thread src/main/java/org/opentripplanner/transit/raptor/api/request/RaptorProfile.java Outdated
Comment thread src/main/java/org/opentripplanner/transit/raptor/package.md
Co-authored-by: Joel Lappalainen <lappalj8@gmail.com>
hannesj
hannesj previously approved these changes Jan 7, 2022
@t2gran
t2gran merged commit 60b7df5 into opentripplanner:dev-2.x Jan 7, 2022
@t2gran
t2gran deleted the otp2_fix_heuristics_access_boarding branch January 7, 2022 14:42
t2gran pushed a commit that referenced this pull request Jan 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Bug Apply to issues describing a bug and PRs witch fixes it. Entur Test This is currently being tested at Entur Entur On Entur Roadmap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cleanup Raptor routing strategy naming

3 participants