-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Process boarding location for OSM ways (linear platforms) #6247
Process boarding location for OSM ways (linear platforms) #6247
Conversation
We talked about this today and we came to the conclusion that we want to introduce a data structure that is available during graph build only and then discarded. Lets discuss this in a meeting. |
Does moving the |
Unfortuntately not. We want to design a new system for keeping information that is used during graph build only and then discarded. |
185a5b8
to
89b3ac7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6247 +/- ##
=============================================
+ Coverage 69.79% 69.84% +0.05%
- Complexity 17943 17976 +33
=============================================
Files 2046 2049 +3
Lines 76685 76756 +71
Branches 7829 7832 +3
=============================================
+ Hits 53520 53608 +88
+ Misses 20423 20412 -11
+ Partials 2742 2736 -6 ☔ View full report in Codecov by Sentry. |
@t2gran will come with some suggestions of how to solve this without adding anything to the serialized graph. |
There is one thing I do not understand here. Is it true that in the OSM model the boarding reference can be on both way and node, and that it is only on Vertex in OTP? If so, why are we not just coping the boarding references to the vertexes it is connected to? I think the correct behavior for the routing would be to "arrive at the stop" when you arrive at one of the vertexes included in the platform. |
I think your behaviour is correct but it isn't supported even for the area platforms yet. The centroid is currently calculated and used. Maybe I should try to connect the stop to every vertexes instead (and also do it for area platforms as well) |
I am almost done with storing the refs between builders, so I will publish that here when I am done. Lets, discuss this on Tuesday again. I would like some feedback from the "AStar experts" on how to NOT model a stop as a point? This is likely to be a bigger refactoring, but for some stops modeling it as a point is problematic. |
…modules The Repository and Service is not available during routing, only during the graph build.
Here is how you can excange data between builders - the data is persserved during serialization: |
30e6bd6
to
d40d0b3
Compare
0afdc47
to
eaafc68
Compare
application/src/main/java/org/opentripplanner/routing/linking/VertexLinker.java
Show resolved
Hide resolved
application/src/main/java/org/opentripplanner/service/osminfo/OsmInfoGraphBuildRepository.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Leonard Ehrenfried <[email protected]>
...ation/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java
Outdated
Show resolved
Hide resolved
...ation/src/main/java/org/opentripplanner/graph_builder/module/OsmBoardingLocationsModule.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me. I wonder if linking the platform centroid to all edges of a linear platform would be a more straightforward solution; now the code links the centroid with a subset of close enough edges. This not a problem, though, just a bit more complexity.
I agree on what Leonard stated about splitting the node-way-area linking to 3 separate functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite useful functionality! I will next make some fixes to area boarding location linking.
Summary
This adds the processing of boarding locations on linear platforms.
Fixes #6056
Unit tests
added
Documentation
None
Bumping the serialization version id
No longer needed