Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve stop linking by taking OSM platform polygons into account #4116

Merged

Conversation

leonardehrenfried
Copy link
Member

@leonardehrenfried leonardehrenfried commented Apr 26, 2022

Summary

Built on the ideas in #4065, this PR implements linking stops to platform ways, more specifically, to a platform's centroid (center of gravity).

It also allows you to configure a list of tags that should be checked for references and matches them up to both stop_id and stop_code.

Result

Schwabstrasse in Stuttgart is a subway station that is often incorrectly linked.

Before

Screenshot from 2022-04-26 14-20-19

After

Screenshot from 2022-04-26 14-12-35

Refactoring

It also includes the following refactorings:

  • TransitStopStreetVertex is renamed to OsmBoardingLocationVertex
  • TransitTaggedStopsModule is equally renamed to OsmBoardingLocationsModule
  • Make some fields final in OpenStreetMapModule
  • Some tests migrated to Junit5

Issue

closes #4103

Unit tests

Unit tests added.

Code style

Autoformatted.

Documentation

added

cc @demory

@leonardehrenfried leonardehrenfried requested a review from a team as a code owner April 26, 2022 11:53
@leonardehrenfried leonardehrenfried force-pushed the improve-platform-linking branch 2 times, most recently from 8506548 to 0445bb6 Compare April 26, 2022 11:56
@leonardehrenfried leonardehrenfried force-pushed the improve-platform-linking branch 2 times, most recently from 95d8ebc to 383d6f1 Compare April 28, 2022 08:15
@leonardehrenfried leonardehrenfried changed the title Improve platform linking by taking OSM ways into account Improve stop linking by taking OSM platform polygons into account Apr 28, 2022
@t2gran t2gran added this to the 2.2 milestone Apr 28, 2022
@t2gran t2gran requested a review from hannesj April 28, 2022 14:10
@leonardehrenfried
Copy link
Member Author

In today's meeting we all agreed that we don't want to show a line from the center of the platform to the actual stop location to the user, however it will be necessary to show it in the debug client. The last part is not yet implemented but I will do so soon.

@hannesj
Copy link
Contributor

hannesj commented Apr 28, 2022

The last part is not yet implemented but I will do so soon.

It should be enough to have it be visible in the debug tiles.

@slvlirnoff
Copy link
Member

Another small idea I had afterward, what if you pick the closest point of the platform polygon to the stop instead of the center? Sometimes you have the same platform for two railway and potentially a few different stops (when the platform is split in several sectors). Picking the closest point in the platform geometry could have nice changes between trips on the same platform but on a different railway (for instance from one side to the other side) and would make the distance of that missing line smaller.

@slvlirnoff
Copy link
Member

Also, how does this interact with areaVisibility code?

@vesameskanen
Copy link
Contributor

In my opinion, the center is the best choice. We do not know in which car the passenger travels, so starting the walk from the middle gives the best average estimate.

Stop positions from public transit data are often unpredictable an do not necessarily indicate the best linking point.

@leonardehrenfried
Copy link
Member Author

I also think that the centroid is the best option for the reasons that Vesa outlined.

Also, how does this interact with areaVisibility code?

The linking runs after the area visibility has so you will link the waiting point to the closest visibility line. This might look not quite like a straight line. So maybe I should make the centroid also part of the platform polygon so that the area visibility calculator calculates a straight line from all the other nodes.

@leonardehrenfried
Copy link
Member Author

I've now re-added the link in the debug UI.

Screenshot from 2022-04-29 13-42-45

This is ready for review now.

@leonardehrenfried
Copy link
Member Author

BTW, the screenshot is without areaVisibility so you can actually see what is going on. When you turn it on it becomes hard to see individual lines.

@leonardehrenfried leonardehrenfried merged commit e08ae4c into opentripplanner:dev-2.x May 3, 2022
@leonardehrenfried leonardehrenfried deleted the improve-platform-linking branch May 3, 2022 08:45
t2gran pushed a commit that referenced this pull request May 3, 2022
@leonardehrenfried leonardehrenfried added IBI Developed by or important for IBI Group and removed IBI test labels Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IBI Developed by or important for IBI Group improvement New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve linking of stops to OSM platform ways
5 participants