Fixed a rare non-determinism in the AdaptiveChainPruner#7851
Merged
jamesemery merged 1 commit intomasterfrom May 19, 2022
Merged
Fixed a rare non-determinism in the AdaptiveChainPruner#7851jamesemery merged 1 commit intomasterfrom
jamesemery merged 1 commit intomasterfrom
Conversation
…ityQueue is undefined for tied elements
Codecov Report
@@ Coverage Diff @@
## master #7851 +/- ##
===============================================
- Coverage 86.948% 86.947% -0.001%
Complexity 36927 36927
===============================================
Files 2219 2219
Lines 173673 173674 +1
Branches 18755 18755
===============================================
- Hits 151006 151005 -1
+ Misses 16055 16054 -1
- Partials 6612 6615 +3
|
Author
|
@davidbenjamin Can I get a review on this? I'm not quite sure how to go about testing this (since i think the only reason it appeared in the first place was because it was a very complicated site that was forcing some edges to be filtered to satisfy the variants per graph limit (but non-deterministically). |
Contributor
|
@jamesemery Looks good! |
davidbenjamin
approved these changes
May 19, 2022
This file contains hidden or 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
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.
It turns out the java PriorityQueue implementation is is undefined in its behavior when elements are equal. It turns out it was possible for there to be non-deterministic behavior for chains that had identical scores and the same starting vertex (but different paths in the graph. I think this comparator should be airtight now. @davidbenjamin let me know what you think.