FIX: 158 - Ensure GTFS and RAPTOR router have the same stop set

[munterfi]

If a stop occurs in the GTFS schedule, it should be included in RAPTOR, even if no departures are scheduled from that stop. While adding a large number of such stops could negatively impact performance during the generation of transfers at startup, the effect on routing would be minimal since no route scanning occurs from these stops.

When transfers are generated, it could be that connections from or to stops with no trips are available via a footpath / transfer as first or last leg. Currently we are not generating (between stop) transfers when no trip is active on a stop.

Since this issue originates in the GTFS input, we could issue a warning when adding stops with no departures. Ultimately, however, it is the responsibility of the GTFS producer to address this. Our public transit service should handle such cases gracefully but should not attempt to fix the underlying data issue.

This PR:

• Adds all stops to RAPTOR, even if they have no departures / stopping trips or transfers.
• Adjust and refactor GtfsToRaptorConverterIT (make tests passing, asserts are checking actual transfer ids or keys, differentiate between same stop transfers and between stop transfers)

@clukas1: This is an alternative to PR #160 to address the issue #158, which I would prefer due to the consistency between the data in the schedule and the router.

[munterfi]

Hi @clukas1, I have implemented what we discussed in the morning: Only add stops with departures to raptor, catch exceptions from router in service

Reason: In many GTFS there are a lot of parent stops without departures, which would then be added to the raptor, which leads to a lot of copying of the stop arrays.

And some more:

• Move transfer generation to convert package.
• Pass generators to converter instead of transfers.
• Generator uses a stop collection to build transfers instead of the complete schedule.
• Service catches IllegalArgumentException of raptor router and returns an empty list.
• Introduce GtfsToRaptorTestSchedule to reuse the ManualSchedule.
• Rename isoLines to isolines, be consistent.

There are still open TODOs since we should think about the exception handling in the service.

[clukas1]

Some minor improvements needed, mainly I think we can improve how transfers are added to the raptor builder and I would include the fallback to location routing if no stops with departures are included in the request.

[clukas1]

TODO: Add fallback on location routing?

[clukas1]

Maybe also fallback to location isolines?

[munterfi]

Maybe something like this:

@Override
    public Map<Stop, Connection> getIsolines(Stop source, LocalDateTime time, TimeType timeType,
                                             ConnectionQueryConfig config) throws ConnectionRoutingException {
        Map<String, LocalDateTime> sourceStops = getAllChildStopsFromStop(source, time);
        Map<String, ch.naviqore.raptor.Connection> isolines;

        try {
            isolines = raptorRouter.routeIsolines(sourceStops, TypeMapper.map(timeType), TypeMapper.map(config));
        } catch (RaptorAlgorithm.InvalidStopException e) {

            // route from stop location to cover the case where a stop has no departures, but a walk to another
            // stop with departures would be feasible.
            log.debug("{}: {}, trying to route from stop location", e.getClass().getSimpleName(), e.getMessage());
            Map<String, LocalDateTime> sourceLocations = getStopsWithWalkTimeFromLocation(source.getLocation(), time,
                    config.getMaximumWalkingDuration(), timeType);
            isolines = raptorRouter.routeIsolines(sourceLocations, TypeMapper.map(timeType), TypeMapper.map(config));

        } catch (IllegalArgumentException e) {
            throw new ConnectionRoutingException(e);
        }

        return mapToStopConnectionMap(isolines, null, config, timeType);
    }

The solution is relativerly easy for getIsolines, but more complex for the getConnections.

I will try to come up with a seimple solution, this may involve refactoring the cases.

[clukas1]

See my suggestion in ebea0b0, I think we using the template method pattern this can actually be handled fairly nicely. However, I think we should add some connection post processing to add the source stop to the first/lastMile walk etc. and also add post processing with the walk calculator if first/lastMile walks are followed by a walk transfer as first or last leg in the raptor connection.

[munterfi]

Thanks @clukas1, nice work! All changes are good from my side.

Currently I am trying to refactor the service using the template method pattern on a separate branch: https://github.com/naviqore/public-transit-service/tree/concept/refactor-service

I will open a PR into this branch as soon as I am ready.

[clukas1]

@munterfi Changes here look good, should I approve this PR or should we wait for the template method pattern PR (including correct InvalidStopException handling)?

Regarding your new branch, I've had a look and added a commit mainly with comments but also some suggestions how I think we can handle Invalid Stop Exceptions by falling back to location routing in ebea0b0.

[munterfi]

Hi @clukas1, thanks! I think we can merge this PR, since it fixes the original issue.

The new branch is mainly a refactoring to simplify the fallback solution with the location search.