Optimize routing time vertex linking with areas #6495

vesameskanen · 2025-02-26T20:55:22Z

Summary

Several optimizations:

An expanded (more complex) version of the area polygon is no longer used
Vertex is not snapped to the expanded polygon edge, but original location is used
Too high visibility vertex count is reduced during the graph building by taking vertices with most connections. There is usually no need to apply skip logic while routing. N^2 type slowdown by area complexity is compensated by applying less visibility connections to closest vertices when complexity grows over a defined limit.
Some unnecessary intersection tests and geometry conversions removed from vertex linker
Linker checks if area linking is already done before the time consuming point in polygon test
Linker reuses the created street vertices inside areas, which reduces edge count a lot
Linker uses two strategies to connect a vertex to an area: the visibility edges and connection to closest edge pair
Very bad linking fallback by ignoring all boundaries and connecting to all visibility vertices removed. Only one connection to the closest vertex is used in the highly unprobable case where both strategies fail.
Hard coded visibility vertex limit is now defined in a single place, StreetConstants.java. It used both in graph building and routing time linking. Default visibility vertex count increased from 150 to 200.

The notorious areas of Lund, Sweden now route fast. Some detours along area edges may occur when a visibility connection is not available, but such paths are mostly reasonable.

Unit tests

Existing tests updated to reflect new linking. Three new tests added.

Documentation

Related javadoc and comments updated.

Vertexlinker should not create an expanded area polygon whenever itinerary search starts/ends in an area. Buffer operation for a complex geometry is obviously very slow. Precompute the expansion.

Keep the visibility points which have the best connectivity.

A test breaks if the limit is taken literally

This eliminates the need for several point inside polygon tests.

codecov · 2025-02-26T21:02:28Z

Codecov Report

Attention: Patch coverage is 83.33333% with 27 lines in your changes missing coverage. Please review.

Project coverage is 70.25%. Comparing base (532da22) to head (9ee7de6).
Report is 139 commits behind head on dev-2.x.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
.../opentripplanner/routing/linking/VertexLinker.java	81.88%	18 Missing and 7 partials ⚠️
...n/java/org/opentripplanner/standalone/OTPMain.java	0.00%	1 Missing ⚠️
...anner/street/model/StreetLimitationParameters.java	75.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6495      +/-   ##
=============================================
+ Coverage      70.18%   70.25%   +0.07%     
- Complexity     18320    18386      +66     
=============================================
  Files           2084     2087       +3     
  Lines          77278    77429     +151     
  Branches        7834     7843       +9     
=============================================
+ Hits           54235    54396     +161     
+ Misses         20272    20262      -10     
  Partials        2771     2771

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

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModuleBuilder.java

There is no apparent reason for using it. Expanded geomerty typically has 4 times more points which slows down computations a lot.

Very complex areas have many visibility points. Computation of visibility edge intersections with are edges therefore eventually becomes too slow. Now the linker creates less visibility edges when the area gets more complex. Closest visibility points are used for connections. Probablility that a random visibility points far away could be connected are low, so this is a fair heuristics to speed up linking.

VertexLinker used a hard coded value to limit routing time area linking. Configured maxAreaNodes expresses better the current hardware and service resource limitations.

…tLimitationParameters

New optimizations speed up area linking. Increase the default value accordingly.

…exing The value will be used in street routing.

Area linking connected vertex to all visibility vertices, ignoring area boundaries such as buildings, when all attempted connections were blocked. Undesired fallback behavior removed.

- Divide linking code into smaller reusable functions - Separate edge snapping logic from area linking - Optimize parameters and geometry conversions - Add more documentation, remove outdated comments

Visibility vertices are optimized for passing through areas, They do not always provide an optimal path from a random area point.

It is not wise to overload same name with different functionality

- Remember created area interior vertices and reuse them to simplify the graph - Snap added area interior vertices to all closest edge candidates - Fallback when both visibility edges and connections to closest edges fail

application/src/main/java/org/opentripplanner/street/model/StreetLimitationParameters.java

leonardehrenfried · 2025-03-12T15:10:55Z

application/src/test/java/org/opentripplanner/routing/linking/LinkStopToPlatformTest.java

+   * but visibility edges which would cross the area boundary are not added
+   */
+  @Test
+  void testLinkStopToConcaveArea() {


😍 Great!

…eetLimitationParameters.java Co-authored-by: Leonard Ehrenfried <[email protected]>

…Planner into optimize-area-linking

habrahamsson-skanetrafiken · 2025-03-13T12:06:50Z

application/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java

-      // If more than one area intersects, we pick one by random for the name & properties
+    // we already know that the edge is inside the areaGroup and hit != null, but test anyway
+    if (hit == null) {
+      LOG.warn("No intersecting area found. This indicates a bug.");


I'm seeing this log message when linking transit stops. I guess that means that something is wrong? If you want I can extract a dataset to reproduce the issue?

[...] 11:20:13.702 INFO [main] (Graph.java:288) Index street model complete. 11:20:21.391 INFO [main] (OsmBoardingLocationsModule.java:116) Found 0 OSM references which match a stop's id or code 11:20:21.392 INFO [main] (TimetableRepository.java:159) Index timetable repository... 11:20:21.392 INFO [main] (TimetableRepositoryIndex.java:63) Timetable repository index init... 11:20:22.150 INFO [main] (TimetableRepositoryIndex.java:117) Timetable repository index init complete. 11:20:22.150 INFO [main] (TimetableRepository.java:161) Index timetable repository complete. 11:20:22.150 INFO [main] (Graph.java:286) Index street model... 11:20:22.587 INFO [main] (StreetIndex.java:426) Index street vertex progress tracking started. 11:20:27.617 INFO [main] (StreetIndex.java:437) Index street vertex progress: 1,078,000 of 2,413,678 (44%) 11:20:32.620 INFO [main] (StreetIndex.java:437) Index street vertex progress: 2,270,000 of 2,413,678 (94%) 11:20:33.348 INFO [main] (StreetIndex.java:443) Index street vertex progress tracking complete. 2,413,678 done in 10s (224,278 per second). 11:20:33.349 INFO [main] (Graph.java:288) Index street model complete. 11:20:33.578 INFO [main] (StreetLinkerModule.java:95) Linking transit stops to graph progress tracking started. 11:20:34.326 WARN [main] (VertexLinker.java:729) No intersecting area found. This indicates a bug. 11:20:34.334 WARN [main] (VertexLinker.java:729) No intersecting area found. This indicates a bug. 11:20:35.558 WARN [main] (VertexLinker.java:729) No intersecting area found. This indicates a bug. 11:20:35.569 WARN [main] (VertexLinker.java:729) No intersecting area found. This indicates a bug. 11:20:35.753 WARN [main] (VertexLinker.java:729) No intersecting area found. This indicates a bug. 11:20:35.754 WARN [main] (VertexLinker.java:729) No intersecting area found. This indicates a bug. 11:20:35.795 WARN [main] (VertexLinker.java:729) No intersecting area found. This indicates a bug. [...]

It was such a good idea to add the warning. This is probably an accuracy issue which appears because I removed the polygon buffering step.

Thanks for a good catch, I will fix the error!

Define thresholds as degrees in the class data.

…rtex Instead, use the split vertex directly

Linker tried to connect two overlapping split vertices at the area boundary. This is not necessary.

Leaving e.g. a transit stop unlinked due to boundary accuracy issues is a bad bug.

vesameskanen · 2025-03-14T10:16:00Z

It turned out that linking failed when a stop was very close to an area boundary. Code created very short (or zero length) connecting edges, which could be seen in the graph build log.

Now the collapsed case it catched. This actually makes the graph simpler. Also the case when one link connection is forced for any unexpected accuracy reason is now handled cracefully instead of rejecting the linking. HSL data set never hits this fallback, though.

Latest changes also optimize some unnecessary conversions of threshold constanst from meters to degrees during routing.

One new unit tests added to ensure that collapsed edges are not created.

vesameskanen · 2025-03-14T11:59:28Z

Walk routing between two complex areas in lund (using default visibility node counts) was before over 600 ms, and is now about 100 ms. Speedup is more than 5 times.

leonardehrenfried · 2025-03-17T11:20:40Z

...va/org/opentripplanner/routing/algorithm/raptoradapter/router/street/DirectStreetRouter.java

  public static List<Itinerary> route(OtpServerRequestContext serverContext, RouteRequest request) {
    if (request.journey().direct().mode() == StreetMode.NOT_SET) {
      return Collections.emptyList();
    }
    OTPRequestTimeoutException.checkForTimeout();

+    long searchBeginTime = System.currentTimeMillis();


we actually already have logs for this: https://github.com/leonardehrenfried/otp2-setup/blob/main/logback.xml#L51

This reverts commit c403f67.

habrahamsson-skanetrafiken

Nice work. This works fine for all cases I've tested and the speedup is big.

vesameskanen added 7 commits February 26, 2025 12:09

Do not generate expanded area geometries during routing

7ed561f

Vertexlinker should not create an expanded area polygon whenever itinerary search starts/ends in an area. Buffer operation for a complex geometry is obviously very slow. Precompute the expansion.

Simplify code and update code comments

5d5c001

Remove unnecessary projection

5f98a5e

Reduce excessive visibility point count already in graph building time

8ec6fef

Keep the visibility points which have the best connectivity.

Merge remote-tracking branch 'otp/dev-2.x' into optimize-area-linking

7fcd0b1

Set non-zero default for maxAreaNodes

3b89afd

A test breaks if the limit is taken literally

Test if area is already linked earlier

5f6dea6

This eliminates the need for several point inside polygon tests.

mariokhoury4 reviewed Feb 27, 2025

View reviewed changes

application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModuleBuilder.java Outdated Show resolved Hide resolved

vesameskanen added 20 commits February 27, 2025 08:18

Remove use of expanded geometry

b7c4b04

There is no apparent reason for using it. Expanded geomerty typically has 4 times more points which slows down computations a lot.

Add debug logging

d8cd764

Pass configured maxAreaNodes to VertexLinker

d6f26ea

VertexLinker used a hard coded value to limit routing time area linking. Configured maxAreaNodes expresses better the current hardware and service resource limitations.

Do not change areaVisibility to addExtraEdgesToAreas in some modules

cfd2d61

Define default maxAreaNodes in StreetConstants and inject it to Stree…

d6c6a52

…tLimitationParameters

Use higher default for maxAreaNodes

7375ffe

New optimizations speed up area linking. Increase the default value accordingly.

Do not use StreetLimitationParameters for transfering maxAreaNodes

01ec7cd

Update maxAreaNodes default value in docs

664cfa0

Pass the applied maxAreaNodes to vertex linker after street graph ind…

eec374e

…exing The value will be used in street routing.

Remove bad fallback in area linking

cbd742b

Area linking connected vertex to all visibility vertices, ignoring area boundaries such as buildings, when all attempted connections were blocked. Undesired fallback behavior removed.

Refactor vertex linker

edbaad3

- Divide linking code into smaller reusable functions - Separate edge snapping logic from area linking - Optimize parameters and geometry conversions - Add more documentation, remove outdated comments

Skip one more usually unnecessary intersection test

77f9007

Link both to the nearest edge and to visibility vertices

a5fd183

Visibility vertices are optimized for passing through areas, They do not always provide an optimal path from a random area point.

Give unique name to the lowest level link method

ff7268f

It is not wise to overload same name with different functionality

More robust and optimal vertex linking with areas

d7a6f19

- Remember created area interior vertices and reuse them to simplify the graph - Snap added area interior vertices to all closest edge candidates - Fallback when both visibility edges and connections to closest edges fail

Update stop linking unit tests

9d5dd85

Remove debug logging

99a3859

Merge remote-tracking branch 'otp/dev-2.x' into optimize-area-linking

89c1093

Update docs

683f808

vesameskanen marked this pull request as ready for review March 6, 2025 12:15

vesameskanen requested a review from habrahamsson-skanetrafiken March 12, 2025 12:54

leonardehrenfried reviewed Mar 12, 2025

View reviewed changes

application/src/main/java/org/opentripplanner/street/model/StreetLimitationParameters.java Outdated Show resolved Hide resolved

leonardehrenfried reviewed Mar 12, 2025

View reviewed changes

vesameskanen and others added 3 commits March 12, 2025 18:41

Update application/src/main/java/org/opentripplanner/street/model/Str…

79eb137

…eetLimitationParameters.java Co-authored-by: Leonard Ehrenfried <[email protected]>

Use unboxed long comparator when sorting visibility nodes

0286871

Merge branch 'optimize-area-linking' of github.com:HSLdevcom/OpenTrip…

fea0705

…Planner into optimize-area-linking

vesameskanen requested a review from leonardehrenfried March 13, 2025 05:49

leonardehrenfried previously approved these changes Mar 13, 2025

View reviewed changes

habrahamsson-skanetrafiken reviewed Mar 13, 2025

View reviewed changes

vesameskanen added 5 commits March 14, 2025 11:41

Do not convert meters to degrees all the time

8df8ed6

Define thresholds as degrees in the class data.

Do not add very short edge from area interior vertex to edge split ve…

d98d423

…rtex Instead, use the split vertex directly

Do not create collapsed visibility edges

8798486

Linker tried to connect two overlapping split vertices at the area boundary. This is not necessary.

Use any found area when a vertex outside an area is force linked.

0332b1e

Leaving e.g. a transit stop unlinked due to boundary accuracy issues is a bad bug.

Add unit test for the case where stop is on the edge of an area

1436532

vesameskanen dismissed leonardehrenfried’s stale review via 1436532 March 14, 2025 10:05

vesameskanen requested review from habrahamsson-skanetrafiken and leonardehrenfried March 14, 2025 10:16

Debug routing speed logging in direct street router

c403f67

leonardehrenfried reviewed Mar 17, 2025

View reviewed changes

Revert "Debug routing speed logging in direct street router"

9ee7de6

This reverts commit c403f67.

habrahamsson-skanetrafiken approved these changes Mar 17, 2025

View reviewed changes

leonardehrenfried approved these changes Mar 17, 2025

View reviewed changes

leonardehrenfried added the bump serialization id Add this label if you want the serialization id automatically bumped after merging the PR label Mar 17, 2025

vesameskanen merged commit 34f4bf6 into opentripplanner:dev-2.x Mar 17, 2025
6 checks passed

vesameskanen deleted the optimize-area-linking branch March 17, 2025 15:06

t2gran pushed a commit that referenced this pull request Mar 17, 2025

Add changelog entry for #6495 [ci skip]

7098ff6

t2gran pushed a commit that referenced this pull request Mar 17, 2025

Bump serialization version id for #6495

8a50f3a

leonardehrenfried mentioned this pull request Apr 4, 2025

Exception during linking a boarding location #6596

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Optimize routing time vertex linking with areas #6495

Optimize routing time vertex linking with areas #6495

Uh oh!

vesameskanen commented Feb 26, 2025 •

edited

Loading

Uh oh!

codecov bot commented Feb 26, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

leonardehrenfried Mar 12, 2025

Uh oh!

habrahamsson-skanetrafiken Mar 13, 2025

Uh oh!

vesameskanen Mar 13, 2025

Uh oh!

vesameskanen commented Mar 14, 2025 •

edited

Loading

Uh oh!

vesameskanen commented Mar 14, 2025

Uh oh!

leonardehrenfried Mar 17, 2025

Uh oh!

habrahamsson-skanetrafiken left a comment

Uh oh!

Uh oh!

Uh oh!

Optimize routing time vertex linking with areas #6495

Optimize routing time vertex linking with areas #6495

Uh oh!

Conversation

vesameskanen commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Unit tests

Documentation

Uh oh!

codecov bot commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

leonardehrenfried Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

habrahamsson-skanetrafiken Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

vesameskanen Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

vesameskanen commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vesameskanen commented Mar 14, 2025

Uh oh!

leonardehrenfried Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

habrahamsson-skanetrafiken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vesameskanen commented Feb 26, 2025 •

edited

Loading

codecov bot commented Feb 26, 2025 •

edited

Loading

vesameskanen commented Mar 14, 2025 •

edited

Loading