FIX: Stable linking with degeneracy, via sorting#802
Open
nkeim wants to merge 3 commits into
Open
Conversation
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.
This fixes #776 by making sure candidates are considered in order during subnet linking. It supersedes #801 because that fix's performance regression was much worse (and, it did not actually guarantee ordering).
Each subnet linker implementation now sorts source candidates by the number of forward candidates first, and then by their uuid. The performance penalty appears to be within 1%, too small to measure easily.
Previously, the numba linker did not sort by the number of forward candidates, which is a helpful optimization for this depth-first algorithm. Because the core linking algorithm was so fast, this optimization was not always worthwhile in practice. However, introducing sorting by uuid in the numba linker caused a serious performance regression for large subnets, much worse than for the other linker implementations. I found that when the numba linker also sorts by forward candidates, the regression seems to go away. So I include that optimization in this PR.