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: Non-deterministic behaviour of SwissRailRaptor #3568

Merged
merged 6 commits into from
Nov 22, 2024

Conversation

tkchouaki
Copy link
Contributor

Hi @mrieser,

@sebhoerl and I noticed that some of our simulations were not determinstic (i.e, running the same simulation twice did not necessarily generate the same events and plans). After investigating we found out that there are at least 2 places in SwissRailRaptor that are not deterministic.

  • DefaultRaptorStopFinder.findStops does not always return the same stops, we tracked this down to the initialization of the stops QuadTree in SwissRailRaptorData.create where stops are retrieved from a HashMap. We replaced it by a LinkedHashMap to always have the same order.
  • After this fix, raptor still does not always return the same elements, we tracked this down to SwissRailRaptorCore.calcLeastCostRoute where some HashMap objects are also built then looped over, which we also replaced by LinkedHashMap.

This PR introduces a RaptorDeterminismTest class in ch.sbb.matsim with a unit test to uncover non-deterministic behaviour in SwissRailRaptor. Moreover, a DeterminismTest class is introduced in org.matsim.contrib.discrete_mode_choice to check that a whole simulation is deterministic.

The commits introducing the tests are separated from the ones introducing the fixes, so you can checkout each step and see what is being solved.

As some non-deterministic behaviour only appear in rare cases, we will incorporate some systematic checks on other simulation scenarios in our internal workflows and keep you updated if any other source of variability is identified.

Best,
Tarek Chouaki.

@sebhoerl sebhoerl requested a review from mrieser November 18, 2024 12:55
@steffenaxer
Copy link
Collaborator

Thanks for improving this. You spend probably some hours in investigating this issue!

@Janekdererste
Copy link
Member

Janekdererste commented Nov 19, 2024

I would recommend adding a short comment, as the next person will otherwise re-replace the LinkedHashMap with a regular one, which tends to be faster.

@kainagel
Copy link
Member

kainagel commented Nov 19, 2024 via email

@sebhoerl sebhoerl merged commit d16f858 into matsim-org:master Nov 22, 2024
47 checks passed
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.

5 participants