Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missed trip when arrive-by search-window is off by one minute #5520

Merged

Conversation

t2gran
Copy link
Member

@t2gran t2gran commented Nov 20, 2023

Summary

The dynamic SearchWindow for arriveBy search is off by one minute. The searchWindow is calculated by Raptor after the heuristics is done. Raptor uses the min-travel-time, the search-window and latest-arrival-time to calculate the earliest-departure-time. But, the algorithm rounded the min-travel-time up, not down to the closest minute. This causes departure-time of the journey to all outside(after) the search-window - if the journey arrive within 59 seconds of the latest-arrival-time.

This is fixed by rounding down, instead of rounding up.

Issue

This fixes the bug repoted by @jtorin here.

Tests

✅ There where unit-test on this - they also was wrong. So, the test are now updated and a test on an exact journey matching the min-travel-time is added.

I have also recreated the bug reported by @jtorin and verified that this solves it. I have also adjusted the access/egress to exactly match whole minutes. This will cause the min-travel-time to match the missed-journey-travel-time exact.

Documentation

✅ Relevant doc is updated.

Changelog

✅ This bug has existed in OTP for a long time, but was not discovered, because a bug in the itinerary-filter failed to remove it. This bug was fixed in: #5433

Bumping the serialization version id

🟥 No needed.

In the current version the dynamic search-window is set 1 minute too early to include a trip if
both the minimum travel time can be calculated exact, and the journey arrival-time is withing 59s
of the LAT.
@t2gran t2gran added Bug Entur Test This is currently being tested at Entur labels Nov 20, 2023
@t2gran t2gran changed the title Otp2 fix arrive by off by one minute Fix missed trip when arrive-by search-window is off by one minute Nov 20, 2023
Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (0a0d8b0) 66.95% compared to head (85994c2) 66.91%.
Report is 16 commits behind head on dev-2.x.

❗ Current head 85994c2 differs from pull request most recent head e6ebb46. Consider uploading reports for the commit e6ebb46 to get more accurate results

Files Patch % Lines
...planner/routing/service/DefaultRoutingService.java 36.84% 11 Missing and 1 partial ⚠️
...java/org/opentripplanner/raptor/RaptorService.java 41.66% 6 Missing and 1 partial ⚠️
...entripplanner/raptor/api/request/SearchParams.java 0.00% 4 Missing ⚠️
...uting/algorithm/mapping/RoutingResponseMapper.java 78.94% 2 Missing and 2 partials ⚠️
...ipplanner/raptor/api/model/RaptorAccessEgress.java 0.00% 1 Missing ⚠️
...m/filterchain/ItineraryListFilterChainBuilder.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5520      +/-   ##
=============================================
- Coverage      66.95%   66.91%   -0.04%     
+ Complexity     15794    15755      -39     
=============================================
  Files           1832     1824       -8     
  Lines          70540    70417     -123     
  Branches        7410     7403       -7     
=============================================
- Hits           47229    47120     -109     
+ Misses         20840    20830      -10     
+ Partials        2471     2467       -4     

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

@jtorin
Copy link
Contributor

jtorin commented Nov 21, 2023

I've reviewed the last commit (85994c2) as requested, it looks ok ✔️.
Should I review more or how do we progress?

@t2gran t2gran marked this pull request as ready for review November 21, 2023 14:59
@t2gran t2gran requested a review from a team as a code owner November 21, 2023 14:59
@t2gran
Copy link
Member Author

t2gran commented Nov 21, 2023

I've reviewed the last commit (85994c2) as requested, it looks ok ✔️. Should I review more or how do we progress?

Thank you for that, the base PR is now merged and I have merged dev-2.x into this PR - and as you can see this PR is now just one commit + pluss the merge. You can approve again - or if not I will just merge when I have the second approval. Thanks again for the fast approvel.

@t2gran t2gran added this to the 2.5 (next release) milestone Nov 22, 2023
@t2gran t2gran merged commit 2c247ef into opentripplanner:dev-2.x Nov 22, 2023
5 checks passed
@t2gran t2gran deleted the otp2_fix_arrive_by_off_by_one_minute branch November 22, 2023 12:12
t2gran pushed a commit that referenced this pull request Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Entur Test This is currently being tested at Entur
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants