motis-project / motis

Intermodal Mobility Information System
https://motis-project.de
MIT License
197 stars 47 forks source link

handling of GTFS Time values #175

Open derhuerst opened 2 years ago

derhuerst commented 2 years ago

GTFS Time is not defined relative to midnight, but relative to noon - 12h. While that makes "writing" GTFS feeds easier, it makes processing a lot harder. As an example, I expect

I'm currently checking well-known GTFS parsers/consumers for this bug, so I wondered if MOTIS is affected by the described problem on those days that the DST <-> standard time switch occurs on? I'm not familiar with this code base, I tried to dig into how GTFS is parsed, but I couldn't understand enough from quickly looking at the code.


related: https://github.com/google/transit/pull/15

felixguendling commented 2 years ago

Thank you for noting this!

Currently, we're doing it wrong: https://github.com/motis-project/motis/blob/9c5536c8febf415994dc0fac96a918b1a45909b0/base/loader/src/timezone_util.cc#L82-L94

Basically, the GTFS and HAFAS Rohdaten parsers create a schedule.raw in the Flatbuffers format specified in base/loader/schedule-format. Currently, the schedule.raw contains local times + timezone information. Maybe a better idea would be to move the translation from local time to UTC into the HRD/GTFS parsers so the schedule.raw (which is then the input for the graph builder) already contains UTC.

The question is also: do all the converters and data management tools for GTFS conform? Maybe they overlooked this section in the GTFS standard, too.

derhuerst commented 2 years ago

The question is also: do all the converters and data management tools for GTFS conform? Maybe they overlooked this section in the GTFS standard, too.

I have listed some of the relevant Issues here: https://github.com/r-transit/tidytransit/issues/175#issuecomment-979213277

Also, this is the discussion in the GTFS repo: https://github.com/google/transit/pull/15

felixguendling commented 2 years ago

Thank you very much for your effort of informing everybody about this issue! I think handling these edge cases is important to make Open Source + Open Data a viable option compared to commercial closed approaches.

I'll look into this and try to make your test case green :-)

felixguendling commented 1 year ago

This should now be correctly implemented in MOTIS' upcoming nigiri core: https://github.com/motis-project/nigiri/blob/60d256a410a69d0f832c1e4d6910228a77a54f62/test/loader/gtfs/service_test.cc https://github.com/motis-project/nigiri/blob/60d256a410a69d0f832c1e4d6910228a77a54f62/test/loader/gtfs/gtfs_to_utc_test.cc

Thank you @derhuerst for pointing out this issue! Your test cases were really helpful for implementing the conversion correctly.