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

145 bug non pareto optimal solution #153

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

clukas1
Copy link
Member

@clukas1 clukas1 commented Nov 5, 2024

Found the issue, the problem was only present in range raptor configurations and can be considered somewhat edge case.

In this exact case the problem was that there are many departures from Wankdorf and therefore a raptor routing request was made in 6 minute increments starting from the latest time (in my case range 1800s) --> 12:15 (when query time was set at 11:45). At 12:15 (or 12:09, 12:03, 11:57) the optimal earliest arrival time was found with 3 rounds and since the same bestLabelsPerRound data structure is used for all, there was a label set in round 3. And then even if the earliest arrival time is then later found with a 2 round connection, this label will never be removed, thus will be returned without post processing. This post processing / filtering is now done in ch.naviqore.raptor.router.LabelPostprocessor.reconstructParetoOptimalSolutions (commit: 48fb7c3).

A further minor bug (with no big effect) was also found in the process, the method ch.naviqore.raptor.router.QueryState.getActualBestTime did actually not return the actual best time, as it was looking for the highest round label which was not null for a given stop. Again for range raptor the highest round might not always be the actual best time and thus it's better to check all possible times to determine the best. Fix implemented in (d3895b1).

I haven't implemented any tests, as coming up with a dummy schedule to cover this in the raptor tests is currently beyond my mental capacity. And this seems to niche to justify using reflections to do real unit tests on underlying methods.

PS: @munterfi If you feel like implementing a dummy schedule feel free. I'll pay you a beer, else I might do this tomorrow as a small challenge.

@clukas1 clukas1 added the bug Something isn't working label Nov 5, 2024
@clukas1 clukas1 requested a review from munterfi November 5, 2024 21:51
@clukas1 clukas1 self-assigned this Nov 5, 2024
@clukas1 clukas1 linked an issue Nov 5, 2024 that may be closed by this pull request
@clukas1
Copy link
Member Author

clukas1 commented Nov 5, 2024

Simplification of fix added in 23a2265 and tests added in 732f829. Sorry the fear of paying beers has given me some extra mental capacity

Copy link
Member

@munterfi munterfi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @clukas1!
Also, the test case clearly illustrates the issue. Good to merge.

@munterfi munterfi merged commit e5ec3bc into main Nov 6, 2024
2 checks passed
@munterfi munterfi deleted the 145-bug-non-pareto-optimal-solution branch November 6, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Non pareto-optimal solution
2 participants