Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid implied alignments #166

Open
recursion-ninja opened this issue Jun 28, 2020 · 1 comment
Open

Invalid implied alignments #166

recursion-ninja opened this issue Jun 28, 2020 · 1 comment
Labels

Comments

@recursion-ninja
Copy link
Collaborator

recursion-ninja commented Jun 28, 2020

We currently generate invalid implied alignment. The pre-order traversal of implied alignment algorithm relies on knowing if the current node is the left or right child of it's parent. This information is context sensitive, potentially different for each dynamic character, given that it will depend on the character's traversal foci selected during the rerooting procedure. Currently, the information of the whether the current node is left or right child is passed into the implied alignment algorithm from the pre-order traversal infrastructure code. This supporting infrastructure code supplies erroneous left or right child values that do not correctly reflect whether the child is the left or right child of the parent node as determined by the re-rooting context. This garbage input to the implied alignment algorithm causes garbage alignments as output.

This issue is critical and should be fixed as soon as possible.

@Boarders
Copy link
Collaborator

A quick note: currently we use an IntMap e data structure for child nodes which makes consistently tracking left and right nodes extremely difficult to impossible and a direct rewrite of this code will probably require herculean effort to get closer to correct for what the algorithm needs (this code is already inherently extremely fiddly). The new graph type uses a pair type and should make it easier to add this sort of information but this is not at all in place at present on that branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants