valhalla icon indicating copy to clipboard operation
valhalla copied to clipboard

GTFS Readiness Checklist

Open kevinkreiser opened this issue 3 years ago • 10 comments

  • [x] fix stitching in ingest_transit. for whatever reason the stitcher cant find the missing destination/origin for 1 of the stop pairs in the test. the result is we get a couple of logs that say we are bailing on one of the stop pairs. this is because the pair connects across the tile boundary (which is a great thing to test)
  • [x] make sure convert transit can actually add the edges between stations (actually between stations platforms) so that you can actually route on a kRail or kBus edge
  • [x] the routing algorithm doesnt work, says it cant find a transit station near by (shouldnt be the case we know the connection does work currently so perhaps). im hoping fixing the first 2 will make this one go away but i expect there to be more routing issues
  • [x] fix the routing algorithms likely bugs due to the code being unexorcised for years
  • IF WE MADE IT HERE WE COULD SAY WE ARE IN BETA WITH CAVEATS
  • [x] hierarchy builder is not fully robust to the presence of transit data, so currently we cant build hierarchies if we want to have transit data as well. this in itself wouldnt stop us from supporting transit but its definitely required to be fixed before transit can be considered fully functional again.
  • [x] unit test multi feed imports and fix if broken
  • IF WE MADE IT HERE WE HAVE SOMETHING WE COULD SAY IS OUT OF BETA
  • [ ] transit connect edges are not always formed on both sides of the street network edge, this happens because we currently dont look in adjacent tiles to find the end nodes. ive put a bunch of hints in the code about what to do to fix this (and also get better matches for edges to connect stations to)
  • [ ] clean up the code in convert transit where we hook up the 3 transit node types and connect them to each other. i wrote a big todo in there
  • [ ] another group of tasks to tackle adding transit to the graph after the graph is fully built as mentioned over in: https://github.com/valhalla/valhalla/issues/3806#issuecomment-1339231858

TODO: check off some of this list after we have merged #3988

kevinkreiser avatar Mar 03 '23 01:03 kevinkreiser

According to above we'd be ready for early beta, but I'd still like to merge the next PR when I had a chance to run a full real feed into the graph build and manage some routing.

I won't copy/paste all my observations here, so also refer to this list at some point (also need to talk about some of those things before someone jumps in): https://github.com/valhalla/valhalla/pull/3988#issue-1598966223

nilsnolde avatar Mar 10 '23 18:03 nilsnolde

What's left before cutting a new release? You wanna wait until we figured out the hierarchies?

For me, the minimum would be to:

  • [x] add a bit more testing to the /route PBF output
  • [x] fix the isochrone algo for multimodal

Both should hopefully be rather mechanical.

nilsnolde avatar Mar 16 '23 15:03 nilsnolde

in addition to the 2 you said, i would say we need to do something about the hierarchies. 2 options exist:

  1. leave it broken for enabling hierarchies BUT error out of the tilebuild when there is transit data and hierarchies are enabled OR
  2. debug and fix hierarchybuilder

i prefer the latter but only very slightly. im ok with releasing so long as the build fails immediately with the currently unsupported configuration

kevinkreiser avatar Mar 16 '23 16:03 kevinkreiser

Jep agreed. I'd commit to fixing them, but not sure what's lurking there yet;) I can have a look before our next session, to get myself up to speed, and maybe we can pair on it for a while.

nilsnolde avatar Mar 16 '23 16:03 nilsnolde

I got curious about what's exactly failing when building transit and hierarchies and enabled it in the test. It didn't error out, but also I couldn't get the test to build shortcuts for some reason (which might still indicate a bug?).

Now, I finally had time to do a bigger test with this tiny real feed: https://transitfeeds.com/p/swhn-stadtwerke-heilbronn/1239/latest (after hacking the calendar.txt to 2023). Aaaand it just works!

2023/03/24 12:10:22.250854 [INFO] GraphFilter - nothing to filter. Skipping...
2023/03/24 12:10:22.257370 [INFO] Adding 2 transit tiles to the local graph...
2023/03/24 12:10:40.482419 [INFO] Found 1535991564216692 connection edges
2023/03/24 12:10:40.482449 [INFO] Finished - TransitBuilder took 18 secs
2023/03/24 12:10:40.482519 [INFO] HierarchyBuilder
2023/03/24 12:10:41.229829 [INFO] Done HierarchyBuilder
2023/03/24 12:10:41.233350 [INFO] Creating shortcuts on level 1
2023/03/24 12:10:41.614391 [INFO] Finished with 942 shortcuts
2023/03/24 12:10:41.614456 [INFO] Creating shortcuts on level 0
2023/03/24 12:10:41.687036 [INFO] Finished with 514 shortcuts

This time it reported built shortcuts and some bullshit 1535991564216692 transit connection edges. I ran the service and verified I can route on multimodal & auto! Pheew, that's a huge relief for me! I'd bet this one fixed it: #4020..

Then we're very close to beta. I'll just add some more testing next week.

nilsnolde avatar Mar 24 '23 12:03 nilsnolde

So it seems that feeding a GTFS triggers a very very quick build

shakedk avatar May 11 '23 17:05 shakedk

Na, it takes quite a long time currently. There's TODO's for that. We can make it quite a bit faster, lots of the code isn't really optimized yet. Breaking out the transit building of the general tile build pipeline will go a long way, and so will removing the intermediate PBF stage (which remains to undergo a somewhat closer feasability study..).

nilsnolde avatar May 11 '23 18:05 nilsnolde

Excuse me, but is this usable in some way? Are there any instructions / examples on how this could be used with valhalla docker / compose?

marisancans avatar Jan 02 '25 22:01 marisancans

Hi, any update on gtfs support ?

thblt-thlgn avatar Mar 12 '25 09:03 thblt-thlgn

GTFS is definitely supported, I use it in production.

We only miss up-to-date docs on how to ingest GTFS feeds (see #5049 that's on my to-do list)

But in the meantime this docker image seems to support it out of the box (I don't use it so no guarantee from my side).

jthiard avatar Mar 12 '25 10:03 jthiard