search

marbl / MetagenomeScope

Visualization tool for (meta)genome assembly graphs
https://marbl.github.io/MetagenomeScope/
GNU General Public License v3.0
24 stars 8 forks source link

sfdp relies on GTS even without triangulation requested #135

Closed fedarko closed 5 years ago

fedarko commented 5 years ago

Related TODO: add -nt to the SPQR test invocations of the preprocessing script (since the benefit of triangulation is cosmetic, and not really needed for the tests).

fedarko commented 5 years ago

It looks like the -nt option is being used correctly, and sfdp is still giving a warning? I wasn't sure why the warning was only showing up for the implicit SPQR graph, but it looks like -- when you switch the ordering of "explicit" and "implicit" around in the SPQR layout loop -- the "explicit" layout gives the warning also, which leads me to believe that either my code has a strange bug or it's a race condition-esque thing with PyGraphviz/Graphviz.

See https://github.com/ellson/MOTHBALLED-graphviz/issues/1269 for reference, btw. For the time being, our workarounds for this are:

1) accept the layout program for SPQR layouts as an argument (so the user can use other engines like neato, fdp, circo, twopi, etc, even if they don't have GTS installed).

2) ask the user to build GTS themselves as part of the SPQR installation instructions (question: is it possible to do this and still use graphviz via conda? If not, we'll have to move the graphviz installation to outside of conda -- see here.)

In either case, we should also make setting -nt change overlap=false to overlap=true. This won't change anything now due to the Graphviz bug mentioned above (1269), but in the future it'll actually make -nt work as intended (we should also consider renaming -nt to -ngts or something).

fedarko commented 5 years ago

I guess sfdp just requests GTS regardless, even if you don't request triangle smoothing and even if you set overlap=true (although that looks like a bug -- again, see ellson/MOTHBALLED-graphviz#1269).

So it seems our options are the two above: enable the user to not use sfdp, or ask the user to build GTS themselves. (Alternately, we could just say "hey uh sfdp is going to give a warning but it works anyway but might include overlaps," which isn't that unreasonable.)

fedarko commented 5 years ago

This is closed in the python3support branch, in which we do away with -nt entirely in favor of just setting overlap="false"; or overlap="scalexy"; at different levels of the SPQR layout. sfdp still throws a warning, but that's not a big deal; we can catch this in the future, and per here either overlap option should be supported without building GTS (overlap=false downgrades to overlap=voronoi).