Dot2tex reverses Poset.show() upside down

[jmantysalo]

Poset([[1,2],[[1,2]]]).top() says 2, so one expect

Poset([[1,2],[[1,2]]]).show()

showing to "2" on top. And so it happens without dot2tex. However, it shows "1" at top if dot2tex is installed. This was changed from version 6.1 to 6.2.

This ticket implements a rankdir option for DiGraph.layout_acyclic and sets a default value of 'up' for all implementations, which fixes the above inconsistency, while leaving the down feature available to whoever would really care.

[nthiery]

+1 to uniformly drawing posets with the top on top. This has annoyed me as well.

For LaTeX / PDF output, this is taken care of by the backward option of set_latex_options in Poset.hasse_diagram (called by Poset._latex_).

For plots, Poset.plot calls Graph.plot on the hasse diagram, and passes it by default the layout=acyclic option. The layout is then computed by Graph.layout_acyclic which in turn either calls Graph.layout_acyclic_dummy or Graph.layout_graphviz if dot2tex is available.

It turns out that Graph.layout_acyclic_dummy orients edges pointing up, while, Graph.layout_graphviz uses the default behaviour of graphviz, namely to have edges pointing down. Hence the varying behaviour.

Soo ... what's the cleanest way out?

One strategy:

• have an option in graphs stating whether the preferred acyclic layout is going up or down (a generalization of the backward trick in set_latex_option)

• make sure both layout_acyclic_dummy and layout_graphviz honor this option.

• configure this option appropriately in the construction of the Hasse diagram.

Another strategy would be to set as default for all graphs that the acyclic layout shall be going up. That's simpler. But not consistent with graphviz. It also also a larger change for the users (but only those using acyclic layout and having dot2tex installed).

What do you think?

Cheers,

Nicolas

[jmantysalo]

Unfortunately I do not know enought Sage internals to ba able to comment on this. I just reported this...

[ncohen]

Sorry, LaTeX output of graphs is combinat code. I don't maintain it.

[nthiery]

I am not asking you to maintain it. I am asking your opinion on what should be done. Note that this is *not* about LaTeX output.

[ncohen]

I am not asking you to maintain it. I am asking your opinion on what should be done.

It should be fixed. Poset([[1,2],[[1,2]]]).top() says '2', so '2' should be on top. It apparently fails to do so when your package dot2tex is installed (which is why I said 'latex output'), and I do not see exactly what there is to do except... fix it.

Or is the problem that graphviz/dot2tex can't plot the top element on top?

Nathann

[nthiery]

Replying to ncohen:

I am not asking you to maintain it. I am asking your opinion on what should be done.

It should be fixed.

Sure. That's not the question.

I have proposed two strategies for this in 2. Which one do you prefer. If we go for the first one, what's the right way for storing such a ploting option in the graph?

[ncohen]

I have proposed two strategies for this in 2. Which one do you prefer. If we go for the first one, what's the right way for storing such a ploting option in the graph?

Storing a plotting option in the graph? Why on earth would you do that? O_o

Why can't you just make the default be to plot the top element on top, and only do something different if some guy calls my_poset.show(mess_up_top_and_bottom=True) ?

[ncohen]

(I am not even sure that implementing an option to inverse top and bottom upon request is worth the time)

[jmantysalo]

Replying to ncohen:

(I am not even sure that implementing an option to inverse top and bottom upon request is worth the time)

No, it isn't. One can use P.dual().show() when needed for that.

Is it so that we can not have something like show(..., direction='down') when we use dot2tex? (I.e. that would need modifying dot2tex code.)

[nthiery]

Replying to ncohen:

I have proposed two strategies for this in 2. Which one do you prefer. If we go for the first one, what's the right way for storing such a ploting option in the graph?

Storing a plotting option in the graph? Why on earth would you do that? O_o

So that when you create a hasse diagram, and later plot it, it remembers to plot it going up. But I can live without this feature.

Why can't you just make the default be to plot the top element on top, and only do something different if some guy calls my_poset.show(mess_up_top_and_bottom=True) ?

That is not the question. The goal is to always plot the poset with top on top.

But to plot a poset we need to plot a digraph. So either it's ok if the acyclic layout of graphs always points up. Or we need to have an option (of the graph, or just of Graph.plot) to specify whether the acyclic layout goes up or down.

[ncohen]

But to plot a poset we need to plot a digraph. So either it's ok if the acyclic layout of graphs always points up.

It is the default behaviour in Sage. If it is messed up when dot2tex gets installed then let *this* be fixed, and let us move on.

They chose that an arrow a->b meant that "a is larger than b" and Posets do the opposite. I do not think that it is worth more consideration than that.

Nathann

[nthiery]

New commits:

​502e621

16865: implement rankdir for Digraph.layout_acyclic, enforce a default value of 'up' for all implementations, and use it for posets

[ncohen]

With your code, the value of 'rankdir' is dropped without a warning if have_dot2tex is False.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​22cd536

16865: pass down rankdir from layout_acyclic to layout_acyclic_dummy + fixed tests

[nthiery]

Replying to ncohen:

With your code, the value of 'rankdir' is dropped without a warning if have_dot2tex is False.

Oops, yes; and the tests in layout_acyclic were using in fact layout_acyclic_dummy ... Fixed. Luckily that would have been caught by the patchbot. But thanks for the early feedback!

[nthiery]

Why needs work?

[ncohen]

Sorry, I had not noticed that you had added a commit. Since you apparently "added a parameter" in 'graphviz_string' could you document it? If you do not want to document the same parameter in different places only because it is forwarded to some function later, could you say so explicitly in the docstring so that people can actually figure out that this 'rankdir' parameter exists?

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​95a223b

16865: documentation of the rankdir option for graphviz_string

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​08e9b8e

16865: documentation of the rankdir option for graphviz_string: proofreading

[nthiery]

Replying to ncohen:

Since you apparently "added a parameter" in 'graphviz_string' could you document it?

Ah, yes, thanks for catching this. Just an oversight. Given that the default is not the same as for acyclic_layout we really want to document this option here. I used the occasion to clean a bit the beginning of this docstring.

---

New commits:

​08e9b8e

16865: documentation of the rankdir option for graphviz_string: proofreading

---

New commits:

​08e9b8e

16865: documentation of the rankdir option for graphviz_string: proofreading

[ncohen]

Jori? Could you try this patch? I do not have dot2tex installed on my machine, and these things cannot be uninstalled.

Nathann

[jmantysalo]

Replying to ncohen:

Jori? Could you try this patch? I do not have dot2tex installed on my machine

Just tested quickly. Seems to work, but it takes a little longer to really test this.

and these things cannot be uninstalled.

That could be a reason for another ticket...

[ncohen]

Just tested quickly. Seems to work, but it takes a little longer to really test this.

Well, please change the ticket's status when you are done, then.

That could be a reason for another ticket...

It is not a problem with 'dot2tex' only.

Nathann

[jmantysalo]

I tested with some random posets and with empty and 1-element poset. Seems to work.

But still I don't know Sage internals enought to be able to review this. Sorry for that.

[ncohen]

I tested with some random posets and with empty and 1-element poset. Seems to work.

But still I don't know Sage internals enought to be able to review this. Sorry for that.

I asked you to test it because I cannot. I read the code, and I think that it can pass. If you are satisfied with the drawings (try all options) please change the ticket's status and add your name to the list of reviewers.

Nathann

[jmantysalo]

Replying to ncohen:

If you are satisfied with the drawings (try all options) please change the ticket's status and add your name to the list of reviewers.

Tested. (Not all combinations, thought.) I think that this one is ready. For a note: layout='tree' reverses the poset. But I doubt anyone would use it for posets.

[vbraun]

Author name missing