opentripplanner / OpenTripPlanner

An open source multi-modal trip planner
http://www.opentripplanner.org
Other
2.18k stars 1.02k forks source link

Duplicate "Follow Signs" steps with the new signposted pathways feature #5322

Closed EmmaSimon closed 4 days ago

EmmaSimon commented 1 year ago

Really loving the follow signs and entrance/exit walking directions added in #5285! However, I did notice an issue where the same sign is repeated multiple times in a row. I know this is based directly on our GTFS pathways, so I'm not sure if it would be expected that our pathways not be set up in such a way that doesn't generate duplicates like this, or if it's reasonable for OTP to detect and filter out cases like this. I haven't dug too deeply into our GTFS yet, but I can see it being reasonable for the same sign to be duplicated multiple times in one path as other paths converge into it to follow the same route.

Expected behavior

The same sign should be followed until a new destination is reached, and isn't immediately followed by multiple duplicates.

Observed behavior

Screenshot 2023-08-25 at 3 17 57 PM

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

79d1cbca0c31c52258fabce48bf225e790c706a9

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

curl -o var/mbta-ma-us.gtfs.zip https://mbta-gtfs-s3.s3.amazonaws.com/google_transit.zip
curl -o var/massport-ma-us.gtfs.zip https://data.trilliumtransit.com/gtfs/massport-ma-us/massport-ma-us.zip
curl --output-dir var/ -O http://download.geofabrik.de/north-america/us/massachusetts-latest.osm.pbf
curl --output-dir var/ -O http://download.geofabrik.de/north-america/us/rhode-island-latest.osm.pbf

Command line used to start OTP

java -Xmx4G -jar otp/target/otp-*-shaded.jar --load var/

Router config and graph build config JSON

build-config.json

{
  "boardingLocationTags": ["gtfs:stop_id", "ref"],
  "embedRouterConfig": false,
  "osmDefaults": {
    "timeZone": "America/New_York"
  }
}

router-config.json

{
  "updaters": [
    {
      "type": "real-time-alerts",
      "frequency": "100s",
      "url": "https://cdn.mbta.com/realtime/Alerts.pb",
      "feedId": "mbta-ma-us"
    },
    {
      "type": "stop-time-updater",
      "frequency": "10s",
      "url": "https://cdn.mbta.com/realtime/TripUpdates.pb",
      "feedId": "mbta-ma-us"
    },
    {
      "type" : "vehicle-positions",
      "frequency" : "1m",
      "url" : "https://cdn.mbta.com/realtime/VehiclePositions.pb",
      "feedId" : "mbta-ma-us"
    }
  ]
}

Steps to reproduce the problem

from: {42.35371, -71.05755} to: {42.36514, -71.01777}

Note the walking directions inside South Station.

leonardehrenfried commented 1 year ago

Your pathways are fine and OTP is wrong here. Are you interested in contributing a fix?

If you are then the code for it is in StatesToWalkStepMapper. I'm afraid that class contains a lot of gnarly, mutable state manipulation code from the early days of OTP that's difficult to understand.

On the upside, the test infrastructure is good and should not be hard to write a few test cases for the various scenarios. Once we have a precise enough test suite we can think about refactoring the logic to make it more readable.

EmmaSimon commented 1 year ago

I'm actually being moved to a new project next week, and will be working on cutting over to OTP2 in prod this week, but I might have some time to work on a fix, depending on how smoothly things go.

github-actions[bot] commented 10 months ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

EmmaSimon commented 10 months ago

This is still an issue, but I haven't had time for a fix yet.

github-actions[bot] commented 7 months ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

EmmaSimon commented 7 months ago

🙅

github-actions[bot] commented 4 months ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

EmmaSimon commented 4 months ago

Keeping open for when I can eventually work on it ⏳

github-actions[bot] commented 1 month ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days