handling of GTFS Time values

[derhuerst]

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

• the departure_time of 00:30 of a trip running on 2021-03-28 to happen at 1616884200/2021-03-28T00:30+02:00, not at 1616887800/2021-03-28T00:30+01:00;
• the departure_time of 06:30 of a trip running on 2021-03-28 to happen at 1616905800/2021-03-28T06:30+02:00, not at 1616909400/2021-03-28T06:30+01:00.

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: google/transit#15

[felixguendling]

Thank you for noting this!

Currently, we're doing it wrong:

motis/base/loader/src/timezone_util.cc

Lines 82 to 94 in 9c5536c

	date::time_zone const* zone = date::locate_zone(tz);
	auto const zero =
	date::sys_seconds(std::chrono::seconds(schedule_begin));
	auto const abs_zoned_time = date::zoned_time<std::chrono::seconds>(
	zone,
	date::local_seconds(std::chrono::seconds(
	schedule_begin +
	(day_idx + SCHEDULE_OFFSET_DAYS) * MINUTES_A_DAY * 60 +
	local_time * 60)));
	return static_cast<time>(
	std::chrono::duration_cast<std::chrono::minutes>(
	abs_zoned_time.get_sys_time() - zero)
	.count());

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]

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: r-transit/tidytransit#175 (comment)

Also, this is the discussion in the GTFS repo: google/transit#15

[felixguendling]

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]

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.