#14589: binary matrices, dense graphs, and faster is_strongly_regular
---------------------------------+----------------------------------
Reporter: ncohen | Owner: jason, ncohen, rlm
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.1
Component: graph theory | Resolution:
Keywords: | Merged in:
Authors: Nathann Cohen | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
Dependencies: #14805 | Stopgaps:
---------------------------------+----------------------------------
Comment (by ncohen):
Yooooooo !
> As promised I took the time to review your patch.
Thaaaaaaaaaaanks !! `;-)`
> The entire thing is easy to read and looks good. The fact that it was
not reviewed for so long makes me wonder if I am overlooking some non
obvious caveat?
The "someone else with do it" syndrome. And the fact that with git the
developpers as a whole probably suffered a loss of enthusiasm `:-P`
> 1. Line 67 - can we always assume that g.vertices() will return a sorted
list of vertices? Also, it looks like this can be written in a better way?
Unfortunately I think we can. Though I try not to rely on that. Did I do
it in this patch ?
The problem is that while the vertices are indeed "sorted" before being
returned, but that this sorting can be meaningless : the "natural"
ordering on the vertices may be a partial ordering from time to time (i.e.
when the vertices are set : `graphs.KneserGraph`) and well, that causes
bugs from time to time.
> 2. Line 48 function inline binary_matrix_free. Shouldn't you free
m.rows[1],...,m.rows[n_rows] as well?
No, because only `m.rows[0]` is allocated on line 39. All of `m.rows[i]`
point toward some part of this very long vector, but regardless of that if
you deallocate `m.rows[0]` the whole segment is deallocated.
> 3. Line 69 in inline binary_matrix_set0 the comment should really say
set matrix entry to 0
Oh. Right `:-PPPPPPPPP`
> Also off topic to this patch - am I the only one being tempted to change
the function edges() and set edge_labels to false as default?
`T_T`
I hate labels. And I also type `.edges(labels=False)` much more often than
`.edges()`. Well. I didn't try to change this kind of stuff because I
consider that it is just "my own use of graphs"... But well, if many of us
begin to hate labels/loops/multiple edges now that's a different story
`:-P`
The combinat/word guys may probably not agree, though... But really, I
have no idea. I just never did that because I don't want to change stuff
just because it makes it easier for me, without knowing how useful it can
be for others.
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/14589#comment:15>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
