Implement GTFS Flex safe duration spec draft

[leonardehrenfried]

Summary

It implements the proposed fields flex_safe_duration_factor and flex_safe_duration_offset which are currently undergoing specification and discussion at MobilityData/gtfs-flex#79

These fields give data producers some influence over how travel times are computed for flex trips. The implementation in this PR limits itself to what the OTP flex implementation calls UnscheduledTrip but the name "demand-responsive" is used more commonly. Here the duration modifier can add (or even subtract) travel time to what OTP's A* algorithm thinks it will take to go from origin to destination.

These fields are experimental as of now so I expect a few future changes.

Unit tests

Lots added.

Documentation

Lots of Javadoc.

Bumping the serialization version id

✔️

cc @tsherlockcraig @westontrillium

[codecov]

Codecov Report

Attention: Patch coverage is 94.33962% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 67.94%. Comparing base (8823782) to head (695161a).
Report is 35 commits behind head on dev-2.x.

Files	Patch %	Lines
...a/org/opentripplanner/gtfs/mapping/TripMapper.java	88.88%	0 Missing and 1 partial ⚠️
.../main/java/org/opentripplanner/model/StopTime.java	0.00%	0 Missing and 1 partial ⚠️
...ner/routing/api/request/framework/TimePenalty.java	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5796      +/-   ##
=============================================
+ Coverage      67.90%   67.94%   +0.03%     
- Complexity     16556    16575      +19     
=============================================
  Files           1910     1911       +1     
  Lines          72436    72455      +19     
  Branches        7447     7448       +1     
=============================================
+ Hits           49190    49231      +41     
+ Misses         20725    20700      -25     
- Partials        2521     2524       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leonardehrenfried]

@LeoFrachet @FinnLidbetter

[optionsome]

Can the safe or mean offset/factor be one that decreases the travel time? I quickly took a look at the spec and didn't notice a mention about it.

[leonardehrenfried]

Yes, it can also lower the time, although this is not explicitly stated in the spec.

[leonardehrenfried]

At least in this current implementation, I'm not preventing it from happening.

[optionsome]

Should we stick to call to calling them modifiers instead of factors? Or is there some difference?

[leonardehrenfried]

Yeah, sorry, I did several renames of this field and made a mess. Lets discuss in the dev meeting what this field should be called.

[t2gran]

Lets use timePenalty here - modifier is too wide. Factor is not correct, it is more than a factor.

[optionsome]

Same as above.

[LeoFrachet]

Thanks for that implementation!

Something that was discussed in the past but never decided was the relationship between that penalty and traffic. Do we agree that here you are applying the penalty after applying traffic? (Assuming traffic is at least y = ax, the order of application has an impact since we have an offset.)

[leonardehrenfried]

@LeoFrachet Not sure if you're aware but OTP doesn't have any real-time road traffic information.

[t2gran]

@LeoFrachet To answer your question, I would apply the penalty BEFORE traffic to make the behaviour simpler and more deterministic. You could argue that there is often a correlation between traffic and delays and the wanted penalty, but I think this is not always true and implisitt. I good principle for designing models like this is to keep each factor independent - so traffic should not depend on penalty and vis versa. It become very difficult to tune system where features propagate.

Since OTP does not have support for traffic, this only apply to where in the code we put this. So the question for us is: If traffic is added, will this feature still work as intended? If not, should we consider another implementation? This is about designing with the ability to adopt change.

[tsherlockcraig]

From this user's perspective, I agree this should be separate from any consideration of traffic, which I think is accounted for in the spec: "The MeanTravelDuration may be calculated for the time and the day of the trip to take into account traffic; in other words the consumer is expected to know that DrivingDuration is dynamic." I'm not sure that before/after necessarily matters, and the spec implies this is "after" in the sense that the consumer's independent MeanTravelDuration calculation is assumed to already account for traffic (if the consumer is able to account for traffic). But regardless, it should be additive to traffic if traffic is considered.

[optionsome]

Is this comparison reliable or should we use an epsilon?

[leonardehrenfried]

Well, if it's not exactly 1.0 then I would argue that it does modify and therefore should return true.