Depth-first traversal column reset #659

Merged
merged 2 commits into from Feb 7, 2017

Conversation

Projects
None yet
2 participants
@laerreal
Contributor

laerreal commented Feb 6, 2017

I've investigated the issue about KeyError in propagate_frontier. That was 52-th column in may case. Sometimes the error had not appeared. Hence, I guess the error is related to non-determinism of incremental graph loading. All children of currently added nodes are assigned a column. But, some of them might not be reset because they are not added yet. During consequent iteration the algorithm assumes that corresponding column is allocated because it was not reset. This time the error raises.

I suggest to use depth-first traversal of graph during reset. Complexity of the algorithm seems to remain O(N) while this guaranties that all nodes are reset.

I had not got an error during 10 runs. However, it would be nice if you check it at your side.

laerreal added some commits Feb 6, 2017

dag: use depth-first traversal to reset columns
Signed-off-by: Efimov Vasily <real@ispras.ru>
dag: revert hotfix of undefined frontier
The propagate_frontier assumes that frontier column is always defined. KeyError
exception is an indicator of an error in another part of the grid layout
algorithm. So, do not except KeyError to get the error as soon as possible.

The hotfix may be applied again for temporary error elimination.

At the other hand, defining frontier in this place too, complicates the
algorithm logic.

Signed-off-by: Efimov Vasily <real@ispras.ru>
@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Feb 7, 2017

Member

Thanks for the detailed descriptions. Makes perfect sense

Member

davvid commented Feb 7, 2017

Thanks for the detailed descriptions. Makes perfect sense

davvid added a commit to davvid/git-cola that referenced this pull request Feb 7, 2017

Merge pull request #659 from laerreal/dft_reset
* laerreal/dft_reset:
  dag: revert hotfix of undefined frontier
  dag: use depth-first traversal to reset columns

Signed-off-by: David Aguilar <davvid@gmail.com>

@davvid davvid merged commit 9678b89 into git-cola:master Feb 7, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment