Conversation
Fixes #40 this is caused by an error in factor_order.
…nstants. These edges don't play a part in derivative computation but they lead to errors in reachability calculations.
…ics and the constant patch edge reachability bug
…change back when fix general problem of errors in reachability caused by factoring.
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 is a partial fix for #40 (comment).
Edges in the derivative graph which lead to constant nodes can cause incorrect reachability results in other edges in the graph. This causes an error of the form "there is more than 1 path from root i to variable j".
This fixes the bug by deleting edges to constant nodes upon derivative graph construction. These edges play no part in the derivative evaluation anyway so it is unnecessary to keep them.
More complex MWE's in the same issue trigger a different bug related to PR #66 (comment). This patch does not fix these problems.