Remove BusRouteStreetMatcher and associated classes.

[evansiroky]

Expected behavior

Unused code should be removed from project.

Observed behavior

It appears that the org.opentripplanner.graph_builder.module.map.BusRouteStreetMatcher graph building module (and other associated classes) no longer have any effect if the module is activated. In PR #2266, the class NetworkLinkerLibrary was deleted. The NetworkLinkerLibrary class is referenced as being the place where some of this data would be used. Since this builder is no longer does anything, it should probably be removed.

Version of OTP used (exact commit hash or JAR name)

5e1affe

Data sets in use (links to GTFS and OSM PBF files)

N/A

Command line used to start OTP

N/A

Router config and graph build config JSON

Set the matchBusRoutesToStreets to true in the build config.

Steps to reproduce the problem

N/A

[t2gran]

This is partially the same as issue #2657, but nice to know why. I will keep the issue open and hopefully we will remember to remove this code as part of the cleanup.

[mpaine-act]

Related to this topic, I am getting errors compiling bayarea gtfs files when matchBusRoutesToStreets is set to true. Doing so causes a null exception:

12:03:25.556 INFO (BusRouteStreetMatcher.java:59) Finding corresponding street edges for trip patterns... 12:03:35.013 ERROR (OTPMain.java:45) An uncaught error occurred inside OTP: null java.lang.NullPointerException: null at org.opentripplanner.model.TripPattern.getGeometry(TripPattern.java:184) ~[otp-2.0.0-SNAPSHOT-shaded.jar:1.1] at org.opentripplanner.graph_builder.module.map.BusRouteStreetMatcher.buildGraph(BusRouteStreetMatcher.java:69) ~[otp-2.0.0-SNAPSHOT-shaded.jar:1.1] at org.opentripplanner.graph_builder.GraphBuilder.run(GraphBuilder.java:79) ~[otp-2.0.0-SNAPSHOT-shaded.jar:1.1] at org.opentripplanner.standalone.OTPMain.startOTPServer(OTPMain.java:116) ~[otp-2.0.0-SNAPSHOT-shaded.jar:1.1] at org.opentripplanner.standalone.OTPMain.main(OTPMain.java:38) ~[otp-2.0.0-SNAPSHOT-shaded.jar:1.1]

[t2gran]

@mpaine-act do you use the BusRouteStreetMatcher or is it ok to delete it. The problem you see above can probably be fixed, but if no one is using it we should probably remove the code instead.

[mpaine-act]

I assumed it made the route lines more accurate (snapping waypoints to OSM vectors), but maybe it makes things worse (eg. ramps or wrong way streets).

I have not done an exhaustive review for this feature (how does one verify this? extract the geometry out of graph.obj and compare with route's waypoints?) I know we do corrections in our CADAVL that might not make it back GTFS shape, but we are doing regional OTP so I cannot attest to all agencies in area.

[vitorecdias]

I have the same problem of @mpaine-act . I need to trace of bus routes on the streets between the stops of bus. When i put "matchBusRoutesToStreets:true" on "build-config.json" this problem occur.

[hbruch]

If the GTFS feed does not contain shapes, there are tools like e.g. @patrickbr's https://github.com/ad-freiburg/pfaedle which do map matching to enhance GTFS files in a preprocessing step by shapes deduced from OSM.

We are using pfaedle and though it is not 100% right all the time, it does a fairly good job. E.g. for bus routing, it takes turn restrictions and bus tagging into account, it prefers ways contained in a matching OSM public_transport route relation and various features more.

[vitorecdias]

@hbruch, i will test this. thank you so much

[mpaine-act]

In my situation, GTFS shapes are created. The problem is transitioning between base maps (OSM is the base map for few agencies), so tracing map vectors with GTFS shapes should correct many errors (might cause a few too).

Are you suggesting pfaedle could preprocess the GTFS shapes.txt from all the area agencies before processing with OTP into graph.obj?

[patrickbr]

Are you suggesting pfaedle could preprocess the GTFS shapes.txt from all the area agencies before processing with OTP into graph.obj?

Author of pfaedle here. I understand you are suggesting that pfaedle map-matches an existing shapes.txt to OSM data. pfaedle could do that, but this feature is currently not (yet ☺) implemented.

[hbruch]

To add to @patrickbr comment: pfaedle has options to either overwrite existing shapes or only create missing ones. So if the shapes correctly follow the route, but are just off a bit due to different base maps, you have to weight this against matching exactly the OSM base map, but perhaps taking a wrong turn here or there.

[mpaine-act]

Our agency has to perform manual timepoint/shape edits when moving between systems which use different base maps. Overriding the given GTFS shape files is an option (for the 33 bay area agencies, who knows what base maps everyone is using). I could also point out if OSM is wrong, we can fix the maps too, but that is a big exercise which could be cause contention between OSM maintainers.

Interesting option, thanks everyone.

[hbruch]

From OSM maintainers perspective, they are usually happy seeing their work used by others (at least here in Germany). Just mind to attribute the resulting dataset correctly, like e.g. (c) Shape data: OpenStreetMap contributors. Showing up at a local (now perhaps virtual) meetup and discussing your plans might perhaps even be the start of a joint effort and great cooperation.

[hannesj]

HSL in Helsinki has a specific tool to match the GTFS shapes to the OSM street network, gtfs_shape_mapfit, which is used in the preprocessing phase to keep the actual graph build as short as possible. I think there has been new contenders for the same functionality in recent years, but there hasn't bee time to evaluate those, as it is working well enough.

[hannesj]

Based on my testing the BusRouteStreetMatcher works still today. You can enable it by specifying "matchBusRoutesToStreets": true in your build-config.json.

The referred NetworkLinkerLibrary was used for linking bus stops to the street network, and the matched edges were given preference, so that part could be removed, but the matcher still works and is quite self-contained, so I don't see the need for removing it. EDIT: Done in #3095

[t2gran]

@hannesj Tank you for the update, I will close this issue - we will keep the BusRouteStreetMatcher since there is still a use-case for it.