Improve performance of Park+Ride, Bike+Ride and Bike+Transit by leonardehrenfried · Pull Request #3906 · opentripplanner/OpenTripPlanner

leonardehrenfried · 2022-02-15T11:46:11Z

Summary

This improves the performance of P+R, B+R and B+T trip searches.

The main reason why these searches are slow is that a large number of accesses are computed and then passed to raptor. Both the access as well as the transit searches are slow as the default duration of 45 minutes leads to many stops. For example, if you do a P+R search in a city, you will most likely find all stops in the entire city. However, limiting the access search time will lead to bad results in rural regions.

There is some prior art that limits the nearby stops to the closest 5 stations: #3556

This suffers from the problem that in GTFS stations are not mandatory. It also doesn't terminate the access search which is bad for performance.

That's why I'm proposing the following.

Heuristic

We want to scale the search radius with the density of "important" stops. So in a city the access search should should be aborted relatively early as you will most likely have a dense network of useful stops.

In a rural region you want to search a bit longer.

So to compute the density of stops I'm using the the number of routes at a stop as a termination criteria. If we have found many routes of type RAIL, SUBWAY or FERRY we terminate the access search and pass the stops to raptor. If we are in a rural region where we only have bus stops, we continue searching.

Results

Using this approach leads to faster search times as well as results with less driving/cycling, which are both desirable outcomes.

Before

13:26:14.885 DEBUG (DebugTimingAggregator.java:232) ┌  Routing initialization           :     0 ms
13:26:14.885 DEBUG (DebugTimingAggregator.java:232) ├  Direct street routing            :  1071 ms
13:26:14.885 DEBUG (DebugTimingAggregator.java:232) ├  Direct flex routing              :     0 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) │┌ Creating raptor data model       :   187 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) │├ Access routing (775 accesses)    :   325 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) │├ Egress routing (341 egresses)    :    49 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) ││ Access/Egress routing            :   325 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) │├ Main routing                     :   521 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) │├ Creating itineraries             :    26 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) ├┴ Transit routing total            :  1060 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) │  Routing total:                   :  1071 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) ├  Filtering itineraries            :     1 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) ├  Converting model objects         :     2 ms
13:26:14.886 DEBUG (DebugTimingAggregator.java:232) ┴  Request total                    :  1076 ms

After

13:13:48.325 DEBUG (DebugTimingAggregator.java:232) ┌  Routing initialization           :     0 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) ├  Direct street routing            :     3 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) ├  Direct flex routing              :     0 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) │┌ Creating raptor data model       :   110 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) │├ Access routing (48 accesses)     :    28 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) │├ Egress routing (341 egresses)    :    41 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) ││ Access/Egress routing            :    41 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) │├ Main routing                     :   286 ms
13:13:48.325 DEBUG (DebugTimingAggregator.java:232) │├ Creating itineraries             :     8 ms
13:13:48.326 DEBUG (DebugTimingAggregator.java:232) ├┴ Transit routing total            :   446 ms
13:13:48.326 DEBUG (DebugTimingAggregator.java:232) │  Routing total:                   :   447 ms
13:13:48.326 DEBUG (DebugTimingAggregator.java:232) ├  Filtering itineraries            :     1 ms
13:13:48.326 DEBUG (DebugTimingAggregator.java:232) ├  Converting model objects         :     2 ms
13:13:48.326 DEBUG (DebugTimingAggregator.java:232) ┴  Request total                    :   450 ms

Issue

Closes #3555

Unit tests

I would love to have some advice on how to write one.

Code style

Yes.

Documentation

Javadoc added.

miles-grant-ibigroup

Have played with this and it works really well! Thank you very much

leonardehrenfried · 2022-02-17T15:12:45Z

@hannesj has asked why I don't use the TerminationStrategy. I asked this question to Gard a while ago and he added this helpful comment in the JavaDoc: https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/src/main/java/org/opentripplanner/routing/algorithm/astar/strategies/SearchTerminationStrategy.java#L10-L15

So basically a TerminationStrategy only terminates a single path, not the search. I also find these names quite confusing and would be in favour of renaming.

optionsome

This seems to be an improvement at least from our point of view in a sense that it leads to less driving and more transit. However, I find there are also some downsides to this pr. If I look for an itinerary to a destination which is next to a car park, before it just suggested driving to the park and walking from there to the destination. Now it just suggests to park next to the destination, take a bus that takes you away from the destination by one stop and then walking from there to the destination. I don't remember anymore if there was some configuration option somewhere to get rid of these types of suggestions.

I think these car park + walk "direct" suggestions are not supported at all anymore with this implementation. It's not something we necessarily need in our project but I think it is still something we should support.

optionsome · 2022-02-22T10:17:14Z

+    public final static Set<StreetMode> vehicleToStopModes =
+            Set.of(
+                    StreetMode.BIKE,
+                    StreetMode.BIKE_TO_PARK,
+                    StreetMode.BIKE_RENTAL,
+                    StreetMode.CAR_TO_PARK,
+                    StreetMode.CAR_PICKUP
+            );


Shouldn't this include SCOOTER_RENTAL?

Yes it should. And CAR_RENTAL!

optionsome · 2022-02-22T10:18:13Z

                        transferMode = StreetMode.WALK;
                        egressMode = StreetMode.WALK;
-                        directMode = StreetMode.CAR_TO_PARK;
+                        directMode = null;


This makes it so that it's not possible to ever look for car park itineraries with no transit?

This disables it if you use the Legacy GraphQL and REST APIs and set a mode of CAR_PARK. You can use the Transmodel API to set any direct_mode you want.

I don't at least mind that it would no longer by possible with the legacy GraphQL API. Not sure if that should still remain a possibility for the REST API. Could we make it so that if no transit modes are specified in the query, it would enable the directMode search?

I would prefer the existing functionality being preserved, with the directMode being set. Currently mode=BICYCLE_RENTAL returns a bicycle rental only trip. I would expect mode=CAR_PARK to behave similarly.

I get where you're coming from but in my tests computing the direct mode CAR_PARK was often the slowest part of the entire query. It appears to try to leave the car on all stations along the way and then calculate a walking leg from there.

I would prefer always keeping the direct mode -- regardless of transit modes being set. For now it preserves the existing functionality, is in line with how the other modes work in the rest api, and is easier to understand/document.

This currently only affects the rest api along with the legacy graphql api: an alternative option would be to instead extend either of these apis with an option to included/exclude the direct mode when planning a trip? That would both be backward-compatible, easy to explain/document and can be set as-needed.

Presumably in the new API this wouldn't be needed anymore, since the values would be explicitly set instead of using heuristics.

Even if it's terrible for performance? In some of my tests it takes 3 times as long.

@flaktack Hannes has managed to give CAR_PARK a huge speed boost by modifying the A* heuristic: ee08ebe

Seemingly it also affects vehicle rental but doesn't affect the search when arriveBy=true?

It also improves the rental performance but the arriveBy is correct, because when searching in reverse you actually want to go to the closest rental bike from the destination not the start. Am I describing this correctly, @hannesj ?

optionsome · 2022-02-22T10:23:10Z

+            case RAIL:
+            case FERRY:
+            case SUBWAY:
+                return 20;
+            case BUS:
+                return 1;
+            default:
+                return 2;


This scoring system is probably slightly opinionated although it does seem reasonable. Not sure if this should be made somehow configurable or not.

What has stopped me so far from making it configurable is trying to explain it in documentation and giving it a name. If you have a suggestion, I'm very happy to hear it.

Yeah I'm not sure. If we make it configurable, should the configuration only affect this or could/should it also be used for preference in some other context? My intuition is that the configuration should probably be limited to only affect this scope.

Yes, I agree.

leonardehrenfried · 2022-02-22T14:20:08Z

I think these car park + walk "direct" suggestions are not supported at all anymore with this implementation. It's not something we necessarily need in our project but I think it is still something we should support.

direct mode = null means gives you walking so you should get a walk route. You can then use the itinerary filtering to remove the ridiculous P+R route. If you tell me where you see the route I can give the a config snippet.

But you're right, if you want routes that drive to the destination and park the car, this is not possible in the current Digitransit UI. You'd need to use an API where you can set the direct mode.

optionsome · 2022-02-23T09:34:23Z

direct mode = null means gives you walking so you should get a walk route. You can then use the itinerary filtering to remove the ridiculous P+R route. If you tell me where you see the route I can give the a config snippet.

I think these ridiculous itineraries happen everytime I search for a destination that is next to a car park that is relatively close to the origin. I think because we no longer do the direct search, these itineraries don't get filtered out because they are not being compared against direct result. Without the direct search, in some cases we would not get any itineraries if these ridiculous itineraries are filtered out but I think that is probably a good thing. If you have some configuration snippet for filtering these itineraries out, please let know.

leonardehrenfried · 2022-02-23T10:03:38Z

We do the a direct search but it's a walking one.

I tried playing around in Helsinki for a bit but it wasn't easy to get one of these ridiculous routes. In fact I could not find one.

Starting close to a parking lot

Short trip starting close to a parking lot

Here bad results are filtered out because walking is faster.

leonardehrenfried · 2022-02-23T10:06:08Z

Ah you're probably talking about something like this:

However, if you increase the car reluctance significantly, then you get something better:

optionsome · 2022-02-23T10:38:52Z

I mean cases like this. It seems like I get no results if I increase the carReluctance to 20. I'll have to test some more with higher carReluctance values to see what is the overall effect.

leonardehrenfried · 2022-02-23T10:41:47Z

And what would you want to see in such a case?

optionsome · 2022-02-25T09:59:04Z

And what would you want to see in such a case?

I discussed this topic internally at out our project and I think we are not at least completely opposed to including itineraries that include just car park + walk because some people only search for a part of their journey. However, it's not really necessary that it's possible with legacy GraphQL API as soon we will move to use the new API once it's done anyway before we fully start using OTP2 in production.

optionsome · 2022-02-25T10:00:39Z

I forgot to mention in the last post that we will also like to use a high car reluctance value so transit is preferred over car travel.

