Ticket #6747 (closed defect: fixed)

Opened 7 months ago

Last modified 5 months ago

Improve plotting of trees

Reported by: boothby Owned by: boothby
Priority: minor Milestone: sage-4.2.1
Component: graph theory Keywords:
Cc: sage-combinat, jason, myurko Author(s): Tom Boothby
Report Upstream: Reviewer(s): Nathann Cohen, Michael Yurko
Merged in: sage-4.2.1.alpha0 Work issues:

Description

When one plots a tree with G.plot(layout='tree'), the result is ugly. For one thing, a tree plot should never have crossing lines. I have code that makes a nice tree plot in nearly-linear time, which should be included in Sage.

Attachments

tree2.png Download (32.9 KB) - added by boothby 7 months ago.
old plot
tree.png Download (12.8 KB) - added by boothby 7 months ago.
desired output
6747_tree_layout.patch Download (5.2 KB) - added by boothby 7 months ago.
trac_6747-reviewer.patch Download (5.7 KB) - added by ncohen 5 months ago.

Change History

Changed 7 months ago by boothby

old plot

Changed 7 months ago by boothby

desired output

  Changed 7 months ago by ncohen

You code looks like it first chooses the center vertex in the graph, then converts is as a Poset by picking this special vertex as a root, and plotting this Poset... But you did not post the patch :-)

Changed 7 months ago by boothby

  Changed 7 months ago by boothby

  • summary changed from Improve plotting of trees to [with patch, needs review] Improve plotting of trees

Uhm... my code does nothing like what you said. And I didn't post the patch because I hadn't cleaned it up / incorporated it into the GraphPlot? class / documented it. But now I have!

  Changed 7 months ago by AlexGhitza

  • cc sage-combinat added
  • milestone set to sage-4.1.2

  Changed 7 months ago by ncohen

  • summary changed from [with patch, needs review] Improve plotting of trees to [with patch, needs work] Improve plotting of trees

I just tried your patch, and as I never plotted any tree in Sage, I tried to generate some for a start.... I tried your algorithm on graphs.BalancedTree?(3,4) and graphs.BalancedTree?(3,5). They are displayed on my machine in a very odd shape ( the picture had a large width and a very small height ). I admit these graphs contain a lot of vertices and may not be good examples, but what do you think of this result ? Do you think there would be a way to slightly change you code so that in such cases the plot could have a look closer from what one gets with the current layout=tree parameter ?

I know it is very easy to criticize in such cases, and much harder to come up with a good algorithm.. Thinking about it, I thought that maybe the best way to draw a tree properly would be to begin with a cross-free embedding of the tree in the plane ( as you mentionned ), then to use the usual trick of repulsion between vertices to obtain a balanced shape ? I know it is not a good solution because of its complexity. I'll continue thinking about it, though O_o

  Changed 7 months ago by boothby

These are extreme examples, and I'm not sure that a top-down layout will ever look good. I believe that the solution for this particular problem would be to implement a circular tree layout algorithm. However, if you play with the parameters, you can get a fairly nice plot.

    T = graphs.BalancedTree(3,5)
    P = T.plot(layout='tree',tree_root=0)
    P.show(aspect_ratio=.1,figsize=(15,15))

  Changed 6 months ago by jason

  • cc jason added

follow-up: ↓ 12   Changed 5 months ago by ncohen

Applies fines, and I could not find any bug in it. Clearly improves the plotting for "human" trees. This new patch includes the previous one, plus :

  • A INPUT section
  • A new semicolumn after EXAMPLES ::

If you agree with these changes, you can set this ticket to positive review !

Nathann

Changed 5 months ago by ncohen

  Changed 5 months ago by nthiery

Thanks much for improving the layout of trees!

Please have a look at #7004, which also started splitting the layout algorithms in different methods. So we need to decide on a naming convention.

Then I guess this patch can get in, and #7400 be rebased on it.

  Changed 5 months ago by ncohen

  • status changed from needs_work to needs_review
  • summary changed from [with patch, needs work] Improve plotting of trees to [with patch, needs review] Improve plotting of trees

  Changed 5 months ago by ncohen

  • summary changed from [with patch, needs review] Improve plotting of trees to Improve plotting of trees

  Changed 5 months ago by myurko

  • cc myurko added

in reply to: ↑ 7   Changed 5 months ago by myurko

  • status changed from needs_review to positive_review

Replying to ncohen:

Applies fines, and I could not find any bug in it. Clearly improves the plotting for "human" trees. This new patch includes the previous one, plus : * A INPUT section * A new semicolumn after EXAMPLES :: If you agree with these changes, you can set this ticket to positive review ! Nathann

The change look fine.

  Changed 5 months ago by mhansen

  • status changed from positive_review to closed
  • reviewer set to Nathann Cohen, Michael Yurko
  • resolution set to fixed
  • merged set to sage-4.2.1.alpha0
  • author set to Tom Boothby
Note: See TracTickets for help on using tickets.