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

Make Speedy Graph Builder more deterministic #3638

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Royal2Flush
Copy link
Contributor

We have noticed inconsistent car routing when multiple routes with equal travel time/disutility are available. This fix adds the links to the SpeedyGraphBuilder in order of their sorted id, not in order of their index as the standard iterator does. This makes the graph independent of any changes of the order in which the links are created (which to my understanding determines the relationship index<->id)

@Royal2Flush Royal2Flush marked this pull request as draft December 13, 2024 17:41
@Royal2Flush Royal2Flush self-assigned this Dec 13, 2024
@Royal2Flush
Copy link
Contributor Author

The current state of the pull request causes several failed tests (see https://github.com/matsim-org/matsim-libs/actions/runs/12318669861):

  • matsim: Some tests (SpeedyALTTest, SpeedyDijkstraTest, SpeedyGraphTest, ModeRestrictionTest) using the SpeedyGraph fail on tests where there is a symmetrical scenario with multiple equal routes. The router now in some cases chooses the other possibility due to the different ordering. This is in my opinion not at all worrying and I would just adjust the tests to pass with the new ordering
  • application: This is a failure in CreateScenarioCutOutTest, where unrouted plans get routes on a chessboard(!) network and then the network is cut based on the links used. Since the chessboard has a lot of different route options it is also not surprising that something changes here. Again, I would just replace the reference test results
  • taxi: This one is a lot more complex as it has high level integration tests failing with a lot of moving parts (literally). As far as I can see it is mostly small changes in travel time and score for a handful of agents. This might be caused by a different route choice and/or its effects downstream, even if the network used here is not symmetrical.

@Royal2Flush
Copy link
Contributor Author

Before I just replace the expected outcome of the failing tests, could @mrieser as the original author of the SpeedyALT have a quick look at this change?

Also, fyi @michalmac , @nkuehnel and @Aleksander1234519 since it would affects tests in contribs which I think are mostly developed by you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant