Stop count limit for access/egress routing and new accessEgress configuration object

[vesameskanen]

Summary

Limiting the number of stops collected in access/egress routing seems to be a very fundamental need. There are already some heuristics implementations in the sandbox. They apply some non-trivial (and somewhat time consuming) logic to analyze and collect 'good stops'. Surprisingly, such logic seems to cause unexpected errors and arise a need to understand how the transit data is modeled by various data producers, thus making configuring the heuristics difficult. For example: what is a typical bike-compatible trip count of a train stop? it can be anything between 100 and 10000.

in this PR, a very simple and fast option to limit the stop count is implemented. One single configuration value applies to all street modes. As the suitable limit value is typically quite high, e.g. 300-500, the skipEdgeStrategy does not have to be very precise, and therefore fast counting of visited transit stops works fine.

The new strategy does not qualify stops against search parameters, but that seems to have only a little effect. For example, if 300 nearest stops provide no bike-compatible trips, then bike + transit itineraries are likely to be poor.

This PR also collect access/egress routing parameters to a new JSON object called accessEgress.

Documentation

Updated

[leonardehrenfried]

This looks like a solid effort and I believe with Thomas' recent PR (accessEgressPenalty) we can remove the VehicleToStopHeuristics (subject to a bit of experimentation).

Since we now have a handful of properties to configure for access/egress, I think we should have a new JSON object in the config:

{
  "accessEgress": {
    "maximumDuration": "2h",
    "maximumDurationPerMode" : {}
    "maximumStops": 1000
    "penalty": ...
  }
}

What do you think about that?

[vesameskanen]

I agree.

I noticed that flex routing uses separate values for access/egress walking. Would it also benefit from the new options and more control?

[leonardehrenfried]

I'm not sure really. I need to investigate some more.

[leonardehrenfried]

Is this ready for review or do you want to experiment with it some more?

[vesameskanen]

I am still experimenting with it. There is something unexpected going on: commit f600e26 changed itinerary search time from 2.7s to 8.9s in one of our services using complex GTFS data and long access/egress routing time (8h).

I am currently searching which configuration setting is related to the slowdown.

[codecov]

Codecov Report

Patch coverage: 94.87% and project coverage change: +0.03% 🎉

Comparison is base (1baa283) 66.08% compared to head (46fd26b) 66.11%.
Report is 1 commits behind head on dev-2.x.

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5214      +/-   ##
=============================================
+ Coverage      66.08%   66.11%   +0.03%     
- Complexity     14942    14966      +24     
=============================================
  Files           1780     1782       +2     
  Lines          68969    69039      +70     
  Branches        7287     7292       +5     
=============================================
+ Hits           45576    45644      +68     
- Misses         20905    20910       +5     
+ Partials        2488     2485       -3     

Files Changed	Coverage Δ
...tripplanner/ext/traveltime/TravelTimeResource.java	0.00% <0.00%> (ø)
.../graph_builder/module/DirectTransferGenerator.java	91.17% <ø> (ø)
...aptoradapter/router/street/AccessEgressRouter.java	100.00% <ø> (ø)
...radapter/router/street/FlexAccessEgressRouter.java	70.58% <50.00%> (ø)
...pi/request/preference/AccessEgressPreferences.java	91.83% <91.83%> (ø)
...i/mapping/preferences/StreetPreferencesMapper.java	88.88% <100.00%> (+1.38%)	⬆️
...lanner/ext/transmodelapi/model/plan/TripQuery.java	99.63% <100.00%> (ø)
...anner/astar/strategy/MaxCountSkipEdgeStrategy.java	100.00% <100.00%> (ø)
...planner/graph_builder/module/NearbyStopFinder.java	86.44% <100.00%> (+0.59%)	⬆️
.../algorithm/raptoradapter/router/TransitRouter.java	76.11% <100.00%> (+0.36%)	⬆️
... and 3 more

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leonardehrenfried]

I've taken the liberty to improve the generics and add a test here: leonardehrenfried@04f9616

If you are happy with this code, can you pull it into this branch.

[t2gran]

The new strategy does not qualify stops against search parameters, but that seems to have only a little effect. For example, if 300 nearest stops provide no bike-compatible trips, then bike + transit itineraries are likely to be poor.

I think this is a good thing - it make the system more deterministic and more easy to reason about. If changing a parameter yields an unexpected different result (because of access/egress pruning) understanding the system will be difficult. It is important to remember this is a performance tuning optimisation and should be kept "as simple as possible".

[leonardehrenfried]

You still have merge conflicts and have to merge the very latest version of dev-2.x.

[vesameskanen]

One can see stop count conveniently by creating a logback-include-extensions.xml file in otp root:

<included> <logger name="org.opentripplanner.routing.algorithm.raptoradapter.router.street.AccessEgressRouter" level="debug" /> </included>

[t2gran]

One can see stop count conveniently by creating a logback-include-extensions.xml file in otp root:

<included> <logger name="org.opentripplanner.routing.algorithm.raptoradapter.router.street.AccessEgressRouter" level="debug" /> </included>

You may also check this box when starting OTP with the interactive lancher: