Fix for reproducability of results #506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Encountered reproducability issues when running more complex problems against Jsprit. This issue manifests when the ruinedJobSet is created in RuinAndRecreateModule. Ordering in the HashSet is not guaranteed. When re-running the same problem occasionally the order in the HashSet of some Jobs were slightly different. When jobs are shuffled inside an InsertionStrategy (e.g. BestInsertion and BestInsertionConcurrent) the different ordering is exhaserbated. The `Collections.sort(unassignedJobList, new AccordingToPriorities());` ensures that the order of equal items is guaranteed, resulting in different insertions for different runs of the same problem. This issue only showed itself when running with threads. The reason for this is unkown. Notably, when disabling clustersRegret and stringRegret the issue no longer occurred. The reason for this is unclear but is likely related to the order of items in the unassignedJobs list returned from the regret classes. From tests, simply swapping to a LinkedHashSet did not remove this ordering problem so the unassignedJobs were inserted to a list and sorted. Since an ArrayList is faster on iteration the ArrayList was kept, however re-inserting the sortedUnassignedJobs may be preferrable.
|
Nice! I can confirm that after applying this fix, we have been so far able to reproduce all runs. @oblonski Would be great to have this PR merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
I've spent the past few days investigating a reproducibility problem and I think I've found a solution for it, however the ultimate source is still unknown to me.
Since we're using jsprit in a production system (thank you btw!) I can't publicly share the data I've been using to test this and have had trouble writing unit tests for it. We're still running on jsprit
1.7.2but I did all my investigation against the master branch.What we were seeing in production was that the same problem, or near identical problems would sometimes find very different solutions. We had just recently enabled using threads in jsprit so we though this would be the source of the issue. However it seemed only tangentially related.
More than happy to help further the investigation into whats causing this in the first place but I might need some guidance.
Here's the comment from my commit hopefully explaining the issue and how I've decided to resolve it:
Let me know if there's any additional changes you'd like me to make or if I can answer any questions!
Cheers