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

odgi flip: avoid introducing useless edges and losing edges from the original graph #485

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

AndreaGuarracino
Copy link
Member

This PR introduces a different way to construct the flipped graph:

  • copy all edges from the input graph
  • flip paths
  • check missing edges (due to flipped paths)
  • add missing edges (if any)

In the first tests, this avoids introducing edges that are not used in the flipped paths and avoids losing edges that are present in the input graph.

@AndreaGuarracino AndreaGuarracino merged commit 5fb309f into master Mar 23, 2023
@anshumanmohan
Copy link

anshumanmohan commented Apr 4, 2023

Thanks for this, @AndreaGuarracino! I can recreate the fact that it now (correctly) does nothing for the graph LPA.gfa. However, it now does the same fishy thing for note5.gfa.

When I run flip on note5, the only meaningful change is that
L 3 - 4 + 0M
becomes
L 4 - 3 + 0M.
The paths, segments, and other links remain the same.

If I understand flip correctly, the correct behavior for this graph is to do nothing.

anshumanmohan added a commit to cucapra/pollen that referenced this pull request Apr 18, 2023
We agree with odgi on all the handmade files except note5.gfa
I think odgi is doing something fishy there;
see pangenome/odgi#485 (comment)

We agree with odgi on all the handmade files except flip4.gfa
flip4 is not a valid graph and so arguably it does not matter what we do with it,
but `odgi flip` does currently _fix_ flip4, so I think it's worth keeping around.
The point of divergence is that we do not currently fix graphs like flip4, though it's easy enough to do if that were desired.
See pangenome/odgi#496
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants