Opened 7 years ago
Closed 7 years ago
#16449 closed enhancement (fixed)
Add plot() and show() methods to abstract Matroid class
Reported by:  Jayant  Owned by:  

Priority:  minor  Milestone:  sage6.3 
Component:  matroid theory  Keywords:  visualization, geometric representation 
Cc:  Stefan, Rudi  Merged in:  
Authors:  Jayant Apte  Reviewers:  Stefan van Zwam 
Report Upstream:  N/A  Work issues:  
Branch:  7f9a4a7 (Commits, GitHub, GitLab)  Commit:  7f9a4a71218515e90a17096b4f8163bfc6e135c9 
Dependencies:  Stopgaps: 
Description
plot() method will return a sage graphics object corresponding to the geometric representation of rank <=3 matroid. show() method will be analogous to that of graphics objects.
Change History (34)
comment:1 Changed 7 years ago by
 Branch set to u/Jayant/ticket/16449
comment:2 Changed 7 years ago by
 Commit set to de4ef370dd307abae62f0a123e3592c2d637a722
comment:3 Changed 7 years ago by
 Cc Stefan Rudi added
comment:4 Changed 7 years ago by
 Commit changed from de4ef370dd307abae62f0a123e3592c2d637a722 to d1b48a8e5d609b073d54fd7e7b33c5f10a65be68
Branch pushed to git repo; I updated commit sha1. New commits:
d1b48a8  Committed changed matroid.pyx and matroid.pxd which were left out of previous

comment:5 Changed 7 years ago by
 Commit changed from d1b48a8e5d609b073d54fd7e7b33c5f10a65be68 to bc38a21f7110493c9854913f76fde880505eb850
Branch pushed to git repo; I updated commit sha1. New commits:
bc38a21  Added documentation to plot() and show() methods of the abstract matroids class.

comment:6 Changed 7 years ago by
 Commit changed from bc38a21f7110493c9854913f76fde880505eb850 to 228cdac7b0062378ab8bdd43161c5d061a110e05
Branch pushed to git repo; I updated commit sha1. New commits:
228cdac  Added examples to matroids_plot_helpers.py

comment:7 Changed 7 years ago by
 Commit changed from 228cdac7b0062378ab8bdd43161c5d061a110e05 to 51d676452a559595376aef69ccac2fe7ea0534ea
Branch pushed to git repo; I updated commit sha1. New commits:
51d6764  Added caching of coordinates.

comment:8 Changed 7 years ago by
 Commit changed from 51d676452a559595376aef69ccac2fe7ea0534ea to 09b2029d19c35c2960d2102e9bdffc4e10df4f9e
Branch pushed to git repo; I updated commit sha1. New commits:
09b2029  Added ability for user to specify point positions, added option/placeholder for point placement algorithm.

comment:9 Changed 7 years ago by
 Commit changed from 09b2029d19c35c2960d2102e9bdffc4e10df4f9e to 1611e8d06e5bb727068ec18be3d9e19a6d1c314b
Branch pushed to git repo; I updated commit sha1. New commits:
1611e8d  100% docstring coverage and minor documentation fixes.

comment:10 Changed 7 years ago by
 Status changed from new to needs_review
comment:11 Changed 7 years ago by
 Commit changed from 1611e8d06e5bb727068ec18be3d9e19a6d1c314b to 1a26f595814e3b7ef5b01c350858d78fd3dcf56b
Branch pushed to git repo; I updated commit sha1. New commits:
1a26f59  Fixed some spelling mistakes in documentation.

comment:12 followup: ↓ 17 Changed 7 years ago by
Hi Jayant,
The user positioning works as advertised. Very nice!
I just have some minor issues left. Look at the result of executing this code:
M=matroids.named_matroids.BetsyRoss() pos={} s="abcde" t="fghij" x=1.61 y=1/1.61 for i in range(5): pos[s[i]]=(RR(x*sin(2*pi*i/5)), RR(x*cos(2*pi*i/5))) pos[t[i]]=(RR(y*sin(2*pi*(i+1/2)/5)), RR(y*cos(2*pi*(i+1/2)/5))) pos['k']=(0,0) M.show(pos_method=1, pos_dict=pos)
This gives a starlike object. The input is clearly centered at (0,0) and has rotational symmetry.
 The output of show() is flattenened (on my screen) and there is some empty space above the image.
 If you show the output generated by plot(), then there is no flattening, but the some of the dots representing the groundset elements are cut by the boundary of the image.
So I wonder, could you perhaps get rid of the flattening in show()? The behaviour of the boundary is another thing. The minimal solution is to simply state the position of the windowframe in the docstring, and leave it to the user to accomodate. A more advanced solution is autoframing the given picture, but then you should somehow make sure that all the dots and curves fall completely within the frame.
Finally, one more request. I see that the point positioning of M in the above example is cached, so that if I just do M.show() again I get the same nice picture. Could you create a command M._fix_positions(pos_dict) which will likewise fix the point positions, but without showing or plotting the matroid? I guess I could call M.plot() with a user point positioning once and ignore the output to that effect, but I'm fairly sure you can get the same result without calling all the plot routines. I want to fit the named_matroids of rank <=3 with a nice default point positioning, so I could use a command like that.
comment:13 followup: ↓ 20 Changed 7 years ago by
 Status changed from needs_review to needs_work
Some more pedantic comments:
1) There are some formatting issues: try to wrap lines at 78 characters, and get rid of trailing whitespace. You may want to use a pep8 checking tool. 2) Some doctests fail. You can run them by writing
./sage t src/sage/matroids/matroids_plot_helpers.py
from the base Sage directory.
I'll testdrive the code later this morning.
comment:14 Changed 7 years ago by
More issues: "make doc" gives the following errors:
[matroids ] building [inventory]: targets for 15 source files that are out of date [matroids ] updating environment: [config changed] 15 added, 0 changed, 0 removed [matroids ] reading sources... [ 6%] index [matroids ] reading sources... [ 13%] sage/matroids/advanced [matroids ] reading sources... [ 20%] sage/matroids/basis_exchange_matroid [matroids ] reading sources... [ 26%] sage/matroids/basis_matroid [matroids ] reading sources... [ 33%] sage/matroids/catalog [matroids ] reading sources... [ 40%] sage/matroids/circuit_closures_matroid [matroids ] reading sources... [ 46%] sage/matroids/constructor [matroids ] reading sources... [ 53%] sage/matroids/dual_matroid [matroids ] reading sources... [ 60%] sage/matroids/extension [matroids ] reading sources... [ 66%] sage/matroids/linear_matroid [matroids ] reading sources... [ 73%] sage/matroids/matroid [matroids ] reading sources... [ 80%] sage/matroids/matroids_catalog [matroids ] reading sources... [ 86%] sage/matroids/minor_matroid [matroids ] reading sources... [ 93%] sage/matroids/rank_matroid [matroids ] reading sources... [100%] sage/matroids/utilities [matroids ] docstring of sage.matroids.matroid:3: WARNING: Bullet list ends without a blank line; unexpected unindent. [matroids ] docstring of sage.matroids.matroid:8: ERROR: Unexpected indentation. [matroids ] docstring of sage.matroids.matroid:11: WARNING: Block quote ends without a blank line; unexpected unindent. [matroids ] docstring of sage.matroids.matroid:13: WARNING: Bullet list ends without a blank line; unexpected unindent. [matroids ] docstring of sage.matroids.matroid:3: WARNING: Bullet list ends without a blank line; unexpected unindent. [matroids ] docstring of sage.matroids.matroid:8: ERROR: Unexpected indentation. [matroids ] docstring of sage.matroids.matroid:11: WARNING: Block quote ends without a blank line; unexpected unindent. [matroids ] docstring of sage.matroids.matroid:13: WARNING: Bullet list ends without a blank line; unexpected unindent. Error building the documentation. Note: incremental documentation builds sometimes cause spurious error messages. To be certain that these are real errors, run "make docclean" first and try again. Traceback (most recent call last): File "/Users/stefan/sage/src/doc/common/builder.py", line 1477, in <module> getattr(get_builder(name), type)() File "/Users/stefan/sage/src/doc/common/builder.py", line 276, in _wrapper getattr(get_builder(document), 'inventory')(*args, **kwds) File "/Users/stefan/sage/src/doc/common/builder.py", line 487, in _wrapper x.get(99999) File "/Users/stefan/sage/local/lib/python/multiprocessing/pool.py", line 554, in get raise self._value OSError: [matroids ] docstring of sage.matroids.matroid:3: WARNING: Bullet list ends without a blank line; unexpected unindent. make: *** [dochtml] Error 1
These are hard to track down, since line numbers are relative to the comment block, and it doesn't tell you which block we're in. But it's safe to assume these occur in your docstrings.
comment:15 Changed 7 years ago by
The white text labels are an unfortunate choice, as illustrated here:
M1 = Matroid(groundset=['Carolyn', 'Mike', 'Rudi', 'Stefan', 'Jayant', 'Dillon', 'Hassler'], field=GF(2), reduced_matrix=[[1,1,0,1],[1,0,1,1],[0,1,1,1]]) M2 = Matroid(groundset=range(200,207), field=GF(3), reduced_matrix=[[1,1,0,0],[1,0,1,0],[0,1,1,0]]) M1.show() M2.show()
The following results in an error:
M3 = matroids.Uniform(1,6) M3.show()
Traceback (most recent call last): File "<stdin>", line 1, in <module> File "_sage_input_19.py", line 10, in <module> exec compile(u'open("___code___.py","w").write("# * coding: utf8 *\\n" + _support_.preparse_worksheet_cell(base64.b64decode("TTMgPSBtYXRyb2lkcy5Vbmlmb3JtKDEsNikKTTMuc2hvdygp"),globals())+"\\n"); execfile(os.path.abspath("___code___.py")) File "", line 1, in <module> File "/private/var/folders/8p/gpt0bd4x5vg46bg2j4l0mnfr0000gn/T/tmppZYy7q/___code___.py", line 4, in <module> exec compile(u'M3.show() File "", line 1, in <module> File "matroid.pyx", line 4812, in sage.matroids.matroid.Matroid.show (sage/matroids/matroid.c:32349) File "matroid.pyx", line 4850, in sage.matroids.matroid.Matroid.show (sage/matroids/matroid.c:32173) File "matroid.pyx", line 4810, in sage.matroids.matroid.Matroid.plot (sage/matroids/matroid.c:31770) File "/Users/stefan/sage/local/lib/python2.7/sitepackages/sage/matroids/matroids_plot_helpers.py", line 670, in geomrep G = addlp(M1,M,L,P,pts2,G) File "/Users/stefan/sage/local/lib/python2.7/sitepackages/sage/matroids/matroids_plot_helpers.py", line 421, in addlp gnd = sorted(M1.groundset_list()) AttributeError: 'MinorMatroid' object has no attribute 'groundset_list'
This should result in a "NotImplementedError?", but doesn't do anything:
P8 = matroids.named_matroids.P8() P8.show()
comment:16 Changed 7 years ago by
 Commit changed from 1a26f595814e3b7ef5b01c350858d78fd3dcf56b to 3d92382b4f7e2a6131503f77bfee41c5a6107c69
comment:17 in reply to: ↑ 12 Changed 7 years ago by
Replying to Rudi: Added tracking of xy limits which in a sense, does autoframing that you mentioned. It keeps track of min and max x and y points whenever new point or line (set of points) is added. Leaves uniform space (Currently 0.5) on all sides. The _fix_positions() method is also in place now.
Hi Jayant,
The user positioning works as advertised. Very nice!
I just have some minor issues left. Look at the result of executing this code:
M=matroids.named_matroids.BetsyRoss() pos={} s="abcde" t="fghij" x=1.61 y=1/1.61 for i in range(5): pos[s[i]]=(RR(x*sin(2*pi*i/5)), RR(x*cos(2*pi*i/5))) pos[t[i]]=(RR(y*sin(2*pi*(i+1/2)/5)), RR(y*cos(2*pi*(i+1/2)/5))) pos['k']=(0,0) M.show(pos_method=1, pos_dict=pos)This gives a starlike object. The input is clearly centered at (0,0) and has rotational symmetry.
 The output of show() is flattenened (on my screen) and there is some empty space above the image.
 If you show the output generated by plot(), then there is no flattening, but the some of the dots representing the groundset elements are cut by the boundary of the image.
So I wonder, could you perhaps get rid of the flattening in show()? The behaviour of the boundary is another thing. The minimal solution is to simply state the position of the windowframe in the docstring, and leave it to the user to accomodate. A more advanced solution is autoframing the given picture, but then you should somehow make sure that all the dots and curves fall completely within the frame.
Finally, one more request. I see that the point positioning of M in the above example is cached, so that if I just do M.show() again I get the same nice picture. Could you create a command M._fix_positions(pos_dict) which will likewise fix the point positions, but without showing or plotting the matroid? I guess I could call M.plot() with a user point positioning once and ignore the output to that effect, but I'm fairly sure you can get the same result without calling all the plot routines. I want to fit the named_matroids of rank <=3 with a nice default point positioning, so I could use a command like that.
comment:18 Changed 7 years ago by
In order to keep the _cached_info from becoming a mess, I suggest naming the keys "plot_positions" and "plot_lineorders".
When typing
M = matroids.named_matroids.NonFano() M.show? <tab key>
in a notebook, you can see some formatting issues (possibly related to the docbuild errors I mentioned above). Also, the "limits" option is undocumented. It's ok, by the way, to cross reference the "plot" method from the "show" method, instead of copying and pasting all documentation.
comment:19 Changed 7 years ago by
 Commit changed from 3d92382b4f7e2a6131503f77bfee41c5a6107c69 to 4f20c9542bac14a6e4e193e5195c5e8b6142bc14
comment:20 in reply to: ↑ 13 Changed 7 years ago by
Replying to Stefan: Just finished using pep8. The file matroids_plot_helpers.py is now pep8 compliant. Some doctests are still failing. These are the bugs that probably came in when I added the limits tracking feature. [72 tests, 9 failures, 1.32 s]
Some more pedantic comments:
1) There are some formatting issues: try to wrap lines at 78 characters, and get rid of trailing whitespace. You may want to use a pep8 checking tool. 2) Some doctests fail. You can run them by writing
./sage t src/sage/matroids/matroids_plot_helpers.py
from the base Sage directory.
I'll testdrive the code later this morning.
comment:21 Changed 7 years ago by
 Branch changed from u/Jayant/ticket/16449 to u/Stefan/ticket/16449
comment:22 Changed 7 years ago by
 Commit changed from 4f20c9542bac14a6e4e193e5195c5e8b6142bc14 to d280e33dc53ee8bb3ef2be0ca13a6ec6be79ff7d
I added a few fixes to documentation, whitespace, and doctests. The documentation build still seems to give errors, though I didn't do a full build:
[matroids ] docstring of sage.matroids.matroid:3: WARNING: Bullet list ends without a blank line; unexpected unindent. [matroids ] docstring of sage.matroids.matroid:8: ERROR: Unexpected indentation. [matroids ] docstring of sage.matroids.matroid:11: WARNING: Block quote ends without a blank line; unexpected unindent. [matroids ] docstring of sage.matroids.matroid:13: WARNING: Bullet list ends without a blank line; unexpected unindent. Error building the documentation.
New commits:
d280e33  Documentation and doctest fixes

comment:23 Changed 7 years ago by
 Commit changed from d280e33dc53ee8bb3ef2be0ca13a6ec6be79ff7d to fcbbef2ab0053f186c4303feec27bec138dde60f
Branch pushed to git repo; I updated commit sha1. New commits:
fcbbef2  Fixed docbuild errors

comment:24 Changed 7 years ago by
 Branch changed from u/Stefan/ticket/16449 to u/Jayant/ticket/16449
 Commit changed from fcbbef2ab0053f186c4303feec27bec138dde60f to f406d2e45dde540c5e75837660a18d9bdf076944
New commits:
f406d2e  Fixed bugs reported by Stefan in comment 15 and 18.

comment:25 followup: ↓ 26 Changed 7 years ago by
Is this ready for review?
comment:26 in reply to: ↑ 25 Changed 7 years ago by
Replying to Stefan: Yeah, I think so. I did doctest and make doc
and they didn't give any errors. The points are now gray with black labels. _cached_info
dictionary labels are changed. Rank>3 gives not implemented error. matroids.Uniform(1,6).show()
is working. lims
option of show()
has documentation.
Is this ready for review?
comment:27 followup: ↓ 28 Changed 7 years ago by
 Status changed from needs_work to needs_review
Ok. Next time change the ticket status to "review"
comment:28 in reply to: ↑ 27 Changed 7 years ago by
Replying to Stefan: Oops. Sorry about that.
Ok. Next time change the ticket status to "review"
comment:29 Changed 7 years ago by
 Status changed from needs_review to needs_work
There are still issues with parallel elements and lowrank matroids.
This is a good example:
M = matroids.PG(3,2).contract(0) M.show() M.simplify().show()
Also, with the same example, specifying a basis goes wrong:
M.show(B=[5,6,7])
gives
Traceback (most recent call last): File "<stdin>", line 1, in <module> File "_sage_input_24.py", line 10, in <module> exec compile(u'open("___code___.py","w").write("# * coding: utf8 *\\n" + _support_.preparse_worksheet_cell(base64.b64decode("TS5zaG93KEI9WzUsNiw3XSk="),globals())+"\\n"); execfile(os.path.abspath("___code___.py")) File "", line 1, in <module> File "/private/var/folders/8p/gpt0bd4x5vg46bg2j4l0mnfr0000gn/T/tmpBK25kT/___code___.py", line 3, in <module> exec compile(u'M.show(B=[_sage_const_5 ,_sage_const_6 ,_sage_const_7 ]) File "", line 1, in <module> File "matroid.pyx", line 4958, in sage.matroids.matroid.Matroid.show (build/cythonized/sage/matroids/matroid.c:33380) File "matroid.pyx", line 4999, in sage.matroids.matroid.Matroid.show (build/cythonized/sage/matroids/matroid.c:33147) File "matroid.pyx", line 4956, in sage.matroids.matroid.Matroid.plot (build/cythonized/sage/matroids/matroid.c:32728) File "/Users/stefan/sage/local/lib/python2.7/sitepackages/sage/matroids/matroids_plot_helpers.py", line 763, in geomrep [M, L, P] = slp(M1, pos_dict=pd, B=B1) File "/Users/stefan/sage/local/lib/python2.7/sitepackages/sage/matroids/matroids_plot_helpers.py", line 445, in slp newP.extend(list(pcl  set([pcl[0]]))) TypeError: 'frozenset' object does not support indexing
Then there's
M = matroids.Uniform(0,4) M.show()
giving
Traceback (most recent call last): File "<stdin>", line 1, in <module> File "_sage_input_25.py", line 10, in <module> exec compile(u'open("___code___.py","w").write("# * coding: utf8 *\\n" + _support_.preparse_worksheet_cell(base64.b64decode("TSA9IG1hdHJvaWRzLlVuaWZvcm0oMCw0KQpNLnNob3coKQ=="),globals())+"\\n"); execfile(os.path.abspath("___code___.py")) File "", line 1, in <module> File "/private/var/folders/8p/gpt0bd4x5vg46bg2j4l0mnfr0000gn/T/tmp1peeRw/___code___.py", line 4, in <module> exec compile(u'M.show() File "", line 1, in <module> File "matroid.pyx", line 4958, in sage.matroids.matroid.Matroid.show (build/cythonized/sage/matroids/matroid.c:33380) File "matroid.pyx", line 4999, in sage.matroids.matroid.Matroid.show (build/cythonized/sage/matroids/matroid.c:33147) File "matroid.pyx", line 4956, in sage.matroids.matroid.Matroid.plot (build/cythonized/sage/matroids/matroid.c:32728) File "/Users/stefan/sage/local/lib/python2.7/sitepackages/sage/matroids/matroids_plot_helpers.py", line 830, in geomrep list(set(L)  set(P))) File "/Users/stefan/sage/local/lib/python2.7/sitepackages/sage/matroids/matroids_plot_helpers.py", line 157, in it lines.append([B1[pairs[i1][0]]]) IndexError: list index out of range
I'm not so sure about the output of
M3 = matroids.Uniform(1,6) M3.show()
What does the set mean? Why is it so far removed from the point? Putting *all* elements from the parallel class into the set is clearer. And leave the point empty, or give it a new label (use the sage.matroids.utilities.newlabel function) and write on the sidelines
n = {0,1,2,3,4,5}
(where n is the new label).
Finally, I'd go for a lighter gray for the points.
Please test thoroughly with lots of examples, especially degenerate ones, before setting this to "needs_review" again.
comment:30 Changed 7 years ago by
 Commit changed from f406d2e45dde540c5e75837660a18d9bdf076944 to cd80f02c858cad7d4136a75dc4473006c58bf0f0
Branch pushed to git repo; I updated commit sha1. New commits:
cd80f02  Fixed parallel element bug. Modified parallel element placement. Changed point color to lighter gray. Added NotImplementedError for rank 0 matroids.

comment:31 Changed 7 years ago by
Sorry, more nitpicking...
matroids.PG(3,2).contract(0).contract(1).show()
You need to make sure the "newlabel" method gives a unique new label for each parallel class. Also, you can (and should) plot rank0 matroids: they're just a bunch of loops.
comment:32 Changed 7 years ago by
 Commit changed from cd80f02c858cad7d4136a75dc4473006c58bf0f0 to 7f9a4a71218515e90a17096b4f8163bfc6e135c9
Branch pushed to git repo; I updated commit sha1. New commits:
7f9a4a7  Added rank 0 functionality. Fixed repeated labels for parallel classes bug.

comment:33 Changed 7 years ago by
 Reviewers set to Stefan van Zwam
 Status changed from needs_work to positive_review
I'm satisfied, this is ready to go in!
comment:34 Changed 7 years ago by
 Branch changed from u/Jayant/ticket/16449 to 7f9a4a71218515e90a17096b4f8163bfc6e135c9
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Added a file matroids+plot_helpers.py and plot() method to abstract