Sage: Ticket #14514: A constructor for the Brouwer-Haemers graph
https://trac.sagemath.org/ticket/14514
<p>
As the title says !
</p>
<p>
<a class="ext-link" href="http://www.win.tue.nl/~aeb/graphs/Brouwer-Haemers.html"><span class="icon"></span>http://www.win.tue.nl/~aeb/graphs/Brouwer-Haemers.html</a>
</p>
<p>
Nathann
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/14514
Trac 1.1.6ncohenWed, 01 May 2013 13:12:02 GMTstatus changed
https://trac.sagemath.org/ticket/14514#comment:1
https://trac.sagemath.org/ticket/14514#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
With a Sexyyyyyyyyyyyyyyyy embedding <code>:-P</code>
</p>
<p>
Nathann
</p>
TicketaziWed, 01 May 2013 16:19:40 GMT
https://trac.sagemath.org/ticket/14514#comment:2
https://trac.sagemath.org/ticket/14514#comment:2
<p>
Hello!!
</p>
<p>
The patch is fine! I would only add the following test.
</p>
<pre class="wiki">The graph has eigenvalues 20,2,-7
sage: set(G.spectrum()) == set([20,2,-7])
True
</pre><p>
So that we have as much new tests as possible! Otherwise the whole testing of the graph theory module finishes too quickly !!!
</p>
<p>
BTW. I was wondering if its time to redesign this graph database thing. If we keep adding "specific" graphs the codebase will explode with code that basically just constructs new objects.
</p>
<p>
One thing that I would suggest for all fixed graphs, compute their sparse6/graph6 string (whichever is shorter) and simply have Graph(thestring) in the given method?
</p>
<p>
Or perhaps have a data file with graphs/sparse6 strings/layouts and load that at runtime or something?
</p>
<p>
What do you think ??
</p>
TicketncohenWed, 01 May 2013 18:00:35 GMT
https://trac.sagemath.org/ticket/14514#comment:3
https://trac.sagemath.org/ticket/14514#comment:3
<p>
Yoooooooooo !!
</p>
<blockquote class="citation">
<p>
The patch is fine! I would only add the following test.
</p>
</blockquote>
<p>
Done !
</p>
<blockquote class="citation">
<p>
So that we have as much new tests as possible! Otherwise the whole testing of the graph theory module finishes too quickly !!!
</p>
</blockquote>
<p>
Ahem. Yeah, good point <code>:-P</code>
</p>
<blockquote class="citation">
<p>
BTW. I was wondering if its time to redesign this graph database thing. If we keep adding "specific" graphs the codebase will explode with code that basically just constructs new objects.
</p>
</blockquote>
<p>
Yep. I don't like it either.
</p>
<blockquote class="citation">
<p>
One thing that I would suggest for all fixed graphs, compute their sparse6/graph6 string (whichever is shorter) and simply have Graph(thestring) in the given method?
</p>
</blockquote>
<p>
Once again : I don't like the way we do things now either. But replacing the methods with sparse6 string means that our graphs are "proprietary" graphs how they are built is also a nice information to have around. Plus we lose layouts, the vertices' names (which may contain some information too). And it would only shorten the smallgraphs file, not the constructors that actually build families of graphs.
And we lose the doctests and documentation too, perhaps ?
</p>
<blockquote class="citation">
<p>
Or perhaps have a data file with graphs/sparse6 strings/layouts and load that at runtime or something?
</p>
<p>
What do you think ??
</p>
</blockquote>
<p>
I think that we will need to have an index of all sparse6 string of the graphs we have in Sage at some point. When we will want Sage to answer questions like "Have you ever seen this graph ?". But if we do have such an index, I think that we will still have the methods around at the same time. They don't have the same purpose.
</p>
<p>
But still, I don't like it either <code>:-P</code>
</p>
<p>
What do you think ?
</p>
<p>
Patch updated, by the way !
</p>
<p>
Nathann
</p>
TicketaziThu, 02 May 2013 08:38:43 GMT
https://trac.sagemath.org/ticket/14514#comment:4
https://trac.sagemath.org/ticket/14514#comment:4
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14514#comment:3" title="Comment 3">ncohen</a>:
</p>
<blockquote class="citation">
<p>
Yoooooooooo !!
</p>
<blockquote class="citation">
<p>
The patch is fine! I would only add the following test.
</p>
</blockquote>
<p>
Done !
</p>
<blockquote class="citation">
<p>
So that we have as much new tests as possible! Otherwise the whole testing of the graph theory module finishes too quickly !!!
</p>
</blockquote>
<p>
Ahem. Yeah, good point <code>:-P</code>
</p>
<blockquote class="citation">
<p>
BTW. I was wondering if its time to redesign this graph database thing. If we keep adding "specific" graphs the codebase will explode with code that basically just constructs new objects.
</p>
</blockquote>
<p>
Yep. I don't like it either.
</p>
<blockquote class="citation">
<p>
One thing that I would suggest for all fixed graphs, compute their sparse6/graph6 string (whichever is shorter) and simply have Graph(thestring) in the given method?
</p>
</blockquote>
<p>
Once again : I don't like the way we do things now either. But replacing the methods with sparse6 string means that our graphs are "proprietary" graphs how they are built is also a nice information to have around. Plus we lose layouts, the vertices' names (which may contain some information too). And it would only shorten the smallgraphs file, not the constructors that actually build families of graphs.
And we lose the doctests and documentation too, perhaps ?
</p>
<blockquote class="citation">
<p>
Or perhaps have a data file with graphs/sparse6 strings/layouts and load that at runtime or something?
</p>
<p>
What do you think ??
</p>
</blockquote>
</blockquote>
<p>
I agree. Somehow we want to balance usability and performance without making a bloated module. I don't see a solution for that though :-)
</p>
<blockquote class="citation">
<p>
I think that we will need to have an index of all sparse6 string of the graphs we have in Sage at some point. When we will want Sage to answer questions like "Have you ever seen this graph ?". But if we do have such an index, I think that we will still have the methods around at the same time. They don't have the same purpose.
</p>
</blockquote>
<p>
Yes. I was also thinking about that. To have some method of the form "nameGraph()" returning the name of a given graph (if known) or perhaps a construction of it in the sense "cartesian product of petersen and 5-cycle"
</p>
<p>
But this again sounds like a messy thing to implement :-)
</p>
<blockquote class="citation">
<p>
But still, I don't like it either <code>:-P</code>
</p>
<p>
What do you think ?
</p>
<p>
Patch updated, by the way !
</p>
</blockquote>
<p>
Patch is fine I'm gonna change the status to reflect that.
</p>
<blockquote class="citation">
<p>
Nathann
</p>
</blockquote>
TicketaziThu, 02 May 2013 08:41:39 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/14514#comment:5
https://trac.sagemath.org/ticket/14514#comment:5
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Jernej Azarija</em>
</li>
</ul>
TicketncohenThu, 02 May 2013 08:45:21 GMT
https://trac.sagemath.org/ticket/14514#comment:6
https://trac.sagemath.org/ticket/14514#comment:6
<blockquote class="citation">
<p>
Yes. I was also thinking about that. To have some method of the form "nameGraph()" returning the name of a given graph (if known) or perhaps a construction of it in the sense "cartesian product of petersen and 5-cycle"
</p>
</blockquote>
<p>
Well... For sure we will need a db at some point... For sure the function will answer "I've never seen this graph in my life" most of the time. But it is not necessarily complicated to implement..
</p>
<p>
Oh, and by the way we already have a small database of graphs from ISGCI <a class="closed ticket" href="https://trac.sagemath.org/ticket/14396" title="enhancement: ISGCI update, small graphs and recognition (closed: fixed)">#14396</a>.
</p>
<p>
It's a good thing to have around, though it also looks like there will be a lot of duplicated information if we ever do that ;-)
</p>
<blockquote class="citation">
<p>
Patch is fine I'm gonna change the status to reflect that.
</p>
</blockquote>
<p>
Thanks ! <code>:-)</code>
</p>
<p>
Nathann
</p>
TicketjdemeyerFri, 03 May 2013 08:10:23 GMTstatus changed; dependencies set
https://trac.sagemath.org/ticket/14514#comment:7
https://trac.sagemath.org/ticket/14514#comment:7
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>dependencies</strong>
set to <em>#14283</em>
</li>
</ul>
<p>
There is a conflict with <a class="closed ticket" href="https://trac.sagemath.org/ticket/14283" title="enhancement: M22 and Cameron graph constructors (closed: fixed)">#14283</a>.
</p>
TicketncohenFri, 03 May 2013 08:21:06 GMTstatus changed
https://trac.sagemath.org/ticket/14514#comment:8
https://trac.sagemath.org/ticket/14514#comment:8
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
TicketncohenFri, 03 May 2013 08:22:53 GMT
https://trac.sagemath.org/ticket/14514#comment:9
https://trac.sagemath.org/ticket/14514#comment:9
<p>
Done !
</p>
TicketjdemeyerFri, 03 May 2013 12:32:44 GMTstatus changed
https://trac.sagemath.org/ticket/14514#comment:10
https://trac.sagemath.org/ticket/14514#comment:10
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
<strong>PLEASE RUN DOCTESTS BEFORE SETTING A PATCH TO NEEDS_REVIEW OR POSITIVE_REVIEW</strong>
</p>
<pre class="wiki">sage -t devel/sage/sage/graphs/generators/smallgraphs.py
**********************************************************************
File "devel/sage/sage/graphs/generators/smallgraphs.py", line 1067, in sage.graphs.generators.smallgraphs.BrouwerHaemersGraph
Failed example:
set(G.spectrum()) == {20,2,-7}
Exception raised:
Traceback (most recent call last):
File "/mazur/release/merger/sage-5.10.beta2/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 466, in _run
self.execute(example, compiled, test.globs)
File "/mazur/release/merger/sage-5.10.beta2/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 825, in execute
exec compiled in globs
File "<doctest sage.graphs.generators.smallgraphs.BrouwerHaemersGraph[2]>", line 1, in <module>
set(G.spectrum()) == {Integer(20),Integer(2),-Integer(7)}
NameError: name 'G' is not defined
**********************************************************************
</pre>
TicketncohenFri, 03 May 2013 12:38:36 GMTattachment set
https://trac.sagemath.org/ticket/14514
https://trac.sagemath.org/ticket/14514
<ul>
<li><strong>attachment</strong>
set to <em>trac_14514.patch</em>
</li>
</ul>
TicketncohenFri, 03 May 2013 12:40:59 GMT
https://trac.sagemath.org/ticket/14514#comment:11
https://trac.sagemath.org/ticket/14514#comment:11
<p>
Fixed.......
</p>
<p>
Nathann
</p>
TicketncohenFri, 03 May 2013 12:43:25 GMTstatus changed
https://trac.sagemath.org/ticket/14514#comment:12
https://trac.sagemath.org/ticket/14514#comment:12
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
TicketjdemeyerWed, 08 May 2013 07:44:51 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/14514#comment:13
https://trac.sagemath.org/ticket/14514#comment:13
<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.10.beta2</em>
</li>
</ul>
Ticket