serviceDayLookout: add parameter to look more days out in future (or past, for arriveBy=true)

[sdjacobs]

Some transit agencies, especially in rural areas, run infrequent service. For example, the Kingdom Shopper 2 run by Rural Community Transit (RCT) in Vermont runs the 2nd and 4th Wednesday of every month. In these cases, it would be good to give a result for trip plans even if the time is quite far from the time that the service runs.

This PR adds an extra parameter, serviceDayLookout, which when specified allows OTP to consider more ServiceDays when looking for transit service. If serviceDayLookout=n, OTP will search out n days in the future for service, or n days in the past for arriveBy=true searches. If serviceDayLookout is not specified, behavior is unchanged.

This PR includes a test of this behavior using RCT data.

[gmellemstrand]

I've looked through the code and tested this, and it looks good. I'm just wondering how you are using this feature from the client. How do you know when to specify extra servicedays? Is it something you do if the main search does not return any results?

I also tested a search between Oslo and Bergen on the Norwegian network, and search time increased about 50% when specifying a serviceDayLookout of 7 days. Searches within Oslo increased by about the same.

We have added another feature which relates to this, but fixes another problem. This feature is probably Entur-specific, but it's worth mentioning. We have some very long routes (7 days), which you should be able to board midway (say after 3 days). This feature identifies these routes and searches 7 days back/forward for just those routes. This is to get these specific routes to work without significant slowdown.

entur@40f3282

(This is one of the commits related to the feature, to give you an idea of how it works.)

[sdjacobs]

We're just setting the serviceDayLookout to be very large globally (31, since some routes are once a month in Vermont). Since it's a small network, the performance effects weren't a concern.

This is making me think though, maybe a more flexible solution would be to make the change in TransitBoardAlight:

OpenTripPlanner/src/main/java/org/opentripplanner/routing/edgetype/TransitBoardAlight.java

Line 266 in c9e760b

for (ServiceDay sd : rctx.serviceDays) {

Maybe instead of searching through a specific number of days, if you determine that the nearest day with service is too far away, it could calculate what ServiceDay to instantiate by looking at the service IDs for trips in the TripPattern. This still might have poor performance effects though, I'm not sure. (I would think in both implementations performance would depend on how many TripPatterns have intermittent service.)

[gmellemstrand]

OpenTripPlanner/src/main/java/org/opentripplanner/routing/edgetype/TransitBoardAlight.java

Lines 250 to 261 in c9e760b

	/*
	* Find the next boarding/alighting time relative to the current State. Check lists of
	* transit serviceIds running yesterday, today, and tomorrow relative to the initial
	* state. Choose the closest board/alight time among trips starting yesterday, today, or
	* tomorrow. Note that we cannot skip searching on service days that have not started
	* yet: Imagine a state at 23:59 Sunday, that should take a bus departing at 00:01
	* Monday (and coded on Monday in the GTFS); disallowing Monday's departures would
	* produce a strange plan. We also can't break off the search after we find trips today.
	* Imagine a trip on a pattern at 25:00 today and another trip on the same pattern at
	* 00:30 tommorrow. The 00:30 trip should be taken, but if we stopped the search after
	* finding today's 25:00 trip we would never find tomorrow's 00:30 trip.
	*/

It seems like you cannot break after finding a trip, because of overlap near midnight. And possibly ring routes could also complicate this?

I think if you could break after the first trip, you wouldn't get so much of a performance hit from searching a long time into the future.

EDIT: I think if you assume that no trips are longer than 24 hours, you could just keep looping through serviceDays and when a trip is found, just search one more serviceDay. That assumption has to hold now anyway, with yesterday/today/tomorrow as the search range.

[gmellemstrand]

I did some tests and I think what you are thinking about calculating what serviceDays to instantiate is interesting. However, it might be not be worth spending too much time on something that is going to be replaced by a new algorithm anyway. I think the feature as-is is useful to us at Entur, and I see no problem with it being merged into master.

[t2gran]

I am not sure if I am inline with the rest here, but this is my view on issues and PRs:

For the future it is good to create an Issue and a PR, when the PR contain new feature(s). Then the functionality and high level design should be discussed in the issue and code in the PR.

The group of people interested in issues and PRs are slightly different and the lifecycle, too. An issue may be referenced and document a feature, hence provide insight for a long time after it is closed - a PR more or less when the PR is merged into master.

Any thoughts on this?

[abyrd]

@t2gran that's how I see it too - an issue describes the problem or need, people can discuss it on the issue, then a PR responds to that issue with a proposed implementation.

We don't need to be completely strict - for a small fix or change, a detailed description on the PR might be acceptable, but generally it's better for the PR to be a response to an issue (ideally with a commit message somewhere in the PR that says fixes #123, auto-closing the issue when the PR is merged).

[gmellemstrand]

Looks good for now. The implementation looks good and straight forward. We are going to reimplement this in Raptor, but the functionality should basically be the same.

[drewda]

@sdjacobs is this PR still relevant to merging all the VTrans MOD work in to mainline OTP? If so, please update and resolve conflicts -- or let others know how we can help get this ready to merge.

[sdjacobs]

Thanks @drewda! I just resolved conflicts. This PR is relevant to the VTrans MoD project, but it's orthogonal to GTFS-Flex: we needed this change to show customers trips which may be relevant but far in the future (e.g. a twice-monthly "shopper" route.)

At this point, all that's remaining are the approvals. @gmellemstrand could you take another look? Nothing has changed since your last approval but it's been merged with the dev-1.x branch.

[sdjacobs]

Just pushed some changes. I added some documentation. Also, in reference to the issues @abyrd and I discussed above, I made some specific decisions (though I'm happy to change back if people disagree):

• Refactored to avoid the double-adding of service days
• Removed from API parameters, to avoid callers locking up the server. This also provides us more flexibility if we want to change this moving forward.
• I didn't add the "backward lookout" we discussed. Looks like @gmellemstrand's code only increases the amount of service days for the specific routes its applicable to. That's a more performant way of handling this, but I thought outside the scope of this PR.

[gmellemstrand]

The way I see it we have these problems to solve:

1. Show infrequent trips in areas with little transit coverage.
2. Multi-day trips consisting of multiple legs (can be up to 3-4 days in Norway, and longer when we start thinking about multiple countries)
3. Get a timetable view of up to a month at a time
4. Single legs that lasts multiple days and can be boarded at any of those days (Hurtigruta takes 7 days to travel all along the Norwegian coast)

This pull request solves problem 1 and 2 at the cost of about 50% increase in search time when testing with the Norwegian graph. Probably with a combination of running parallell searches for each applicable day for 1 and 3, and loading multiple service days for 2. Case 4 would probably best be handled with special logic for those specific routes.

I agree that the scope of this PR should be to solve 1 and 2. Maybe add a parameter called something like "maxServiceDayLookout" in router-config and then keep "serviceDayLookout" as a request parameter?

[sdjacobs]

@gmellemstrand @abyrd are there other changes I should make to this PR?

[gmellemstrand]

I have no other requests for changes before this is merged.

[abyrd]

Merged! I'm not going to cherry-pick this one onto dev-2.x because it is coupled to the transit routing implementation.