Opened 5 years ago

Closed 4 years ago

#16449 closed enhancement (fixed)

Add plot() and show() methods to abstract Matroid class

Reported by: Jayant Owned by:
Priority: minor Milestone: sage-6.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) 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 5 years ago by Jayant

  • Branch set to u/Jayant/ticket/16449

comment:2 Changed 5 years ago by git

  • Commit set to de4ef370dd307abae62f0a123e3592c2d637a722

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

de4ef37Added a file matroids+plot_helpers.py and plot() method to abstract

comment:3 Changed 5 years ago by Jayant

  • Cc Stefan Rudi added

comment:4 Changed 5 years ago by git

  • Commit changed from de4ef370dd307abae62f0a123e3592c2d637a722 to d1b48a8e5d609b073d54fd7e7b33c5f10a65be68

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

d1b48a8Committed changed matroid.pyx and matroid.pxd which were left out of previous

comment:5 Changed 5 years ago by git

  • Commit changed from d1b48a8e5d609b073d54fd7e7b33c5f10a65be68 to bc38a21f7110493c9854913f76fde880505eb850

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

bc38a21Added documentation to plot() and show() methods of the abstract matroids class.

comment:6 Changed 5 years ago by git

  • Commit changed from bc38a21f7110493c9854913f76fde880505eb850 to 228cdac7b0062378ab8bdd43161c5d061a110e05

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

228cdacAdded examples to matroids_plot_helpers.py

comment:7 Changed 5 years ago by git

  • Commit changed from 228cdac7b0062378ab8bdd43161c5d061a110e05 to 51d676452a559595376aef69ccac2fe7ea0534ea

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

51d6764Added caching of coordinates.

comment:8 Changed 5 years ago by git

  • Commit changed from 51d676452a559595376aef69ccac2fe7ea0534ea to 09b2029d19c35c2960d2102e9bdffc4e10df4f9e

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

09b2029Added ability for user to specify point positions, added option/placeholder for point placement algorithm.

comment:9 Changed 5 years ago by git

  • Commit changed from 09b2029d19c35c2960d2102e9bdffc4e10df4f9e to 1611e8d06e5bb727068ec18be3d9e19a6d1c314b

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

1611e8d100% docstring coverage and minor documentation fixes.

comment:10 Changed 5 years ago by Jayant

  • Status changed from new to needs_review

comment:11 Changed 5 years ago by git

  • Commit changed from 1611e8d06e5bb727068ec18be3d9e19a6d1c314b to 1a26f595814e3b7ef5b01c350858d78fd3dcf56b

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

1a26f59Fixed some spelling mistakes in documentation.

comment:12 follow-up: Changed 5 years ago by Rudi

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 star-like 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 auto-framing 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 follow-up: Changed 5 years ago by Stefan

  • 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 test-drive the code later this morning.

comment:14 Changed 4 years ago by Stefan

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 doc-clean" 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: *** [doc-html] 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 4 years ago by Stefan

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: utf-8 -*-\\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/site-packages/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/site-packages/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 4 years ago by git

  • Commit changed from 1a26f595814e3b7ef5b01c350858d78fd3dcf56b to 3d92382b4f7e2a6131503f77bfee41c5a6107c69

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

95ac8b6Added x-y limits tracking. Added _fix_positions() method to abstract matroids class.
3d92382Added x-y limits tracking. Added _fix_positions() method to abstract matroids class.

comment:17 in reply to: ↑ 12 Changed 4 years ago by Jayant

Replying to Rudi: Added tracking of x-y limits which in a sense, does auto-framing 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 star-like 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 auto-framing 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 4 years ago by Stefan

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 4 years ago by git

  • Commit changed from 3d92382b4f7e2a6131503f77bfee41c5a6107c69 to 4f20c9542bac14a6e4e193e5195c5e8b6142bc14

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

f2e5934Added NotImplementedError for rank > 3 matroids in show() method of the abstract matroids class.
4f20c95pep8 compliance for matroids_plot_helpers.py.

comment:20 in reply to: ↑ 13 Changed 4 years ago by Jayant

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 test-drive the code later this morning.

comment:21 Changed 4 years ago by Stefan

  • Branch changed from u/Jayant/ticket/16449 to u/Stefan/ticket/16449

comment:22 Changed 4 years ago by Stefan

  • 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:

d280e33Documentation and doctest fixes

comment:23 Changed 4 years ago by git

  • Commit changed from d280e33dc53ee8bb3ef2be0ca13a6ec6be79ff7d to fcbbef2ab0053f186c4303feec27bec138dde60f

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

fcbbef2Fixed docbuild errors

comment:24 Changed 4 years ago by Jayant

  • Branch changed from u/Stefan/ticket/16449 to u/Jayant/ticket/16449
  • Commit changed from fcbbef2ab0053f186c4303feec27bec138dde60f to f406d2e45dde540c5e75837660a18d9bdf076944

New commits:

f406d2eFixed bugs reported by Stefan in comment 15 and 18.

comment:25 follow-up: Changed 4 years ago by Stefan

Is this ready for review?

comment:26 in reply to: ↑ 25 Changed 4 years ago by Jayant

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 follow-up: Changed 4 years ago by Stefan

  • Status changed from needs_work to needs_review

Ok. Next time change the ticket status to "review"

comment:28 in reply to: ↑ 27 Changed 4 years ago by Jayant

Replying to Stefan: Oops. Sorry about that.

Ok. Next time change the ticket status to "review"

comment:29 Changed 4 years ago by Stefan

  • Status changed from needs_review to needs_work

There are still issues with parallel elements and low-rank 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: utf-8 -*-\\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/site-packages/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/site-packages/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: utf-8 -*-\\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/site-packages/sage/matroids/matroids_plot_helpers.py", line 830, in geomrep
    list(set(L) | set(P)))
  File "/Users/stefan/sage/local/lib/python2.7/site-packages/sage/matroids/matroids_plot_helpers.py", line 157, in it
    lines.append([B1[pairs[i-1][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 4 years ago by git

  • Commit changed from f406d2e45dde540c5e75837660a18d9bdf076944 to cd80f02c858cad7d4136a75dc4473006c58bf0f0

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

cd80f02Fixed parallel element bug. Modified parallel element placement. Changed point color to lighter gray. Added NotImplementedError for rank 0 matroids.

comment:31 Changed 4 years ago by Stefan

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 rank-0 matroids: they're just a bunch of loops.

comment:32 Changed 4 years ago by git

  • Commit changed from cd80f02c858cad7d4136a75dc4473006c58bf0f0 to 7f9a4a71218515e90a17096b4f8163bfc6e135c9

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

7f9a4a7Added rank 0 functionality. Fixed repeated labels for parallel classes bug.

comment:33 Changed 4 years ago by Stefan

  • Authors set to Jayant Apte
  • 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 4 years ago by vbraun

  • Branch changed from u/Jayant/ticket/16449 to 7f9a4a71218515e90a17096b4f8163bfc6e135c9
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.