opentripplanner / OpenTripPlanner

An open source multi-modal trip planner
http://www.opentripplanner.org
Other
2.12k stars 1.01k forks source link

Add timePenalty to Transmodel API #5910

Closed t2gran closed 1 week ago

t2gran commented 3 weeks ago

Summary

The time-penalty is an input parameter adding a virtual time penalty and cost penalty to itineraries with less preferable access/egress. To test this feature it would be nice to be able to list the actual calculated penalty for the access/egress it is applied to. The internal model operate with generalized-cost and generalized-cost-including-penalty. This is NOT copied over to the API. The reason for this is that we use the generalized-cost without penalty rarely (in itinerary-filtering). So, to make the API less confusing the API only have one generalized-cost. The calculated time-penalty is however added as an output field. So, it should be easy to subtract the penalty in cases where the internal generalized-cost is debugged.

Issue

These is not issue for this. This is just a small pice missing from the #5180 PR.

Unit tests

✅ Only a small mapper with logic is added, and a unit test on this is provided. The old REST API is changed to return generalized-cost including time-penalty. I did not add a test for this, since this code is going away soon.

Documentation

✅ All new API type and fields are documented, so are new classes and method.

Changelog

🟥 This allow debugging timePenalty in the Transmodel API easier - not worth mentioning in the release log.

Bumping the serialization version id

🟥 This do not change any serialized types.

codecov[bot] commented 3 weeks ago

Codecov Report

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

Project coverage is 69.47%. Comparing base (b1a7c53) to head (7f5ba09). Report is 12 commits behind head on dev-2.x.

Files Patch % Lines
...nsmodel/model/plan/TripPatternTimePenaltyType.java 89.65% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev-2.x #5910 +/- ## ============================================= + Coverage 69.45% 69.47% +0.01% - Complexity 17066 17074 +8 ============================================= Files 1928 1930 +2 Lines 73580 73625 +45 Branches 7550 7550 ============================================= + Hits 51108 51152 +44 - Misses 19847 19849 +2 + Partials 2625 2624 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

t2gran commented 3 weeks ago

When testing this we noticed that time-penalty is added to the entire FLEX access/egress. The access may or may not include a walking leg from the FLEX leg to a regular stop where the journey can continue using regular transit. If FLEX trip ends at a regular stop, then so do the access leg. But, Raptor will now add transfers to this. In this case the time-penalty is only applied to the transfer when it is part of the access, not the one added by Raptor. The result set of legs can be identical. This is bad, and we need to decide how to apply the penalty. I will bring this up in a developer meeting and then create an issue for it.