opentripplanner / OpenTripPlanner

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

Keep at least one result for min-transfers and each transit-group in itinerary-group-filter #5919

Open t2gran opened 2 weeks ago

t2gran commented 2 weeks ago

Summary

The MaxLimit filter in GroupByFilter in the ItineraryListFilterChain did only look at generalized-cost when it reduced the number of itineraries in each group. This problematic because the itinerary with the best number of transfers or good alternatives for transit-groups could be lost. The new McMaxLimit filter will look at all pareto criterias used in the Raptor search and keep at least one optimal result for each criteria, before limiting the result to N itineraries.

To enable this the OTPFeature.MultiCriteriaGroupMaxFilter must be set in otp-config.json.

Issue

There is no issue for this, but one case we have seen at Entur is:

Skjermbilde 2024-06-19 kl  14 31 43

The alternative with two legs with the same agency is filtered away, in favor of a slightly better cost alternative with two legs with different agencies. The first leg is the same, but because of the switch to another agency the customer has to bye two tickets and have no guarantee in case of delays. So we want both options to be shown.

This is related to #5436 and #3833.

Unit tests

✅ All new business logic has unit-tests with 100% coverage.

Documentation

✅ There is extensive JavaDoc documentation, and a little bit of explaination on the new OTPFeature to enable this behaivour. OTP will work as before, if the feature is not enabled in config.

Changelog

✅ This is a minor improvment

Bumping the serialization version id

🟥 Not needed

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 92.15686% with 12 lines in your changes missing coverage. Please review.

Project coverage is 69.51%. Comparing base (67e6510) to head (9b4b7fe). Report is 60 commits behind head on dev-2.x.

Files Patch % Lines
...m/filterchain/ItineraryListFilterChainBuilder.java 35.71% 8 Missing and 1 partial :warning:
...rithm/mapping/RouteRequestToFilterChainMapper.java 0.00% 1 Missing and 1 partial :warning:
...orithm/filterchain/filters/system/mcmax/State.java 98.78% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev-2.x #5919 +/- ## ============================================= + Coverage 69.46% 69.51% +0.05% - Complexity 17068 17137 +69 ============================================= Files 1928 1941 +13 Lines 73580 73821 +241 Branches 7550 7562 +12 ============================================= + Hits 51109 51316 +207 - Misses 19847 19876 +29 - Partials 2624 2629 +5 ```

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

t2gran commented 2 weeks ago

This PR is improving the group-by filter, reminding improvements is documented her https://github.com/opentripplanner/OpenTripPlanner/issues/5923