fixed bug in is_variable. Was recursing through the graph for each call. Asymptotically slower than it should have been. #101
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
…ll. Made code asymptotically slower than it should have been. Now much faster for large graphs. changed simplification rules for unary -. Previously the conditionals in the if statements were creating node, which caused allocations even if the condition proved false. Changed to removed all Node create in conditions. over 2x speed improvement.
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.
A long time ago changed is_variable to allow for
q(t)types, even though support for these was never completed. This inadvertently caused the code to become asymptotically slower, because it recursed through the graph for every is_variable call:For graphs with 1e4 nodes was roughly 100x slower, for graphs with 1e6 nodes it was maybe 2000x slower. Don't know because it took too long to run.
is_variable is called by is_constant, and these two functions are called in various places in simplify_check_cache functions.
Changed simplification rules for unary -. Previously the conditionals in the if statements were creating Node instances, which caused allocations even if the condition proved false. Changed to remove all Node creation in conditions. Over 2x speed improvement.