leonardehrenfried · 2022-02-28T14:29:34Z

@optionsome I did some improvements today with @hannesj and I think we came to a good compromise solution: we calculate CAR as the direct mode (fast) and increase the car reluctance which means that you still get results even though driving all the way is almost always the fastest.

Here is your example in Järvenperä:

And from Järvenperä to central Helsinki you still get useful results.

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

optionsome

While testing this, I spotted #3958 but it doesn't originate from this pr so I think it can be fixed in a separate pr.

leonardehrenfried requested a review from a team as a code owner February 15, 2022 11:46

leonardehrenfried marked this pull request as draft February 15, 2022 11:46

leonardehrenfried changed the title ~~Improve performance of Park+Ride, Bike+Ride and Use heuristic to limit park+ride accesses~~ Improve performance of Park+Ride, Bike+Ride and Bike+Transit Feb 15, 2022

leonardehrenfried marked this pull request as ready for review February 15, 2022 12:17

leonardehrenfried added the IBI test label Feb 15, 2022

miles-grant-ibigroup previously approved these changes Feb 16, 2022

View reviewed changes

t2gran requested review from hannesj and optionsome February 17, 2022 14:50

t2gran added this to the 2.1 milestone Feb 17, 2022

t2gran added the !Optimization The feature is to improve performance. label Feb 17, 2022

leonardehrenfried dismissed miles-grant-ibigroup’s stale review via 75ed569 February 17, 2022 15:05

leonardehrenfried force-pushed the park-and-ride-heuristics branch from b31f2af to 75ed569 Compare February 17, 2022 15:05

optionsome reviewed Feb 22, 2022

View reviewed changes

hannesj reviewed Feb 24, 2022

View reviewed changes

Comment thread ...va/org/opentripplanner/routing/algorithm/astar/strategies/VehicleToStopSkipEdgeStrategy.java Outdated

t2gran modified the milestones: 2.1, 2.2 Feb 25, 2022

leonardehrenfried force-pushed the park-and-ride-heuristics branch 4 times, most recently from 6e353a2 to 24b36b5 Compare February 28, 2022 13:53

leonardehrenfried and others added 19 commits March 3, 2022 13:45

Finetune maxScore

9dd20b9

Add Javadoc

cdca391

Extend the included modes

8819ac9

Increase the value for railway stops

ea5fcfb

Use correct comment

e238a12

Improve strategies for P+R, B+R

8f4685f

Satisfy compiler

0028bd5

Resolve compilation error by replacing stream

d52a6ec

Help compiler

ca052b8

Fix compilation errors

84f2a4d

Improve performance of direct CAR_PARK

7205d5b

Don't check for reverseDirection for bike + transit

26ab623

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

Use array instead of List

ad6a6a7

Add Javadoc

45a0d00

Move feature behind a feature flag

f2acc95

Add sandbox documentation

8c37ffe

Add SCOOTER_RENTAL to list of modes

4515100

Use correct behaviour for arriveBy searches

364df99

Move strategies into a sandbox folder

c589473

leonardehrenfried force-pushed the park-and-ride-heuristics branch from 135ed2c to c589473 Compare March 3, 2022 12:45

hannesj requested review from hannesj and optionsome March 3, 2022 14:27

optionsome approved these changes Mar 4, 2022

View reviewed changes

hannesj approved these changes Mar 4, 2022

View reviewed changes

leonardehrenfried merged commit 2f0af92 into opentripplanner:dev-2.x Mar 8, 2022

leonardehrenfried deleted the park-and-ride-heuristics branch March 8, 2022 10:49

t2gran pushed a commit that referenced this pull request Mar 8, 2022

Add changelog entry for #3906 [ci skip]

dc19959

leonardehrenfried mentioned this pull request Mar 29, 2022

Filter access/egress stops for modes that involve driving #3556

Closed

leonardehrenfried added IBI Developed by or important for IBI Group and removed IBI test labels Jun 21, 2022

Uh oh!

Conversation

leonardehrenfried commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Heuristic

Results

Before

After

Issue

Unit tests

Code style

Documentation

Uh oh!

miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried commented Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

optionsome left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried commented Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

optionsome commented Feb 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leonardehrenfried commented Feb 23, 2022

Starting close to a parking lot

Short trip starting close to a parking lot

Uh oh!

leonardehrenfried commented Feb 23, 2022

Uh oh!

optionsome commented Feb 23, 2022

Uh oh!

leonardehrenfried commented Feb 23, 2022

Uh oh!

Uh oh!

optionsome commented Feb 25, 2022

Uh oh!

optionsome commented Feb 25, 2022

Uh oh!

leonardehrenfried commented Feb 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

optionsome left a comment

leonardehrenfried commented Feb 15, 2022 •

edited

Loading

leonardehrenfried commented Feb 17, 2022 •

edited

Loading

leonardehrenfried commented Feb 22, 2022 •

edited

Loading

optionsome commented Feb 23, 2022 •

edited

Loading

leonardehrenfried commented Feb 28, 2022 •

edited

Loading