Opened 7 years ago

Closed 7 years ago

#12872 closed enhancement (fixed)

A show method for permutations

Reported by: ncohen Owned by: sage-combinat
Priority: major Milestone: sage-5.1
Component: combinatorics Keywords:
Cc: wdj, fhivert, sage-combinat Merged in: sage-5.1.beta0
Authors: Nathann Cohen Reviewers: David Coudert
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by ncohen)

Following the discussion that took place there [1], this patch adds a .show() method to the Permutation class, to obtain drawings like the own we can see there [2].

Nathann

[1] https://groups.google.com/d/topic/sage-combinat-devel/vdfE7iaJTxs/discussion

[2] http://upload.wikimedia.org/wikipedia/en/7/78/Permutation_graph.svg

Attachments (8)

swap-diagram-nathann.png (57.6 KB) - added by wdj 7 years ago.
reoriented diagram plot
swap-diagram-sage.png (33.5 KB) - added by wdj 7 years ago.
bipartite example plot (same permutation as other plot)
tmp_1.png (50.2 KB) - added by ncohen 7 years ago.
1.png (24.9 KB) - added by ncohen 7 years ago.
2.png (54.3 KB) - added by ncohen 7 years ago.
3.png (90.1 KB) - added by ncohen 7 years ago.
4.png (75.4 KB) - added by ncohen 7 years ago.
trac_12872.patch (5.8 KB) - added by ncohen 7 years ago.

Download all attachments as: .zip

Change History (36)

comment:1 Changed 7 years ago by ncohen

  • Description modified (diff)

comment:2 Changed 7 years ago by ncohen

  • Cc wdj fhivert added
  • Status changed from new to needs_review

Now needs a review ! And comments too, if you do not agree with the patch :-)

Nathann

comment:3 Changed 7 years ago by ncohen

  • Description modified (diff)

comment:4 follow-up: Changed 7 years ago by wdj

Testing now, but some comments:

(1) I don't understand the line 1166 (inversion_list = []). Am I missing something?

(2) In the examples, there is only one:-) What do you think of the following idea: In the examples, compare your graph with what you get using show BipartiteGraph?, as below? Note the difference in the orientation of the graph. (I prefer the orientation in the bipartite graph plot personally.) Just food for thought. I'm attaching two plots, one is your plot, re-oriented to match the graph plot, and the other is the graph plot.

sage: p = Permutations(10).random_element(); p
[5, 10, 4, 8, 3, 1, 9, 6, 2, 7]
sage: E = [(i, 10+p(i)) for i in range(1,11)] 
sage: G = BipartiteGraph(E); G.show()         
sage: p.show()
sage: G = BipartiteGraph(E); G.show()
sage: p.show()     

Changed 7 years ago by wdj

reoriented diagram plot

Changed 7 years ago by wdj

bipartite example plot (same permutation as other plot)

comment:5 in reply to: ↑ 4 Changed 7 years ago by ncohen

(1) I don't understand the line 1166 (inversion_list = []). Am I missing something?

*Sigh*

No, you are right :-)

(2) In the examples, there is only one:-) What do you think of the following idea:

Right ! What do you think of this patch ? :-)

Nathann

comment:6 follow-up: Changed 7 years ago by ncohen

Here it is !

comment:7 in reply to: ↑ 6 Changed 7 years ago by wdj

Replying to ncohen:

Here it is !

Thanks very much! However, the labels (0-9) do not match the permutation set (1-10). Shouldn't they match?

comment:8 Changed 7 years ago by ncohen

Well, for me the problems is that permutations do not match the natural choice, which is 0-9 :-P

But you are right of course, and it is now fixed.

Nathann

comment:9 Changed 7 years ago by hivert

Hi Nathann,

Your drawing are very cool ! I'm not completely sure this is what I expected by typing p.show(). Maybe, it's worth asking on sage-combinat-devel for a better name.

Florent

comment:10 Changed 7 years ago by hivert

  • Cc sage-combinat added

comment:11 Changed 7 years ago by ncohen

Well, I am not sure either and perhaps you have in mind another representation that would fit well in Permutation.show. What I know though, is that the drawing we have now would *NEVER* be found by an interested user unless it can be reached by Permutation.show (I do not even know if this drawing has a name, Jason says that the name he uses is non-standard, ...), and so the best for me is to have some arguments in Permutation.show() that would yield different kinds of drawings.

Do we get this patch inside of Sage while we wait for another type of drawing to be added ? I'm writing to sage-combinat-devel in the meantime !

Nathann

Changed 7 years ago by ncohen

Changed 7 years ago by ncohen

Changed 7 years ago by ncohen

Changed 7 years ago by ncohen

Changed 7 years ago by ncohen

comment:12 Changed 7 years ago by wdj

This last patch passed all tests on 5.0.b10. I like the drawings now:-)

comment:13 follow-up: Changed 7 years ago by dcoudert

Hello,

I'm able to install the patch on 5.0.beta14, but shouldn't you had some tests to the function?

Also:

  • paralell -> parallel
  • why are you using "chord-diagram" instead of "circular"?
  • You could use capitalize() in tests to ease usage.

comment:14 in reply to: ↑ 13 Changed 7 years ago by ncohen

Hellooooooooo !!!

but shouldn't you had some tests to the function?

Well, there are 4 ! Of course the image are not "checked", but the code is run anyway and nothing crashes.. That's more or less all we can do with the docstring system :-/

  • paralell -> parallel

Arggggggggggggggggggggg !!!

  • why are you using "chord-diagram" instead of "circular"?

Oh. Well, because it is the word that the guys from Sage-combinat picked the most naturally. I like permutations but that's their stuff much more than mine, so that they hold for True is true for me :-D

  • You could use capitalize() in tests to ease usage.

Hmmmm. Well, I think that it had some meaning to do this for our LP solvers because we never know which letters should be upper case and which one are not (Gurobi or GUROBI ? Cplex or CPLEX), but let's not add a layer of administration to *all* the functions that take a parameter like that. I added a "else" statement though, so that there is an exception raised if the given string does not match any of the good ones. These arguments are always lower-case anyway.

Patch updated !

Nathann

comment:15 Changed 7 years ago by dcoudert

OK (install, tests, docbuild, etc.), so it remains to wait for answers from sage-combinat.

comment:16 Changed 7 years ago by ncohen

? To what do you expect answers ?

comment:17 Changed 7 years ago by dcoudert

It's written above: "Do we get this patch inside of Sage while we wait for another type of drawing to be added ? I'm writing to sage-combinat-devel in the meantime ! "

So what's the feedback?

comment:18 Changed 7 years ago by ncohen

Oh, sorry. Of course when you re-upload a patch all the previous times when you uploaded it disappear. Actually, in this patch there was at first only the "braid" drawing, and people on sage-combinat said that the most natural was to plot th disjoint union of circuits, and also the chord-diagram. So I updated my patch like that and the default drawing is now the circuit thing, with the braid only as an option. The braid was the only one implemented at first, and the only one I was personally interested in having :-)

Nathann

comment:19 Changed 7 years ago by dcoudert

  • Reviewers set to David Coudert

I was about to give positive review when I saw line 1169 "orizontally". Please add an 'h' ;-)

comment:20 Changed 7 years ago by ncohen

Done !

comment:21 Changed 7 years ago by dcoudert

No!

comment:22 Changed 7 years ago by ncohen

Right... qrefresh :-P

Changed 7 years ago by ncohen

comment:23 Changed 7 years ago by dcoudert

  • Status changed from needs_review to positive_review

Nice. For me the patch is OK, so I give positive review. D.

comment:24 Changed 7 years ago by ncohen

Thaaaaaaaaaaanks !!!

Nathann

comment:25 Changed 7 years ago by jdemeyer

  • Authors set to Nathann Cohen

comment:26 follow-up: Changed 7 years ago by stumpc5

Hi Natann,

thanks for working on that! Here are some further comments:

  1. for the braid representation: I would think of options for orientation being X-to-Y for X,Y being top,bottom or bottom,top or left-right or right-left. Also, I think of a braid in a different way: order the inversions in some order (say lex) and then swap two strings at a time. This produces pictures like on the wiki page for the braid group, http://en.wikipedia.org/wiki/Braid_group.
  1. A second way of drawing the chord diagram is by having two vertices (i,"in") and (i,"out") for each letter. The pic then becomes a matching of 2n points placed on a circle.
  1. do you think of implementing the Rothe (or inversion) diagram Florent mentioned?
  1. The one-line notation is also nice when just drawing arcs between points.

Maybe at least points 3 and 4 might only be worth implementing when doing it in latex/tikz - but this might also apply to 1.

Feel free to say that you don't want to do it, I might then start looking at it one day...

Best, Christian

comment:27 in reply to: ↑ 26 Changed 7 years ago by ncohen

Hellooooooo Christian !!

  1. for the braid representation: I would think of options for orientation being X-to-Y for X,Y being top,bottom or bottom,top or left-right or right-left. Also, I think of a braid in a different way: order the inversions in some order (say lex) and then swap two strings at a time. This produces pictures like on the wiki page for the braid group, http://en.wikipedia.org/wiki/Braid_group.

You mean the kind of drawings produced by my patch on pseudolines (#12090) ? :-P

I will not code that twice, but that feature of pseudolines can be exposed in the .show method of course !

  1. A second way of drawing the chord diagram is by having two vertices (i,"in") and (i,"out") for each letter. The pic then becomes a matching of 2n points placed on a circle.
  1. do you think of implementing the Rothe (or inversion) diagram Florent mentioned?
  1. The one-line notation is also nice when just drawing arcs between points.

Actually I only needed to plot permutations as done by the initial patch, that is the algorithm="braid" version of the current patch. I implemented others because it did not seem to be the universal way to draw them but besides that I will stay as clear from graphical codes as I can :-P

Have fuuuuuuuuuuuuuuuuuuuuuuuuuuuun !!!

Nathann

comment:28 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.1.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.