fix the accessibility of transit connect edges #3948
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevinkreiser
commented
Feb 3, 2023
| const auto& tn = transit.nodes(i); | ||
| stops.insert(tn.onestop_id()); | ||
| if (tn.type() == static_cast<uint32_t>(NodeType::kTransitEgress)) { | ||
| EXPECT_NE(tn.traversability(), static_cast<uint32_t>(Traversability::kNone)); |
There was a problem hiding this comment.
pbf better have it set
kevinkreiser
commented
Feb 3, 2023
| @@ -405,6 +409,10 @@ TEST(GtfsExample, MakeTile) { | |||
| graph_tile_ptr tile = reader.GetGraphTile(graphid); | |||
| for (const auto& edge : tile->GetDirectedEdges()) { | |||
| uses[edge.use()]++; | |||
| if (edge.use() == Use::kTransitConnection) { | |||
| EXPECT_TRUE((edge.forwardaccess() & kPedestrianAccess) || | |||
There was a problem hiding this comment.
resulting connect edge better have access
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So the transit routing test was failing because the transit connect edges (the ones from the street network to the transit network) are marked with no pedestrian (or any other) access. This was because during the transit gtfs feed ingestion we forgot to set the in/egress node traversability (this can be either none (default), both, entrance or exit). at the moment the ingestion doesnt have the ability to differentiate between entrance and exit so the fix here is to simply set to both (the new default). transitland provided a "directionality" on in/egresses and it is unclear exactly where this comes from.
looking at the spec it might come from
pathways.txt::is_unidirectionalwhere the spec can tell you between two stops (in this case stop means any of the 4 types: platform, station, in/egress/generic or boarding area) what directions you can walk (the oneway-ness of edges within the station). from that we could determine that if from a an in/egress to a station node you can only go forward and not reverse then it must be ingress-only and not egress, similar story for from station to in/egress must be egress-only and not ingress. we can and should add a TODO to parse this information in the first phase of ingestion so that in the future we abide by it. for now it is enough to allow access in my opinion.anyway by allowing access at these nodes, we propogate (in transit builder) that access onto the transit connection edges we create. because these node traversibilities were not set, and their default is
kNone, when we made the transit connection edges they also got the same access, which is to say no access. costing then couldnt use them to find transit stations in the data and so the first check in the multimodal algorithm failed with: i dont see any freaking transit stations in your data wtf.with this fixed the algorithm now says it just cant find a path, which is likely to do with schedule information. there are 3 things to check there: