Sage: Ticket #7477: Matroids
https://trac.sagemath.org/ticket/7477
<p>
Matroids in Sage could be interesting from the educational point of view, as there are not so many ways to play with matroids on a computer, but also from the algorithmic point of view, as the Graph Theory section could use some help from the Matroid Union and Matroid Intersection Theorems... ( see <a class="closed ticket" href="https://trac.sagemath.org/ticket/7476" title="enhancement: Edge-disjoint spanning trees (closed: fixed)">#7476</a> )
</p>
<p>
Macek is a GPL+C implementation of them <a class="ext-link" href="http://www.fi.muni.cz/~hlineny/MACEK/"><span class="icon"></span>http://www.fi.muni.cz/~hlineny/MACEK/</a> which I never tried but may be a good starting point !
</p>
<p>
Nathann
</p>
<p>
Apply:
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/7477/trac_7477_setup_doc_load.patch" title="Attachment 'trac_7477_setup_doc_load.patch' in Ticket #7477">trac_7477_setup_doc_load.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/7477/trac_7477_setup_doc_load.patch" title="Download"></a> -- changes to module_list.py, all.py, and reference manual
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/7477/trac_7477_code.patch" title="Attachment 'trac_7477_code.patch' in Ticket #7477">trac_7477_code.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/7477/trac_7477_code.patch" title="Download"></a> -- code for the sage.matroids package
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/7477
Trac 1.1.6wdjSun, 26 Sep 2010 11:58:28 GMTupstream set
https://trac.sagemath.org/ticket/7477#comment:1
https://trac.sagemath.org/ticket/7477#comment:1
<ul>
<li><strong>upstream</strong>
set to <em>N/A</em>
</li>
</ul>
<p>
I think adding macek to Sage will be a lot of work... .
</p>
<p>
Another option is to add my code
<a class="ext-link" href="http://boxen.math.washington.edu/home/wdj/sagefiles/matroid_class.sage"><span class="icon"></span>http://boxen.math.washington.edu/home/wdj/sagefiles/matroid_class.sage</a>
I don't have time right now to add this to Sage properly, due to teaching obligations. However, if anyone is interested, I can at least act as one of the referees. If not, I will try to get to this next semester.
</p>
TicketrbeezerWed, 02 Mar 2011 04:55:51 GMTattachment set
https://trac.sagemath.org/ticket/7477
https://trac.sagemath.org/ticket/7477
<ul>
<li><strong>attachment</strong>
set to <em>matroids-beezer-20110105.sage</em>
</li>
</ul>
TicketrbeezerWed, 02 Mar 2011 04:57:07 GMT
https://trac.sagemath.org/ticket/7477#comment:2
https://trac.sagemath.org/ticket/7477#comment:2
<p>
I attended a short course on matroids at the US national math meetings in January 2011, and spent the two days hacking up as much about matroids as I could. I knew David Joyner had done something similar, but I purposely did not look at his work first. So you will see some common ideas and some differences.
</p>
<p>
This is definitely not pretty, nor efficient. The goal was to implement as much functionality as quickly as possible, so there are obvious places where things should be done differently. But it is clear that much of the hard work can be shipped off to Sage routines for graph theory, linear algebra and combinatorics.
</p>
<p>
Implements vector matroid, cycle matroid, bicircular matroid, transversal matroid, uniform matroid, and duals of matroids.
</p>
TicketStefanFri, 25 Mar 2011 09:26:24 GMT
https://trac.sagemath.org/ticket/7477#comment:3
https://trac.sagemath.org/ticket/7477#comment:3
<p>
There is serious interest from the matroid theory community in creating a Sage package. It would provide similar functionality to the graph theory package: lots of methods (extensions, representations, Tutte polynomials, connectivity tests, isomorphism and minor testing, ...) and a database with named matroids and small matroids.
</p>
<p>
Macek has several shortcomings that make it unsuitable; I will mention a few. It only works for representable matroids over the (small number of) partial fields implemented. It has an esoteric command language based on vertex-labelled trees. It has bugs. For instance, it cannot read its own output without modification, and is known not to detect minors when they are clearly present. Its author considers it "done", and will not support it.
</p>
TicketncohenFri, 25 Mar 2011 10:11:22 GMT
https://trac.sagemath.org/ticket/7477#comment:4
https://trac.sagemath.org/ticket/7477#comment:4
<p>
(plus it would speedup the method Graph.edge_disjoint_spanning_tree which is deeeaaad slow right now)
</p>
TicketrbeezerFri, 25 Mar 2011 16:12:00 GMT
https://trac.sagemath.org/ticket/7477#comment:5
https://trac.sagemath.org/ticket/7477#comment:5
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:3" title="Comment 3">Stefan</a>:
</p>
<p>
Hi Stefan,
</p>
<p>
Thanks for the information on MACEK. I didn't know the details, but it looked to me like all support had ended, so it is good to have the confirmation.
</p>
<p>
What will it take to get the matroid community started with Sage?
</p>
<p>
Rob
</p>
TicketwdjFri, 25 Mar 2011 17:19:00 GMT
https://trac.sagemath.org/ticket/7477#comment:6
https://trac.sagemath.org/ticket/7477#comment:6
<p>
Neither Rob's code nor mine is a patch. Any preference? I'm happy to convert my code into a patch and try to integrate Rob's new aspects in, or Rob can create a patch from his.
</p>
TicketStefanFri, 25 Mar 2011 21:55:07 GMTcomponent changed
https://trac.sagemath.org/ticket/7477#comment:7
https://trac.sagemath.org/ticket/7477#comment:7
<ul>
<li><strong>component</strong>
changed from <em>numerical</em> to <em>combinatorics</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:5" title="Comment 5">rbeezer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:3" title="Comment 3">Stefan</a>:
</p>
<p>
What will it take to get the matroid community started with Sage?
</p>
</blockquote>
<p>
Short answer (I sent you a longer by email): the matroid community has already started. We had a <a class="ext-link" href="http://msor.victoria.ac.nz/Main/MatroidComputation"><span class="icon"></span>meeting</a> in December, and will have a followup meeting in June. Several people have committed themselves to the effort.
</p>
<p>
By the way, I changed the "Component" field to combinatorics. I hope that's ok.
</p>
TicketkcrismanMon, 28 Mar 2011 18:47:28 GMTcc set
https://trac.sagemath.org/ticket/7477#comment:8
https://trac.sagemath.org/ticket/7477#comment:8
<ul>
<li><strong>cc</strong>
<em>kcrisman</em> added
</li>
</ul>
TicketrbeezerTue, 29 Mar 2011 04:50:00 GMT
https://trac.sagemath.org/ticket/7477#comment:9
https://trac.sagemath.org/ticket/7477#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:6" title="Comment 6">wdj</a>:
</p>
<blockquote class="citation">
<p>
Neither Rob's code nor mine is a patch. Any preference? I'm happy to convert my code into a patch and try to integrate Rob's new aspects in, or Rob can create a patch from his.
</p>
</blockquote>
<p>
Hi David,
</p>
<p>
Sorry for the delay in replying to this - recovering from Bug Days.
</p>
<p>
I think the computational matroid folks are quite serious about moving a lot of their work to Sage. Maybe it would be best if we let them decide what structure will work best for their purposes, rather than putting in something now that may not work well long-term?
</p>
<p>
You and I could probably best help them by advising and reviewing their contributions, I think.
</p>
<p>
But we shouldn't wait for them forever. ;-)
</p>
<p>
Rob
</p>
Ticketjeremy.l.martinTue, 10 Jul 2012 20:46:19 GMT
https://trac.sagemath.org/ticket/7477#comment:10
https://trac.sagemath.org/ticket/7477#comment:10
<p>
I am brand new to Sage development, but I do a lot of work with matroids and would like to see them implemented in Sage.
</p>
<p>
I will be teaching a graduate course in algebraic combinatorics this fall. I am thinking of having my students create a Matroid sage class as a group project. E.g., they could implement the conversions between all the various ways of presenting a matroid; basic constructions like direct sum and dual; and the Tutte polynomial.
</p>
<p>
Thoughts?
</p>
<p>
Jeremy Martin (University of Kansas)
</p>
TicketkcrismanTue, 10 Jul 2012 20:51:33 GMT
https://trac.sagemath.org/ticket/7477#comment:11
https://trac.sagemath.org/ticket/7477#comment:11
<p>
Sounds like a great idea! You may want to try using the code attached here as a starting point. Also, the sage-combinat folks have some great infrastructure for things like categories, and you should ask them about that. But I think this is a very reasonable project. Especially if you could somehow implement graph.matroid() and/or vecspace.basis.matroid - in the sense that one could have a graph, and then get a matroid they could do stuff with from it.
</p>
TicketStefanTue, 10 Jul 2012 21:32:52 GMT
https://trac.sagemath.org/ticket/7477#comment:12
https://trac.sagemath.org/ticket/7477#comment:12
<p>
Hi!
</p>
<p>
There's a significant effort going on behind the screens to get matroids into Sage. You can get to our work-in-progress code at
</p>
<p>
<a class="ext-link" href="https://bitbucket.org/matroid/sage_matroids/"><span class="icon"></span>https://bitbucket.org/matroid/sage_matroids/</a>
</p>
<p>
and we've got a mailing list at
</p>
<p>
<a class="ext-link" href="https://groups.google.com/group/sage-matroid"><span class="icon"></span>https://groups.google.com/group/sage-matroid</a>
</p>
<p>
Please join in the discussion. There is still plenty to be done.
</p>
<p>
A quick description of our current status: we've got an abstract Matroid class with a bunch of subclasses: <a class="missing wiki">BasisMatroid?</a>, <a class="missing wiki">LinearMatroid?</a>, <a class="missing wiki">RankMatroid?</a>, <a class="missing wiki">CircuitClosuresMatroid?</a> are the main ones. There is support for minors and abstract duality. We have a rather fast isomorphism test, and a host of methods for standard queries. There's also a library of common matroids, and a constructor that takes various inputs (including Sage graphs). So you can write
</p>
<p>
M = Matroid(G)
</p>
<p>
We think we need some minor coding and major documentation-string-writing before we want to submit it to Sage.
</p>
Ticketjeremy.l.martinWed, 11 Jul 2012 11:10:55 GMT
https://trac.sagemath.org/ticket/7477#comment:13
https://trac.sagemath.org/ticket/7477#comment:13
<p>
Thanks! I will join that Google group and see what I can do to help.
</p>
<p>
Right now I am at a sage-combinat workshop at the IMA. There is significant interest among the algebraic combinatorialists here in implementing matroids for Sage; on the other hand, I'm not sure if the sage-combinat folks know about the sage-matroid project. I will make an announcement about it and invite people to get involved.
</p>
<p>
Jeremy
</p>
TicketStefanWed, 01 May 2013 17:08:26 GMTstatus, milestone, description changed; author set
https://trac.sagemath.org/ticket/7477#comment:14
https://trac.sagemath.org/ticket/7477#comment:14
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-wishlist</em> to <em>sage-5.10</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/7477?action=diff&version=14">diff</a>)
</li>
<li><strong>author</strong>
set to <em>Stefan van Zwam, Rudi Pendavingh</em>
</li>
</ul>
TicketStefanWed, 01 May 2013 17:11:15 GMT
https://trac.sagemath.org/ticket/7477#comment:15
https://trac.sagemath.org/ticket/7477#comment:15
<p>
Note: needs Sage 5.9.rc1 to build (because the package list is now generated automatically in setup.py, this used to be manual).
</p>
<p>
Note: the documentation builds fine from scratch, but I sometimes have trouble when adding these patches to an already built reference manual.
</p>
TicketncohenWed, 01 May 2013 17:51:27 GMT
https://trac.sagemath.org/ticket/7477#comment:16
https://trac.sagemath.org/ticket/7477#comment:16
<p>
This review will take a lifetime, but right now all I can say is that the code of <code>circuits</code>, <code>cocircuits</code>, <code>noncospanning_cocircuits</code>, and <code>nonspanning_circuits</code> look awfully similar.
</p>
<p>
And there are some parts of the code that are commented out, like that
</p>
<pre class="wiki">+# def bitset_pickle_test(data):
+# """
+# Converts the list of integers ``data`` into a bitset, which gets pickled.
+# """
+# cdef bitset_t bs
+# m = max(data)
+# bitset_init(bs, m+1)
+# bitset_clear(bs)
+# for i in data:
+# bitset_add(bs, i)
+# p = bitset_pickle(bs)
+# bitset_free(bs)
+# return p
</pre><p>
Or 4 functions in the matroid catalog.
</p>
<p>
Nathann
</p>
TicketStefanWed, 01 May 2013 18:56:17 GMT
https://trac.sagemath.org/ticket/7477#comment:17
https://trac.sagemath.org/ticket/7477#comment:17
<p>
What do you mean by the comment that those functions look similar? They compute similar, but not identical, information from the matroid. And all four are useful to end users.
</p>
<p>
I guess we should have gotten rid of commented out parts (though Sage has plenty of that floating around in its source). I'll revise that soon.
</p>
TicketncohenWed, 01 May 2013 18:58:54 GMT
https://trac.sagemath.org/ticket/7477#comment:18
https://trac.sagemath.org/ticket/7477#comment:18
<p>
Hellooooooo !
</p>
<blockquote class="citation">
<p>
What do you mean by the comment that those functions look similar? They compute similar, but not identical, information from the matroid. And all four are useful to end users.
</p>
</blockquote>
<p>
Nonono, of course you need them. I was just saying that perhaps there could have been an internal function computing all 4 things, exposed in 4 differents ways to the user. This way there is no copy/paste of code.
</p>
<blockquote class="citation">
<p>
I guess we should have gotten rid of commented out parts (though Sage has plenty of that floating around in its source). I'll revise that soon.
</p>
</blockquote>
<p>
Nonono I'm sorry I said that, I will give you lengthier reviews soon. Otherwise it will make you update the patch every day... <code>:-)</code>
</p>
<p>
Nathann
</p>
TicketyomcatWed, 01 May 2013 19:00:48 GMTcc changed
https://trac.sagemath.org/ticket/7477#comment:19
https://trac.sagemath.org/ticket/7477#comment:19
<ul>
<li><strong>cc</strong>
<em>yomcat</em> added
</li>
</ul>
TicketncohenThu, 02 May 2013 17:55:31 GMT
https://trac.sagemath.org/ticket/7477#comment:20
https://trac.sagemath.org/ticket/7477#comment:20
<p>
Hellooooooooooo again !
</p>
<p>
Well, I've been spending some time on that code, and I first have to say that it is awfully clean. I was a bit afraid that something developped outside of Sage would be more combinat-like, and that is not the case at all here.
</p>
<p>
The other thing is that, stupid as it may seem, I had not realized until a couple of minutes that I cannot realistically review 21 000 lines of code by myself. I mean, with a real job that I have to do, with 3 meals per day, everything. Looks complicated. This is one of the problems of developping everything for a while then trying to create a single patch out of it.
</p>
<p>
I will be glad to spend time on that, and also to work with the code later, but yep, I guess that I cannot review 21 000 lines of code <code>:-)</code>
</p>
<p>
I think that in this case, as the code looks preeeeetty clean and all, the best would really be to ask on sage-devel whether :
1) We take it in without a review (as if it were a spkg, actually)
2) Somebody (or many persons) are willing to review it together
</p>
<p>
Perhaps it would also help if you were to say in your message how you produced this code. If many persons wrote that code, if many tried it...
</p>
<p>
I think that I can't do more. Otherwise, I know that I will not set this to "positive review" before I am convinced that every single choice is a good one, or at least having asked about it. And after having thought for a while about each line, wondering if it is the best way to do it. I already spent weeks on easier reviews, and I know that I cannot do that one <code>:-)</code>
</p>
<p>
Nathann
</p>
TicketStefanThu, 02 May 2013 18:02:06 GMT
https://trac.sagemath.org/ticket/7477#comment:21
https://trac.sagemath.org/ticket/7477#comment:21
<p>
I fully understand that you can't - I know I couldn't! I'll post on sage-devel to ask what's to be done.
</p>
<p>
--Stefan.
</p>
TicketkcrismanThu, 02 May 2013 18:03:40 GMT
https://trac.sagemath.org/ticket/7477#comment:22
https://trac.sagemath.org/ticket/7477#comment:22
<p>
Here are two alternate approaches to such a patch bomb.
</p>
<ul><li>Ask the authors to parcel it out into manageable chunks. Given that they have their own bitbucket repository, hopefully it wouldn't be horribly difficult to separate out e.g. the examples, the core functionality, the extended functionality, and then make this a metaticket.
</li><li>Have the authors partially review it for each other; if there are truly 3-5 separate developers, they may be able to vouch for each others' code. I think in this case some non-matroid-project person such as Nathann should look at the overall structure, but I agree that doing it all together is nearly impossible.
</li></ul><p>
I, too, was very impressed with the general layout of this project; a lot of thought has gone into this.
</p>
TicketvbraunFri, 03 May 2013 14:31:36 GMT
https://trac.sagemath.org/ticket/7477#comment:23
https://trac.sagemath.org/ticket/7477#comment:23
<p>
Looks pretty solid. I think you can review the mathematical correctness among yourself. You should have somebody who is more familiar with Sageisms to look at code style, then it should be ready to go. There are some small code style issues that would have been easier if you had started with a smaller chunk of code.
</p>
<p>
PEP8 whitespace <a class="ext-link" href="http://www.python.org/dev/peps/pep-0008/#other-recommendations"><span class="icon"></span>http://www.python.org/dev/peps/pep-0008/#other-recommendations</a>
</p>
<pre class="wiki">i = i + 1 # yes
i=i+1 # no
</pre><p>
Docstring markup <a href="http://www.sagemath.org/doc/developer/conventions.html#docstring-markup-with-rest-and-sphinx">http://www.sagemath.org/doc/developer/conventions.html#docstring-markup-with-rest-and-sphinx</a>:
</p>
<ul><li><code>EXAMPLES::</code> not <code>EXAMPLE::</code> (though thats also misspelled in other places in the sage library)
</li><li>Imperative: "Test that foo is bar" instead of "Tests that foo is bar".
</li><li><code>INPUT:</code> should include type information and our formatting:
<pre class="wiki">- ``n``: The dimension of the projective space. # no
- ``n`` -- positive integer. The dimension of the projective space. # yes
</pre></li><li>We have markup for referencing other docstrings
<pre class="wiki">... see :class:`OtherClass` ...
... or :meth:`other_method` ...
</pre>that you might want to use more consistently.
</li></ul><p>
The SetSystem should probably be factored out and integrated into <code>sage.sets</code>.
</p>
<p>
Your private reimplementation of all matrix functionality has a lot of code-smell. If the only reason is that pivoting is too slow then you should look into fixing that instead of writing your own matrix implementation.
</p>
<p>
Whats this (left-over debugging?):
</p>
<pre class="wiki"> #if d>0:
# F=Fa+Fb
# self._q_projection=self._q_projection.matrix_from_rows_and_columns(F,F)
</pre>
TicketdarijSat, 11 May 2013 06:34:31 GMT
https://trac.sagemath.org/ticket/7477#comment:24
https://trac.sagemath.org/ticket/7477#comment:24
<p>
This looks wildly useful, not just for educational purposes. I always wanted to play around with the matroid Hopf algebra, for example...
</p>
<p>
The remarks below are mostly random and not always to the point. Most of the code is beyound my grasp due to the use of Cython and bitsets and partly due to advanced matroid theory. I'm just looking at random points which seem relevant and/or understandable to me.
</p>
<p>
What does this mean:
</p>
<pre class="wiki">It is up to the child class to update its data structure make information
relative to the new basis more accessible.
</pre><p>
Typo "an an":
</p>
<pre class="wiki">if `I` is covered by a matching an an appropriate bipartite graph
</pre><p>
That said, "covered by" might be useless, given that you're not saying "maximum matching".
</p>
<p>
In the next line, do you really mean "maximal matching", or rather "maximum matching"? (I don't know what you want.)
</p>
<p>
Typo "Compute a the rank". Also, look out for "the the" errors, you got 2 of them.
</p>
<p>
Any chances to get some comments on <code>cdef __fundamental_cocircuit</code> and <code>cdef __fundamental_circuit</code>? I can't say the code is self-explanatory... Is the "fundamental circuit" of a basis B and an element y the set of all elements of B that could be replaced by y, united with {y}? I see why this is a circuit (when y is not in B, that is) but I haven't seen this notation used anywhere...
</p>
<p>
Missing "of" in "of the flats the matroid" and in "of the coflats the matroid". Also, typos: "wheter", "distinguised", "commom".
</p>
<p>
Does the docstring of <code>cpdef _augment(self, X, Y):</code> want to say something like "nonempty (if possible) subset <code>I</code>" or "maximum subset <code>I</code>"? Just a subset is a bit boring...
</p>
<p>
Is matroid union implemented? I can't find it in the code...
</p>
<p>
The "if" in "with the property that if no modular triple of hyperplanes has exactly two members in the modular cut" looks out of place. Incidentally, a definition of the notion of "hyperplane" would be of use, too; I didn't know of that notion so far.
</p>
<p>
The <code>max_weight_independent</code> method raises an error if the ground set is empty and the weights keyword is set, something that might happen in practice in recursive definitions:
</p>
<pre class="wiki">sage: M = matroids.Uniform(0,0)
sage: wt = {}
sage: M.max_weight_independent(weights=wt)
---------------------------------------------------------------------------
IndexError Traceback (most recent call last)
<ipython-input-10-89b2ef4dae56> in <module>()
----> 1 M.max_weight_independent(weights=wt)
/home/darij/sage-5.9/local/lib/python2.7/site-packages/sage/matroids/matroid.so in sage.matroids.matroid.Matroid.max_weight_independent (sage/matroids/matroid.c:25219)()
/home/darij/sage-5.9/local/lib/python2.7/site-packages/sage/matroids/matroid.so in sage.matroids.matroid.Matroid.max_weight_independent (sage/matroids/matroid.c:24847)()
IndexError: list index out of range
</pre><p>
This is due to the <code>if wt[-1][1] < 0:</code> line, of course. Same for <code>max_weight_coindependent</code>.
</p>
<p>
This just looks weird:
</p>
<pre class="wiki"> if self.full_rank()==0:
return True
if self.full_rank()==0 or self.full_corank()==0:
return True
</pre><p>
One of the A's should probably be an Atranspose in:
</p>
<pre class="wiki"> If the matroid is represented by `[I_1 A]`, then the dual is represented by `[-A I_2]` for appropriately sized
identity matrices `I_1, I_2`.
</pre><p>
(The minus sign, on the other hand, I don't see the reason for...)
</p>
<p>
Feature suggestion, if not already implemented: a method to test if a given weight function is generic, i. e., has exactly one maximizing basis. Of course, this is easy thanks to the exchange graph, as one only needs to find a maximizing basis and then check that all its exchange neighbours have strictly smaller weight. This function is useful to some Hopf-algebraic constructions.
</p>
TicketStefanMon, 13 May 2013 14:44:09 GMT
https://trac.sagemath.org/ticket/7477#comment:25
https://trac.sagemath.org/ticket/7477#comment:25
<p>
Thanks for all your kind words, and for your suggestions! Michael Welsh and I worked on them, and the improved code is now on the ticket. Below I'll address your concerns one by one.
</p>
<p>
To ncohenn:
</p>
<ul><li>Similar code for circuits(), cocircuits(), nonspanning_circuits(), nonspanning_cocircuits().
Organizing the code into a common, abstract function is not worth the trouble, in my opinion.
It will be hard to do so without incurring a speed loss in these exponential-in-the-worst-case
methods.
</li><li>Lingering commented-out code from development has been removed.
</li></ul><p>
To vbraun:
</p>
<ul><li>PEP8 compliance has been achieved, except for line lengths.
</li><li>Docstring markup should be much closer to Sage's standards (we use imperatives everywhere, EXAMPLES is plural, INPUT, OUTPUT singular. I did not revisit all inputs to see if they specify type clearly, and did not work on
referencing markup).
</li><li><a class="missing wiki">SetSystem?</a> has twofold functionality right now. It serves as return type for methods like circuits(). We plan to change this in the future: the "yield" keyword did not work in Cython when we started out.
The other function is partition refinement for isomorphism testing. This is mostly of internal use, and makes use of some tweaks. I think a revised <a class="missing wiki">SetSystem?</a> definitely has its place in sage.sets, but our current effort is
not nearly polished enough, and is best kept for internal use by the Matroid code.
</li><li>Matrix functionality: touching Sage's matrix code was daunting: finite field matrices seem to use floating-point implementations, and I did not want to risk speed regressions elsewhere in Sage. I'm following the M4R1
and related efforts with interest, and once they are viable, moving our code back to them is easy enough.
</li></ul><p>
To darij
</p>
<ul><li>We want this code to be useful enough, and fast enough, to answer our research questions. These questions routinely generate and compare hundreds of thousands matroids, so
we aimed for efficiency that matches the best C code out there (Macek and Gordon Royle's private code). We're not there in all methods, but our binary matroids and our isomorphism
test are really fast.
</li><li>I added an example to the description of <a class="missing wiki">BasisExchangeMatroid?</a> to clarify what a child class might do.
</li><li>I expanded the description of the (not yet implemented) class <a class="missing wiki">TransversalMatroid?</a> a little.
</li><li>I fixed the typos mentioned.
</li><li>Yes, "augment" will return the maximal such set.
</li><li>The B-fundamental circuit of e is the unique circuit contained in B+e. This is standard terminology in matroid theory. I added a line to the method explaining this. I was somewhat surprised that we have no user-facing
version of it, but that'll be for a later trac ticket.
</li><li>The B-fundamental cocircuit of e is the unique cocircuit meeting B only in e. This is the dual notion of the previous item.
</li><li>Matroid union is on my wish list. Again, a later trac ticket.
</li><li>I added a definition of hyperplane, and removed the offending "if".
</li><li>Added an extra check to max_weight_independent and max_weight_coindependent (and two doctests covering this case)
</li><li>Removed the double check on self.full_rank()
</li><li>Added the transpose. The minus sign is to ensure the row spaces of the matrices are orthogonal complements of each other.
</li><li>Feature suggestion: I suggest opening a trac ticket. Do you want just True or False, or a list of all maximizing bases? Maybe an option to max_weight_independent would suffice (e.g. find_all=False)? Oh, wait, that is
expensive to compute when the weight function is constant.
</li></ul>
TicketdarijTue, 14 May 2013 00:53:55 GMT
https://trac.sagemath.org/ticket/7477#comment:26
https://trac.sagemath.org/ticket/7477#comment:26
<p>
Thanks for the answers! As for the maximizing bases, I just wanted a True or False; that alone should not be expensive.
</p>
<p>
I shouldn't be burdening you with additional work; I would normally be adding those methods myself if it wasn't for cython (the bitsets scared me as well, but that turned out to be unfounded). If you want people to add functionality, maybe you could document the internal functions better, such as <code>__move_current_basis</code> (not obvious from the title what it does; the same function with only 1 underscore is well-documented), and explain what the pattern behind 2 underscores vs. 1 underscores is (I guess 2-underscore functions take bitsets as input whereas 1-underscore ones use python types?). The doc of <code>nxksrd</code> is too brief; it suggests that the function returns the next k-subset, while apparently what it does is transforming the k-subset in place and returning something that is more like an error code. But take these suggestions with a grain of salt, as I'm not exactly an experienced programmer...
</p>
<p>
A few things you might still want to fix (sorry for not mentioning them earlier):
</p>
<ul><li>You write <code># the engine</code> twice in <code>basis_exchange_matroid.pyx</code> (once on line 199 and again on line 276). A bit confusing ;)
</li></ul><ul><li>The docstring of method <code>_with_coloop</code> incorrectly states that the input is "Nothing".
</li></ul><ul><li>Typos "succesively" and "distinguised".
</li></ul><p>
On an unrelated note: WTF our finite field matrices use floating-point algorithms???
</p>
TicketStefanTue, 14 May 2013 02:32:34 GMT
https://trac.sagemath.org/ticket/7477#comment:27
https://trac.sagemath.org/ticket/7477#comment:27
<p>
I think you started studying the code in the wrong file. The main entry point is matroid.pyx. This is an abstract class of which all others derive. It implements all functionality in terms of just the rank function (ok... it'll convert to <a class="missing wiki">BasisMatroid?</a> for things like isomorphism testing). It should be fairly straightforward, and the code does not swerve far from pure Python.
</p>
<p>
<a class="missing wiki">BasisExchangeMatroid?</a> is a common framework for <a class="missing wiki">BasisMatroid?</a> and <a class="missing wiki">LinearMatroid?</a>. Internally the groundset is translated to a list of integers, which are used for bitset indexing. So we have
</p>
<ul><li>regular methods. User-facing, expected to be careful with input checking.
</li><li>underscored methods. May assume properties regarding their input (type is frozenset, elements are from groundset, two sets are disjoint, ...)
</li><li>doubly underscored methods. Very internal use (usually cdef). Use the encoded version of the groundset, and may have bitset arguments into which the return value is copied.
</li></ul><p>
I think most people who will be adding code, will not move beyond the first underscore (things like union() belong in the generic Matroid class anyway). But certainly the cdef methods deserve a little bit of an explanation.
</p>
<p>
And yes:
</p>
<pre class="wiki">sage: A = Matrix(GF(7), [[1,0,1,1],[0,1,1,2]])
sage: type(A)
sage.matrix.matrix_modn_dense_float.Matrix_modn_dense_float
</pre><p>
In our matroid code we store the entries simply as a list of GF(q) elements, with some splicing commands for row operations. Results in a 10- to 20-time speedup in places. It's weird.
</p>
<p>
I hope I'll get around to doing the further edits soon!
</p>
TicketvbraunTue, 14 May 2013 11:40:42 GMT
https://trac.sagemath.org/ticket/7477#comment:28
https://trac.sagemath.org/ticket/7477#comment:28
<p>
Can you be more specific about which matrix functions are slow? And yes, it is intentional that matrices over certain finite fields are stored as floats --- with SSE you can do 4 float operations in one instruction.
</p>
TicketStefanFri, 17 May 2013 21:45:29 GMT
https://trac.sagemath.org/ticket/7477#comment:29
https://trac.sagemath.org/ticket/7477#comment:29
<p>
vbraun: specifically, operations where the entries of the matrix (over GF(7), say) are changed frequently, if my memory serves me right. I'll come up with some speed tests in a week or two, but right now I'm overwhelmed with work and life.
</p>
<p>
Also: it looks like there are segmentation faults reported by the patchbot... I knew some things broke after Nathann's work on designs, will have to fix that.
</p>
TicketStefanMon, 20 May 2013 19:14:00 GMT
https://trac.sagemath.org/ticket/7477#comment:30
https://trac.sagemath.org/ticket/7477#comment:30
<p>
I posted some info on slow matrix functions on <a class="ext-link" href="https://groups.google.com/d/topic/sage-devel/5PdRIUic2Es/discussion"><span class="icon"></span>https://groups.google.com/d/topic/sage-devel/5PdRIUic2Es/discussion</a>
</p>
<p>
And the example is not contrived: we genuinely saw a 10- to 20-fold speedup in the <a class="missing wiki">LinearMatroid?</a> performance.
</p>
TicketrbeezerSat, 25 May 2013 00:13:05 GMT
https://trac.sagemath.org/ticket/7477#comment:31
https://trac.sagemath.org/ticket/7477#comment:31
<p>
I spent a good chunk of the afternoon looking over the documentation and wrestling with <code>intersphinx</code>.
</p>
<p>
Following are all pretty routine, though maybe the messages from exceptions needs discussion. Second post is about more mysterious stuff.
</p>
<p>
I'm doing
</p>
<pre class="wiki">sage -docbuild reference html
</pre><p>
and might turn to PDF output once this is settled. I'd be happy to ride herd on documentation as part of a group review. By and large, it looks excellent - I like many of the extras, like a discussion on subclassing and accurate synopses and lists at the top of modules. Nits:
</p>
<p>
(1) Set up code in the small patch all looks routine to my eye (except see next post). I have some experience with this, but will not say I am expert at it, so a second look is probably in order. (Interesting to see how little is actually required to add in a whole new field.) On the downside, having "matroid" at the top level is now going to make it harder to auto-complete the top-level "matrix" at the command line. ;-)
</p>
<p>
(2) In "Creating mew Matroid subclasses": "For incidental use, the RankMatroid subclass." needs another "use"?
</p>
<p>
(3) In documentation of matroid.is_isomorphism() the EXAMPLES all have an extra indent. Again with matroid.minor, matroid.equals. You might troll for more of these across all modules.
</p>
<p>
(4) I see "MatroidError" instances in the documentation, specifically here at matroid.max_independent. Is there a precedent for custom exceptions elsewhere in Sage? More commonly errors are TypeError or ValueError it seems to me. (Yes, PEP8 says differently.) And I find the "Problem with a matroid operation:" redundant. I believe it is a Python convention to have error messages begin with a lower case, to follow the colon (but cannot find a reference for this).
</p>
<p>
I'd be inclined to replace
</p>
<pre class="wiki">MatroidError: Problem with a matroid operation: 'Input is not a subset of the groundset.'
</pre><p>
with something like
</p>
<pre class="wiki">ValueError: 'input set is not a subset of the groundset.'
</pre><p>
Even better is to repeat the problem input in the error message. Here:
</p>
<pre class="wiki">ValueError: 'input set ['x'] is not a subset of the groundset.'
</pre><p>
There is usually enough context provided automatically, but echioing the bad input is often very helpful. Also consider making the text of error messages somewhat unique, so that searches will land a user at the right place in the reference manual.
</p>
<p>
(5) Apparently-minor documentation warnings follow. Should be trivial to fix.
</p>
<pre class="wiki">[matroids ] /sage/sage-5.10.beta4/local/lib/python2.7/site-packages/sage/matroids/catalog.py:docstring of sage.matroids.catalog.NonVamos:3: WARNING: Inline interpreted text or phrase reference start-string without end-string.
[matroids ] /sage/sage-5.10.beta4/local/lib/python2.7/site-packages/sage/matroids/catalog.py:docstring of sage.matroids.catalog.P8pp:1: WARNING: Inline interpreted text or phrase reference start-string without end-string.
[matroids ] /sage/sage-5.10.beta4/local/lib/python2.7/site-packages/sage/matroids/catalog.py:docstring of sage.matroids.catalog.P8pp:1: WARNING: Inline interpreted text or phrase reference start-string without end-string.
[matroids ] <autodoc>:0: WARNING: Bullet list ends without a blank line; unexpected unindent.
</pre>
TicketrbeezerSat, 25 May 2013 00:25:07 GMT
https://trac.sagemath.org/ticket/7477#comment:32
https://trac.sagemath.org/ticket/7477#comment:32
<p>
Some errors with building the HTML documentation follow. I think this is from using intersphinx, but am not certain. Perhaps also related to the <code>conf.py</code> files. I'll ping John Palmieri off-list to see if he wants to take a look. On 5.10.beta2, but very similar behaviour on 5.10.beta4.
</p>
<p>
Two files in <code>devel/sage/doc/en/reference</code>: <code>conf.py</code> and <code>conf_sub.py</code>. The first has a long list of top-level sections of the documentation. But to my eye it seems we use the second and not the first (and setup patch here also seems to reference the second).
</p>
<p>
I added <code>'matroids'</code> to the first and tried rebuilding (two passes). Then tried rebuilding with
</p>
<p>
<code>sage -docbuild reference html -S -aE</code>
</p>
<p>
which my notes say will for a rebuild from scratch.
</p>
<p>
Here are the symptoms I'm trying to fix. Many many of these, and indeed the <code>objects.inv</code> is not created for the matroids section.
</p>
<pre class="wiki">[homology ] WARNING: intersphinx inventory '/sage/sage-5.10.beta2/devel/sage/doc/output/html/en/reference/matroids/objects.inv' not fetchable due to <type 'exceptions.IOError'>: [Errno 2] No such file or directory: '/sage/sage-5.10.beta2/devel/sage/doc/output/html/en/reference/matroids/objects.inv'
</pre><p>
I get about seven of these. Perhaps it is due to cdef'ed class definitions. That is a guess. However, limited Googling suggests this can be fixed with a proper <code>conf.py</code>, so maybe another symptom of the same problem. For each file/module, it seem severe enough to keep the whole module's worth of documentation from rendering at all - there is just a title, and no real content.
</p>
<pre class="wiki">[reference] /sage/sage-5.10.beta2/devel/sage/doc/en/reference/matroids/sage/matroids/basis_exchange_matroid.rst:11: WARNING: autodoc can't import/find module 'sage.matroids.basis_exchange_matroid', it reported error: "'module' object has no attribute 'BasisExchangeMatroid'", please check your spelling and sys.path
</pre>
TicketStefanSat, 25 May 2013 00:34:14 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:33
https://trac.sagemath.org/ticket/7477#comment:33
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Hi,
</p>
<p>
I've had trouble building the documentation. I guess adding an extra folder is not easily accounted for. My recipe was
</p>
<ul><li>delete the doc/output directory
</li><li>delete all auto-generated content in doc/en/reference/matroids
</li><li>do a "make doc"
</li></ul><p>
I'll get to the other issues mentioned soon.
</p>
TickettscrimSat, 25 May 2013 02:31:34 GMT
https://trac.sagemath.org/ticket/7477#comment:34
https://trac.sagemath.org/ticket/7477#comment:34
<p>
Hey Rob,
</p>
<p>
It's saying it can't find that file <code>/sage/sage-5.10.beta2/devel/sage/doc/output/html/en/reference/matroids/objects.inv</code>. It probably doesn't exist. When doing top-level doc stuff, I always run
</p>
<pre class="wiki">sage -docbuild all html
</pre><p>
I'd try that first.
</p>
<p>
Also I'm somewhat scared of deleting the entire output directory now. I once did that and consistently got the error your is getting, even doing <code>docbuild all</code> (I ended up reinstalling sage, but that's more because I wanted to upgrade anyways.) Perhaps <code>make doc</code> does a few extra things then <code>docbuild all</code> does...?
</p>
<p>
Stefan,
</p>
<p>
The bullet points need to be formatted like this:
</p>
<pre class="wiki">Some text saying a list:
- the first level, note the blank line
- but this needs a sublist:
- Note the indentation and the blank line inbetween.
- A line which needs a break
will go like this.
</pre><p>
I believe that should take care of the sphinx errors.
</p>
<p>
Best,<br />
Travis
</p>
TicketrbeezerSat, 25 May 2013 03:15:59 GMT
https://trac.sagemath.org/ticket/7477#comment:35
https://trac.sagemath.org/ticket/7477#comment:35
<p>
Dear Travis,
</p>
<p>
Thanks for the suggestions.
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:34" title="Comment 34">tscrim</a>:
</p>
<blockquote class="citation">
<pre class="wiki">sage -docbuild all html
</pre><p>
I'd try that first.
</p>
</blockquote>
<p>
That's looking much better! And without editing a <code>conf.py</code> I'll take it up more seriously in the morning.
</p>
<blockquote class="citation">
<p>
Also I'm somewhat scared of deleting the entire output directory now. I once did that and consistently got the error your is getting, even doing <code>docbuild all</code> (I ended up reinstalling sage, but that's more because I wanted to upgrade anyways.) Perhaps <code>make doc</code> does a few extra things then <code>docbuild all</code> does...?
</p>
</blockquote>
<p>
Well, I just nuked all of <code>doc/output</code> to make it rebuild and it seems to have run fine. Again, I'll double-check in the AM. I wonder what is different, and what needs to change to get <code>-docbuild</code> to just do one part of the docs?
</p>
<p>
Thanks,
Rob
</p>
TicketvbraunSat, 25 May 2013 10:40:13 GMT
https://trac.sagemath.org/ticket/7477#comment:36
https://trac.sagemath.org/ticket/7477#comment:36
<p>
Deleting doc/output is supposed to work, I do it all the time and its works fine. There might have been bugs in specific older Sage versions, though ;-)
</p>
TicketvbraunSun, 26 May 2013 10:45:03 GMT
https://trac.sagemath.org/ticket/7477#comment:37
https://trac.sagemath.org/ticket/7477#comment:37
<p>
Reading more of the code, is there a reason for why the bitset enhancements don't go into <code>sage.misc.bitset</code>? They seem to be useful enough, so why hide it in the matroid directory?
</p>
TicketStefanTue, 28 May 2013 22:30:00 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:38
https://trac.sagemath.org/ticket/7477#comment:38
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
I just uploaded a new version of the patch. I tested it against 5.10.beta4, I hope it didn't break on 5.10.beta5...
</p>
<p>
Items fixed:
</p>
<ul><li>Fixed multiline doctests to have ....:
</li></ul><p>
Responding to darij:
</p>
<ul><li>Improved documentation of cdef methods (especially doubly-underscored ones)
</li><li>Corrected _with_coloop docstring
</li><li>Corrected typos
</li></ul><p>
Responding to rbeezer:
</p>
<ul><li>Fixed (2)
</li><li>Fixed EXAMPLES indentation (3)
</li><li>Got rid of <a class="missing wiki">MatroidError?</a>, made error messages lower case. Did not change content of messages, leaving it for future improvements (4)
</li><li>Fixed the documentation (5)
</li></ul><p>
Responding to vbraun (some comments are replies to issues from sage-devel):
</p>
<ul><li>I'm happy to move the bitset enhancements into the rest of Sage (I think you're right that they belong there), but I'd like to wait until after this patch has been accepted. It just keeps it less of a moving target, and reduces the risk of having to rebase.
</li><li>Fixed/removed some import statements that got broken (this caused the segfaults)
</li><li>Made the lean_matrix interfaces agree with Sage's Matrices to the largest extent (see comments near top of linear_matroid.pyx).
</li></ul><p>
The <a class="missing wiki">LinearMatroid?</a> class only uses the trivial nonstandard <code></code>is_nonzero<code></code> method now. The subclasses <a class="missing wiki">BinaryMatroid?</a>, <a class="missing wiki">TernaryMatroid?</a>, <a class="missing wiki">QuaternaryMatroid?</a>, <a class="missing wiki">RegularMatroid?</a> use a few more methods that have no Sage Matrix equivalent, but all are flagged in the code, and it should be possible to replace them. One last remark: the lean_matrix.pyx code is ONLY used as internal data structure, we don't expose any of it to the user (unless they try really hard). Everything the user sees is a normal Sage matrix.
</p>
TicketvbraunWed, 29 May 2013 12:46:45 GMT
https://trac.sagemath.org/ticket/7477#comment:39
https://trac.sagemath.org/ticket/7477#comment:39
<p>
I get some docbuild errors, e.g.
</p>
<pre class="wiki">[reference] /home/vbraun/opt/sage-5.10.beta5/devel/sage/doc/en/reference/matroids/sage/matroids/basis_exchange_matroid.rst:11: WARNING: autodoc can't import/find module 'sage.matroids.basis_exchange_matroid', it reported error: "'module' object has no attribute 'BasisExchangeMatroid'", please check your spelling and sys.path
[matroids ] /home/vbraun/opt/sage-5.10.beta5/devel/sage/doc/en/reference/matroids/index.rst:7: WARNING: toctree contains reference to nonexisting document 'matroids/sage/matroids/constructor'
</pre>
TicketvbraunWed, 29 May 2013 12:50:13 GMT
https://trac.sagemath.org/ticket/7477#comment:40
https://trac.sagemath.org/ticket/7477#comment:40
<p>
PS: the fact that you are more likely to get conflicts when you also improve other parts of Sage is <strong>not</strong> an excuse for not doing it ;-) You should have separated out the bitset improvements into a different ticket, this would have made this ticket less of a patch bomb, helped with reviewing, and made conflicts (if any) more manageable.
</p>
TicketStefanWed, 29 May 2013 14:34:10 GMT
https://trac.sagemath.org/ticket/7477#comment:41
https://trac.sagemath.org/ticket/7477#comment:41
<p>
Yes, you're right, I didn't look closely enough at the docbuild process. It is related to lazy_import: if I change all.py to do a regular import, then the documentation builds just fine... I guess I'll have to ask sage-devel for advice on that.
</p>
<p>
And opening a separate ticket: that's definitely a good solution, why didn't I think of that?
</p>
TicketrbeezerWed, 29 May 2013 17:49:36 GMT
https://trac.sagemath.org/ticket/7477#comment:42
https://trac.sagemath.org/ticket/7477#comment:42
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:39" title="Comment 39">vbraun</a>:
</p>
<blockquote class="citation">
<p>
I get some docbuild errors, e.g.
</p>
</blockquote>
<p>
Me, too. I got a bit of advice from John Palmieri off-list, but that did not do the trick. FWIW, no matter how hard I try to rebuild from scratch, these persist. I was about to poll sage-devel, but I see that has started.
</p>
<p>
Once this gets sorted, I should be able to finish a review of the documentation part of this.
</p>
<p>
Rob
</p>
TicketjhpalmieriWed, 29 May 2013 21:00:47 GMTdescription changed
https://trac.sagemath.org/ticket/7477#comment:43
https://trac.sagemath.org/ticket/7477#comment:43
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/7477?action=diff&version=43">diff</a>)
</li>
</ul>
<p>
This change ought to fix the docbuilding problems:
</p>
<div class="wiki-code"><div xmlns="http://www.w3.org/1999/xhtml" class="diff">
<ul class="entries">
<li class="entry">
<h2>
<a>doc/en/reference/conf.py</a>
</h2>
<pre>diff --git a/doc/en/reference/conf.py b/doc/en/reference/conf.py</pre>
<table class="trac-diff inline" summary="Differences" cellspacing="0">
<colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
<thead>
<tr>
<th title="File a/doc/en/reference/conf.py">
a
</th>
<th title="File b/doc/en/reference/conf.py">
b
</th>
<td><em></em> </td>
</tr>
</thead>
<tbody class="unmod">
<tr>
<th>94</th><th>94</th><td class="l"><span> 'libs',</span></td>
</tr><tr>
<th>95</th><th>95</th><td class="l"><span> 'logic',</span></td>
</tr><tr>
<th>96</th><th>96</th><td class="l"><span> 'matrices',</span></td>
</tr>
</tbody><tbody class="add">
<tr class="last first">
<th> </th><th>97</th><td class="r"><ins> 'matroids',</ins></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>97</th><th>98</th><td class="l"><span> 'misc',</span></td>
</tr><tr>
<th>98</th><th>99</th><td class="l"><span> 'modabvar',</span></td>
</tr><tr>
<th>99</th><th>100</th><td class="l"><span> 'modfrm',</span></td>
</tr>
</tbody>
</table>
</li>
</ul>
</div></div><p>
However, I think a better solution is to auto-generated this list. Volker, you know the docbuild system. Can you look at my patch?
</p>
TicketjhpalmieriWed, 29 May 2013 21:01:03 GMTattachment set
https://trac.sagemath.org/ticket/7477
https://trac.sagemath.org/ticket/7477
<ul>
<li><strong>attachment</strong>
set to <em>trac_7477-docbuild.patch</em>
</li>
</ul>
TicketjhpalmieriWed, 29 May 2013 21:02:02 GMT
https://trac.sagemath.org/ticket/7477#comment:44
https://trac.sagemath.org/ticket/7477#comment:44
<p>
(If you want, we can move my patch to a different ticket and have it be a prerequisite for this one.)
</p>
TicketrbeezerWed, 29 May 2013 21:27:15 GMT
https://trac.sagemath.org/ticket/7477#comment:45
https://trac.sagemath.org/ticket/7477#comment:45
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:43" title="Comment 43">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
This change ought to fix the docbuilding problems:
</p>
</blockquote>
<p>
Yes, it does, thanks. I had tried that, but was just trying to build the reference only and there was no inventory file. So when you apply the patches here, try
</p>
<pre class="wiki">sage -docbuild all html
</pre><p>
first to get a proper collection of HTML documentation. Maybe John's change should just go in Stefan's setup patch - seems like that is where it belongs. Autogeneration could be something new?
</p>
<p>
And I'm wondering if inventories should get rebuilt on smaller units of the documentation. I'll ask that on sage-devel so as to not clutter this any further.
</p>
TicketjhpalmieriWed, 29 May 2013 21:33:18 GMT
https://trac.sagemath.org/ticket/7477#comment:46
https://trac.sagemath.org/ticket/7477#comment:46
<p>
You should just be able to do <code>sage --docbuild reference/matroids inventory</code> and then <code>sage --docbuild reference/matroids html</code>. Building the documentation this way is a lot like running LaTeX: you need to run it several times to resolve references, links, etc. Running <code>sage --docbuild all html</code> (and the same with <code>pdf</code> in place of <code>html</code>) automates the process, building the docs twice. Using any other options just builds once. (This was a design decision discussed at <a class="closed ticket" href="https://trac.sagemath.org/ticket/6495" title="enhancement: Build the reference manual incrementally (closed: fixed)">#6495</a>.)
</p>
TicketrbeezerWed, 29 May 2013 21:45:07 GMT
https://trac.sagemath.org/ticket/7477#comment:47
https://trac.sagemath.org/ticket/7477#comment:47
<p>
Thanks, John. Just obsoleted much of my post to sage-devel. ;-) I'd read some of <a class="closed ticket" href="https://trac.sagemath.org/ticket/6495" title="enhancement: Build the reference manual incrementally (closed: fixed)">#6495</a> but not carefully enough to follow all the nuances.
</p>
<p>
I've been away too long - need to get caught up. Sorry for the noise.
</p>
TicketStefanThu, 30 May 2013 02:09:43 GMT
https://trac.sagemath.org/ticket/7477#comment:48
https://trac.sagemath.org/ticket/7477#comment:48
<p>
I can confirm that the docbuild patch fixes our problems. I also found that the PDF didn't build. The current update fixes that.
</p>
TicketjdemeyerThu, 30 May 2013 13:34:56 GMT
https://trac.sagemath.org/ticket/7477#comment:49
https://trac.sagemath.org/ticket/7477#comment:49
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:40" title="Comment 40">vbraun</a>:
</p>
<blockquote class="citation">
<p>
PS: the fact that you are more likely to get conflicts when you also improve other parts of Sage is <strong>not</strong> an excuse for not doing it ;-) You should have separated out the bitset improvements into a different ticket, this would have made this ticket less of a patch bomb, helped with reviewing, and made conflicts (if any) more manageable.
</p>
</blockquote>
<p>
Completely agreed.
</p>
TicketvbraunThu, 30 May 2013 14:47:00 GMT
https://trac.sagemath.org/ticket/7477#comment:50
https://trac.sagemath.org/ticket/7477#comment:50
<p>
John's docbuilding patch looks good to me.
</p>
<p>
Rob: are you still proof reading the documentation?
</p>
TicketrbeezerThu, 30 May 2013 18:23:41 GMT
https://trac.sagemath.org/ticket/7477#comment:51
https://trac.sagemath.org/ticket/7477#comment:51
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:50" title="Comment 50">vbraun</a>:
</p>
<blockquote class="citation">
<p>
Rob: are you still proof reading the documentation?
</p>
</blockquote>
<p>
Yes, hope to finish by the end of the weekend (will be way next week at a conference).
</p>
<p>
"proof reading" might be a bit strong. I'm looking for adherence to our conventions and only reporting minor problems as I go. Generally, it is in great shape. (And of course, way better than some of the older bits of Sage.) Now that I can actually build all of the documentation, I should be able to wrap it up soon.
</p>
<p>
John's patch looks good to me. I guess the onus will be to now keep the exclusions up-to-date (eg static, template) if new ones come along. I had not looked too carefully before - it does not really belong in the setup patch here. Does it make sense to put it on its own ticket, so it does not get buried in this (big) ticket? Make it a dependency of this ticket?
</p>
TicketjhpalmieriThu, 30 May 2013 18:47:23 GMTdescription changed; dependencies set
https://trac.sagemath.org/ticket/7477#comment:52
https://trac.sagemath.org/ticket/7477#comment:52
<ul>
<li><strong>dependencies</strong>
set to <em>#14669</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/7477?action=diff&version=52">diff</a>)
</li>
</ul>
<p>
I moved the docbuilding patch to <a class="closed ticket" href="https://trac.sagemath.org/ticket/14669" title="enhancement: autogenerate the list of subdirectories of doc/en/reference (closed: fixed)">#14669</a>.
</p>
TicketStefanThu, 30 May 2013 19:22:08 GMT
https://trac.sagemath.org/ticket/7477#comment:53
https://trac.sagemath.org/ticket/7477#comment:53
<p>
Just for the record, I made tickets <a class="closed ticket" href="https://trac.sagemath.org/ticket/14666" title="enhancement: Test if a weight function is generic for a given matroid (closed: fixed)">#14666</a> <a class="closed ticket" href="https://trac.sagemath.org/ticket/14667" title="enhancement: Matroid union (closed: wontfix)">#14667</a> <a class="closed ticket" href="https://trac.sagemath.org/ticket/14668" title="task: Move functions from sage.matroids.bitset_tools to sage.misc (closed: fixed)">#14668</a> to address the suggestions made in this thread that we haven't implemented right away.
</p>
TicketjdemeyerThu, 30 May 2013 20:32:54 GMT
https://trac.sagemath.org/ticket/7477#comment:54
https://trac.sagemath.org/ticket/7477#comment:54
<p>
About <a class="closed ticket" href="https://trac.sagemath.org/ticket/14668" title="task: Move functions from sage.matroids.bitset_tools to sage.misc (closed: fixed)">#14668</a>: better yet, add the bitset code <strong>only</strong> in <a class="closed ticket" href="https://trac.sagemath.org/ticket/14668" title="task: Move functions from sage.matroids.bitset_tools to sage.misc (closed: fixed)">#14668</a> (as if it never existed in <code>sage/matroids/bitset_tools.pyx</code>) and make this ticket depend on <a class="closed ticket" href="https://trac.sagemath.org/ticket/14668" title="task: Move functions from sage.matroids.bitset_tools to sage.misc (closed: fixed)">#14668</a>.
</p>
TicketrbeezerFri, 31 May 2013 01:38:05 GMT
https://trac.sagemath.org/ticket/7477#comment:55
https://trac.sagemath.org/ticket/7477#comment:55
<p>
Dear Stefan,
</p>
<p>
OK, here's the result of another pass - got through two sections this time. Some of this may be systematic, I'll let you decide what needs mass editing, and what might be one-off. Generally, the documentation is really very good. Lots of thought has gone into it and I'm sure it will be easy for new users to get up-to-speed quickly. Most of this is annoying details (yes, I said annoying). Some themes, amplified in specific comments below:
</p>
<ol><li> Single backticks get TeX treatment for mathematics. Small chunks of code need double backticks. This might be isolated in small parts of the constructor file.
</li><li> Making references to documentation of other classes (inside or outside of the matroids package) would be better as live links, definitely when you say "see...".
</li><li> I'm not too dogmatic about long lines in doctests, but an effort should be made to line-break both input and output in the cases where it goes on forever.
</li><li> Lots of "simple" methods without OUTPUT sections. Yes, it seems silly to just say "foo() returns all the foo's of the matroid." I like to provide some very basic idea of what a foo is if possible, then the documentation seems worthwhile. I've learned a lot of math by reading Sage documentation.
</li></ol><p>
I know this seems like a lot of trouble, but it will be worthwhile in the long run. I'm thinking this package might be a very good example for others to follow if they wish to add-in other new areas of mathematics.
</p>
<p>
Rob
</p>
<p>
Matroid construction (constructor.pyx):
</p>
<blockquote>
<p>
"For EXAMPLES:" appears at least twice, probably a mass substitution hitting the phrase "For example". Perhaps more in other files?
</p>
</blockquote>
<blockquote>
<p>
The flexibility of the Matroid() constructor in the first two arguments may drive you crazy some day down the road. I am not suggesting you change it - just a comment.
</p>
</blockquote>
<blockquote>
<p>
"A matroid specified by a list of circuits gets converted to a <a class="missing wiki">BasisMatroid?</a> internally:"
Making <a class="missing wiki">BasisMatroid?</a> a link to the class would be an improvement.
</p>
</blockquote>
<blockquote>
<p>
"Note: if a groundset is specified, we assume it is in the same order as G.edge_iterator() provides:"
It'd be nice if small chunks of code were typeset as such, here <code></code>G.edge_iterator()<code></code>
</p>
</blockquote>
<blockquote>
<p>
Code: "If that fails, we simply use a list <code>[0..m-1]</code>"
The single backticks are giving TeX, this should definitely be code, hence two backticks
Probably the (i,j) preceding is similar, though we could debate if that is math or code.
</p>
</blockquote>
<blockquote>
<p>
Code: "When the <code></code>graph<code></code> keyword is used, a variety of inputs can be converted to a graph automatically. The following
uses a graph6 string (see the <code></code>Graph()<code></code> method's documentation)::"
Formatting of the keyword is exactly correct. Perhaps "Graph()" should definitely be a link when it is used in a suggestion to go there.
</p>
</blockquote>
<blockquote>
<p>
Code: "sage: M = Matroid(circuit_closures={3: ['edfg', 'acdg', 'bcfg', 'cefh', 'afgh', 'abce', 'abdf', 'begh', 'bcdh', 'adeh'], 4: <a class="missing wiki">abcdefgh?</a>})
Makes a very long line in the documentation, which puts up a scrollable box in my browser.
You could define the the long list on its own line, then just reference it, or I think there is enough flexibility on where you can line break the command (maybe open the dictionary, put each key/value on its own line, mildly indented, then close dictionary at start of a subsequent line?).
</p>
</blockquote>
<p>
Abstract class (matroid.pyx)
</p>
<blockquote>
<p>
URL in [GG12] seems broken
</p>
</blockquote>
<blockquote>
<p>
Doctest output:
</p>
<blockquote>
<p>
"sage: M = matroids.named_matroids.Fano()
</p>
<blockquote>
<p>
sage: sorted([sorted(C) for C in M.circuits()])
<a class="missing wiki">a', 'b', 'c', 'g'], ['a', 'b', 'd', 'e'], ['a', 'b', 'f'], ['a', 'c', 'd', 'f'], ['a', 'c', 'e'], ['a', 'd', 'g'], ['a', 'e', 'f', 'g'], ['b', 'c', 'd'], ['b', 'c', 'e', 'f'], ['b', 'd', 'f', 'g'], ['b', 'e', 'g'], ['c', 'd', 'e', 'g'], ['c', 'f', 'g'], ['d', 'e', 'f?</a>"
</p>
</blockquote>
</blockquote>
<p>
You can line-break output at any whitespace and the test should succeed. Here, you could put each interior list
</p>
</blockquote>
<p>
on a line of its own. I have not looked at the PDF version, but I suspect this will just run off the right edge of the page.
</p>
<blockquote>
<p>
f-vector() documentation:
</p>
<blockquote>
<p>
Has a list with just one item for output. Probably does not need to be a list. Ditto for flats(). I'm seeing more like this. I'm inclined to just write a paragraph, unless returning a pair, triple, or ...
</p>
</blockquote>
</blockquote>
<blockquote>
<p>
groundset():
</p>
<blockquote>
<p>
Not sure an INPUT section is necessary if there is none. Even though this is an abstract method, maybe the OUTPUT should be documented?
</p>
</blockquote>
</blockquote>
<blockquote>
<p>
is_connected(), is_cosimple() (and more is_ methods) lack OUTPUT, even if it seems silly to have it.
</p>
</blockquote>
<blockquote>
<p>
"loops() Return the set of loops of the matroid." Is it easy to say in an OUTPUT section what a loop is? Some people learn mathematics by reading the documentation. You do not have to go overboard, but can you say in one sentence what a loop is? It makes the documantation look less vacuous. (eg OUTPUT: A list of the loops of the matroid. A loop is a one-element dependent set.)
</p>
</blockquote>
TicketyomcatFri, 31 May 2013 20:13:48 GMT
https://trac.sagemath.org/ticket/7477#comment:56
https://trac.sagemath.org/ticket/7477#comment:56
<p>
I made most of the changes detailed below (I'll leave it to Stefan to upload and make sure I didn't break anything)
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:55" title="Comment 55">rbeezer</a>:
</p>
<blockquote class="citation">
<ol><li> I'm not too dogmatic about long lines in doctests, but an effort should be made to line-break both input and output in the cases where it goes on forever.
</li></ol></blockquote>
<p>
You haven't got to catalog.py yet (it's horrible for this), but is there a general guideline on characters per line?
</p>
<blockquote class="citation">
<ol><li> Lots of "simple" methods without OUTPUT sections. Yes, it seems silly to just say "foo() returns all the foo's of the matroid." I like to provide some very basic idea of what a foo is if possible, then the documentation seems worthwhile. I've learned a lot of math by reading Sage documentation.
</li></ol></blockquote>
<p>
I changed all of those that I am able to (some I don't know what they are).
</p>
<blockquote class="citation">
<blockquote>
<p>
Code: "If that fails, we simply use a list <code>[0..m-1]</code>"
The single backticks are giving TeX, this should definitely be code, hence two backticks
Probably the (i,j) preceding is similar, though we could debate if that is math or code.
</p>
</blockquote>
</blockquote>
<p>
There were lots like this, so I left them as they are, as that's a debate I don't really want to have.
</p>
<blockquote class="citation">
<blockquote>
<p>
f-vector() documentation:
</p>
<blockquote>
<p>
Has a list with just one item for output. Probably does not need to be a list. Ditto for flats(). I'm seeing more like this. I'm inclined to just write a paragraph, unless returning a pair, triple, or ...
</p>
</blockquote>
</blockquote>
</blockquote>
<p>
I'm confused. f_vector() returns a list [f_0, ..., f_r] where f_i is the number of flats of rank i, and r the rank of the matroid. That's got more than one thing in it. And flats() returns a <a class="missing wiki">SetSystem?</a>, not a list, and there's normally a lot more than one flat at a given rank.
</p>
<p>
Michael
</p>
TicketrbeezerSat, 01 Jun 2013 03:49:15 GMT
https://trac.sagemath.org/ticket/7477#comment:57
https://trac.sagemath.org/ticket/7477#comment:57
<p>
Thanks, Michael.
</p>
<p>
Comments interspersed.
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:56" title="Comment 56">yomcat</a>:
</p>
<blockquote class="citation">
<p>
You haven't got to catalog.py yet (it's horrible for this), but is there a general guideline on characters per line?
</p>
</blockquote>
<blockquote>
<p>
"Python Enhancemant Proposal 8", aka PEP 8, <a class="ext-link" href="http://www.python.org/dev/peps/pep-0008/"><span class="icon"></span>http://www.python.org/dev/peps/pep-0008/</a>
</p>
</blockquote>
<p>
says "max 79 characters"
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote>
<p>
Code: "If that fails, we simply use a list <code>[0..m-1]</code>"
The single backticks are giving TeX, this should definitely be code, hence two backticks
Probably the (i,j) preceding is similar, though we could debate if that is math or code.
</p>
</blockquote>
</blockquote>
<p>
There were lots like this, so I left them as they are, as that's a debate I don't really want to have.
</p>
</blockquote>
<p>
Yes, no need to debate, but <code>[0..m-1]</code> is definitely code and should definitely be in double backticks. I don't want to debate either - but it is often extremely clear if you have code or math, and these should be formatted right.
</p>
<blockquote class="citation">
<blockquote class="citation">
<blockquote>
<p>
f-vector() documentation:
</p>
<blockquote>
<p>
Has a list with just one item for output. Probably does not need to be a list. Ditto for flats(). I'm seeing more like this. I'm inclined to just write a paragraph, unless returning a pair, triple, or ...
</p>
</blockquote>
</blockquote>
</blockquote>
<p>
I'm confused. f_vector() returns a list [f_0, ..., f_r] where f_i is the number of flats of rank i, and r the rank of the matroid. That's got more than one thing in it. And flats() returns a <a class="missing wiki">SetSystem?</a>, not a list, and there's normally a lot more than one flat at a given rank.
</p>
</blockquote>
<p>
My mistake, not clear. The OUTPUT section documents a single output of trhe function and is formatted as a list with one item. I don't think a list is necessary. INPUT will often have many items, and a list is natural. There are lots of OUTPUT done as a one-item list. <code>f_vector()</code> was just the first one I saw.
</p>
TickettscrimSat, 01 Jun 2013 15:29:14 GMT
https://trac.sagemath.org/ticket/7477#comment:58
https://trac.sagemath.org/ticket/7477#comment:58
<p>
My 2 cents on <code>OUTPUT:</code> and <code>INPUT:</code> blocks. If there is no input other than <code>self</code>, there is no need for an <code>INPUT:</code> IMO since with <code>self</code>, the input is clear. For output blocks, if the short description of the method (i.e. the first line) tells the object that it returns, I don't see the need to write an <code>OUTPUT:</code> block. Additionally if the object needs description (mathematically), then I think that should go in the second paragraph of the documentation. For example:
</p>
<pre class="wiki">def loops(self):
"""
Return the loops of ``self``.
A *loop* is a one-element dependent subset.
EXAMPLES::
...
def is_cosimple(self):
"""
Return ``True`` if ``self`` is cosimple.
A matriod is *cosimple* if ...
EXAMPLES::
...
</pre><p>
Also if you find yourself needing a (longer) definition multiple times, either explicitly referencing the method/function with the definition or using a <code>.. SEEALSO::</code> block is a good way to go IMO. I think this is a good balance between descriptive, straight-forward, and simple. However I'm somewhat splitting hairs here.
</p>
<p>
Additionally, I would like to see <a class="closed ticket" href="https://trac.sagemath.org/ticket/14668" title="task: Move functions from sage.matroids.bitset_tools to sage.misc (closed: fixed)">#14668</a> as a dependency and only have the bitset code there.
</p>
TicketvbraunSat, 01 Jun 2013 15:54:25 GMT
https://trac.sagemath.org/ticket/7477#comment:59
https://trac.sagemath.org/ticket/7477#comment:59
<p>
For the record, <code>self</code> is not considered an argument for docstring purposes. Skip <code>INPUT</code> if <code>self</code> is the only argument, and list everything except <code>self</code> otherwise. The <code>OUTPUT</code> is a good place to say something about the type of the output. Can be very short:
</p>
<pre class="wiki">def is_cosimple(self):
"""
Test whether the matroid is cosimple.
A matriod is *cosimple* if ...
OUTPUT:
Boolean.
EXAMPLES::
...
</pre><p>
Its not silly, since Python is dynamically typed it is generally not obvious what the result type will be.
</p>
TicketStefanFri, 07 Jun 2013 03:08:07 GMTdependencies changed
https://trac.sagemath.org/ticket/7477#comment:60
https://trac.sagemath.org/ticket/7477#comment:60
<ul>
<li><strong>dependencies</strong>
changed from <em>#14669</em> to <em>#14669, #14668</em>
</li>
</ul>
TicketStefanFri, 07 Jun 2013 03:09:36 GMT
https://trac.sagemath.org/ticket/7477#comment:61
https://trac.sagemath.org/ticket/7477#comment:61
<p>
Ok, the bitset code is now separated into a patch in <a class="closed ticket" href="https://trac.sagemath.org/ticket/14668" title="task: Move functions from sage.matroids.bitset_tools to sage.misc (closed: fixed)">#14668</a> (where I also did a bit of cleanup in directly surrounding code).
</p>
TicketStefanSat, 15 Jun 2013 00:48:39 GMTattachment set
https://trac.sagemath.org/ticket/7477
https://trac.sagemath.org/ticket/7477
<ul>
<li><strong>attachment</strong>
set to <em>trac_7477_setup_doc_load-oldish.patch</em>
</li>
</ul>
<p>
Matroid setup, autoload, documentation
</p>
TicketStefanSat, 15 Jun 2013 00:56:45 GMT
https://trac.sagemath.org/ticket/7477#comment:62
https://trac.sagemath.org/ticket/7477#comment:62
<p>
apply trac_7477_setup_doc_load.patch, trac_7477_code.patch
</p>
<p>
I just uploaded a revision. I revisited all documentation strings, adding mathematical explanations, cross references, and revisiting the OUTPUT blocks with the above suggestions in mind. The documentation builds without warnings or errors, I believe.
</p>
TicketrbeezerSun, 16 Jun 2013 03:42:27 GMT
https://trac.sagemath.org/ticket/7477#comment:63
https://trac.sagemath.org/ticket/7477#comment:63
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:62" title="Comment 62">Stefan</a>:
</p>
<blockquote class="citation">
<p>
I just uploaded a revision.
</p>
</blockquote>
<p>
Thanks, Stefan. At Sage Days all next week, so I'll try to give it a look sometime then.
</p>
TicketrbeezerTue, 18 Jun 2013 18:41:45 GMTstatus changed; keywords, reviewer set
https://trac.sagemath.org/ticket/7477#comment:64
https://trac.sagemath.org/ticket/7477#comment:64
<ul>
<li><strong>keywords</strong>
<em>sd48</em> added
</li>
<li><strong>reviewer</strong>
set to <em>Volker Braun, Rob Beezer</em>
</li>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
This looks real good to me. I'm with Volker at Sage Days 48 and he has reviewed the code (and integration with Sage). I'll check off on the documentation, which looks excellent. The HTML output looks good, as do the 145 pages of the PDF reference manual.
</p>
<p>
All doctests in sage/matroids pass on 5.10rc2 with dependencies.
</p>
<p>
So, positive review.
</p>
<p>
Be prepared for a few hiccups once this gets merged into a beta and gets tested across a diverse collection of systems. Hooefully this will also go into an early stage of the 5.11 series. Thanks for your patience with the review process.
</p>
<p>
And thanks for the major contribution to Sage. I hope this makes your meeting later this month even more productive.
</p>
<p>
Next up: a thematic tutorial! ;-)
</p>
<p>
Rob
</p>
TicketStefanTue, 18 Jun 2013 18:56:02 GMT
https://trac.sagemath.org/ticket/7477#comment:65
https://trac.sagemath.org/ticket/7477#comment:65
<p>
That is great news! Nathann, Karl-Dieter, Darij, Travis, Rob, Volker, thanks a lot for helping with the review! I'm very happy that we are finally going to be an official part of Sage!
</p>
<p>
I'll keep an eye out for the hiccups. We've already been through a weird one while the code was in development (see the file minorfix.h).
</p>
TicketrbeezerTue, 18 Jun 2013 19:35:09 GMT
https://trac.sagemath.org/ticket/7477#comment:66
https://trac.sagemath.org/ticket/7477#comment:66
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:65" title="Comment 65">Stefan</a>:
</p>
<blockquote class="citation">
<p>
That is great news! Nathann, Karl-Dieter, Darij, Travis, Rob, Volker, thanks a lot for helping with the review!
</p>
</blockquote>
<p>
I did not intend to exclude any reviewers from the "Reviewers" field - so please add yourself if you wish to be part of that.
</p>
<p>
Rob
</p>
TicketjdemeyerTue, 18 Jun 2013 20:47:04 GMT
https://trac.sagemath.org/ticket/7477#comment:67
https://trac.sagemath.org/ticket/7477#comment:67
<p>
Never use
</p>
<pre class="wiki">except:
</pre><p>
without specifying an exception. Catch a specific exception instead, or if you must use a catch-all, use
</p>
<pre class="wiki">except StandardError:
</pre><p>
(or <code>BaseException</code> if you really need to catch everything, including <code>KeyboardInterrupt</code>)
</p>
TicketjdemeyerTue, 18 Jun 2013 20:47:18 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:68
https://trac.sagemath.org/ticket/7477#comment:68
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
TicketjdemeyerWed, 19 Jun 2013 06:43:11 GMTdependencies changed
https://trac.sagemath.org/ticket/7477#comment:69
https://trac.sagemath.org/ticket/7477#comment:69
<ul>
<li><strong>dependencies</strong>
changed from <em>#14669, #14668</em> to <em>#14669, #14668, #9880</em>
</li>
</ul>
<p>
This needs to be rebased to <a class="closed ticket" href="https://trac.sagemath.org/ticket/9880" title="defect: Pynac comparison functions do not provide a SWO (closed: fixed)">#9880</a>:
</p>
<pre class="wiki">sage -t --long devel/sage/sage/matroids/matroid.pyx
**********************************************************************
File "devel/sage/sage/matroids/matroid.pyx", line 188, in sage.matroids.matroid
Failed example:
M.tutte_polynomial(var('x'), var('y'))
Expected:
x^2*y^2 + 2*x*y^3 + x^3 + y^4 + 3*x^2*y + 3*x*y^2 + y^3
Got:
x^2*y^2 + 2*x*y^3 + y^4 + x^3 + 3*x^2*y + 3*x*y^2 + y^3
**********************************************************************
File "devel/sage/sage/matroids/matroid.pyx", line 362, in sage.matroids.matroid.Matroid
Failed example:
M.tutte_polynomial(var('x'), var('y'))
Expected:
x^2*y^2 + 2*x*y^3 + x^3 + y^4 + 3*x^2*y + 3*x*y^2 + y^3
Got:
x^2*y^2 + 2*x*y^3 + y^4 + x^3 + 3*x^2*y + 3*x*y^2 + y^3
**********************************************************************
</pre>
TicketStefanThu, 20 Jun 2013 02:19:27 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:70
https://trac.sagemath.org/ticket/7477#comment:70
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Uploaded new version:
</p>
<ul><li>no more blanket except: statements
</li><li>all tests pass on 5.11.beta1
</li><li>minor addition to documentation of matroids_catalog.py
</li></ul>
TicketkcrismanThu, 20 Jun 2013 02:24:11 GMT
https://trac.sagemath.org/ticket/7477#comment:71
https://trac.sagemath.org/ticket/7477#comment:71
<p>
Just FYI, there are some people working on getting some form of hyperplane arrangements into Sage, and I've alerted them to this effort :-)
</p>
TicketvbraunThu, 20 Jun 2013 04:21:05 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:72
https://trac.sagemath.org/ticket/7477#comment:72
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketjdemeyerThu, 20 Jun 2013 07:18:22 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:73
https://trac.sagemath.org/ticket/7477#comment:73
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Please use spaces for indentation, not TABs, even in the non-Python file <code>sage/matroids/minorfix.h</code>.
</p>
TicketStefanThu, 20 Jun 2013 12:09:29 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:74
https://trac.sagemath.org/ticket/7477#comment:74
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Tabs changed to spaces
</p>
TicketjdemeyerThu, 20 Jun 2013 12:30:48 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:75
https://trac.sagemath.org/ticket/7477#comment:75
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
positive_review assuming that only the TABs changed.
</p>
TicketjdemeyerThu, 20 Jun 2013 21:33:43 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/7477#comment:76
https://trac.sagemath.org/ticket/7477#comment:76
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.11.beta3</em>
</li>
</ul>
TicketjdemeyerFri, 21 Jun 2013 06:15:10 GMTstatus changed; resolution, merged deleted
https://trac.sagemath.org/ticket/7477#comment:77
https://trac.sagemath.org/ticket/7477#comment:77
<ul>
<li><strong>status</strong>
changed from <em>closed</em> to <em>new</em>
</li>
<li><strong>resolution</strong>
<em>fixed</em> deleted
</li>
<li><strong>merged</strong>
<em>sage-5.11.beta3</em> deleted
</li>
</ul>
<p>
Solaris doesn't like <code>_N</code> as it uses that internally for other purposes. So you must use a different (either longer or either without leading underscore) variable name instead of <code>_N</code> in <code>sage/matroids/set_system.pxd</code>
</p>
TicketStefanFri, 21 Jun 2013 14:13:54 GMTattachment set
https://trac.sagemath.org/ticket/7477
https://trac.sagemath.org/ticket/7477
<ul>
<li><strong>attachment</strong>
set to <em>trac_7477_code.patch</em>
</li>
</ul>
<p>
Matroid code
</p>
TicketStefanFri, 21 Jun 2013 14:14:39 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:78
https://trac.sagemath.org/ticket/7477#comment:78
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replaced some variable names. All doctests pass.
</p>
TicketvbraunFri, 21 Jun 2013 17:58:11 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:79
https://trac.sagemath.org/ticket/7477#comment:79
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Looks good.
</p>
<p>
In C/C++, identifiers starting with an underscore and followed by an upper case letter are reserved. So Solaris is standards-compliant in this case.
</p>
TicketkcrismanMon, 24 Jun 2013 18:52:40 GMT
https://trac.sagemath.org/ticket/7477#comment:80
https://trac.sagemath.org/ticket/7477#comment:80
<p>
So, just to be clear, to avoid
</p>
<pre class="wiki">WARNING: intersphinx inventory '/Users/.../sage-5.11.beta1/devel/sage/doc/output/html/en/reference/matroids/objects.inv' not fetchable due to <type 'exceptions.IOError'>:
</pre><p>
errors, in the future we have to always do
</p>
<pre class="wiki">sage -docbuild all html
</pre><p>
or do this inventory thing? I don't see any mention in the <a href="http://www.sagemath.org/doc/developer/sage_manuals.html?highlight=docbuild">documentation</a>, nor at <a class="closed ticket" href="https://trac.sagemath.org/ticket/14699" title="defect: Install ATLAS header files and cleanup IML spkg (closed: fixed)">#14699</a>, and I am using 5.11.beta1 here.
</p>
TicketvbraunMon, 24 Jun 2013 19:08:31 GMT
https://trac.sagemath.org/ticket/7477#comment:81
https://trac.sagemath.org/ticket/7477#comment:81
<p>
Only the reference manual uses the two-step build process, so its either
</p>
<pre class="wiki"> sage -docbuild reference inventory # only necessary if inventory changed
sage -docbuild reference/DIR
</pre><p>
or
</p>
<pre class="wiki"> sage -docbuild reference html
</pre>
TicketkcrismanMon, 24 Jun 2013 19:22:15 GMT
https://trac.sagemath.org/ticket/7477#comment:82
https://trac.sagemath.org/ticket/7477#comment:82
<p>
I had a problem with the latter, though. Anyway, I won't be changing the inventory much soon...
</p>
TicketjdemeyerSun, 21 Jul 2013 21:28:03 GMTmilestone changed
https://trac.sagemath.org/ticket/7477#comment:83
https://trac.sagemath.org/ticket/7477#comment:83
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
TicketdimpaseSun, 28 Jul 2013 19:29:52 GMT
https://trac.sagemath.org/ticket/7477#comment:84
https://trac.sagemath.org/ticket/7477#comment:84
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/7477#comment:82" title="Comment 82">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
I had a problem with the latter, though. Anyway, I won't be changing the inventory much soon...
</p>
</blockquote>
<p>
I suppose the latter Sage call was meant to replace the last line from the 1st code snippet, not both of them.
</p>
TicketjdemeyerTue, 30 Jul 2013 10:07:29 GMTstatus, dependencies changed
https://trac.sagemath.org/ticket/7477#comment:85
https://trac.sagemath.org/ticket/7477#comment:85
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#14669, #14668, #9880</em> to <em>#14669, #14668, #9880, #8386</em>
</li>
</ul>
<p>
Please rebase to <a class="closed ticket" href="https://trac.sagemath.org/ticket/8386" title="defect: move iet from sage.combinat to sage.dynamics (closed: fixed)">#8386</a>.
</p>
TicketStefanTue, 30 Jul 2013 16:07:25 GMTattachment set
https://trac.sagemath.org/ticket/7477
https://trac.sagemath.org/ticket/7477
<ul>
<li><strong>attachment</strong>
set to <em>trac_7477_setup_doc_load.patch</em>
</li>
</ul>
<p>
Matroid setup, autoload, documentation
</p>
TicketStefanTue, 30 Jul 2013 17:11:49 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:86
https://trac.sagemath.org/ticket/7477#comment:86
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Rebased to <a class="closed ticket" href="https://trac.sagemath.org/ticket/8386" title="defect: move iet from sage.combinat to sage.dynamics (closed: fixed)">#8386</a>, only the reference manual's index needed updating because of a new entry introduced by <a class="closed ticket" href="https://trac.sagemath.org/ticket/8386" title="defect: move iet from sage.combinat to sage.dynamics (closed: fixed)">#8386</a>.
</p>
TicketvbraunThu, 01 Aug 2013 01:15:40 GMTstatus changed
https://trac.sagemath.org/ticket/7477#comment:87
https://trac.sagemath.org/ticket/7477#comment:87
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketjdemeyerFri, 16 Aug 2013 21:10:24 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/7477#comment:88
https://trac.sagemath.org/ticket/7477#comment:88
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.12.beta1</em>
</li>
</ul>
TicketncohenSat, 17 Aug 2013 07:25:07 GMT
https://trac.sagemath.org/ticket/7477#comment:89
https://trac.sagemath.org/ticket/7477#comment:89
<p>
Wow. Sooooo it's in Sage now <code>:-P</code>
</p>
Ticket