Sage: Ticket #16272: redesign transversal designs
https://trac.sagemath.org/ticket/16272
<p>
The tickets <a class="closed ticket" href="https://trac.sagemath.org/ticket/15310" title="enhancement: Wilson's construction of Transversal Designs/Orthogonal Arrays/MOLS (closed: fixed)">#15310</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/16227" title="enhancement: Product construction of Transversal Designs (closed: fixed)">#16227</a> introduce a nice <code>availability</code> keywords to the function <code>transversal_design</code>. With <code>availability=True</code> the return value is the answer to the question "Does Sage know how to build a TD(k,n)"? Using <code>Unknown</code> from <code>sage.misc.unknown</code> we can turn the question into "Do we know mathematically that a TD(k,n) exist?" whose answer would be:
</p>
<ul><li><code>True</code> if Sage knows how to do it
</li><li><code>Unknwon</code> if neither Sage nor mathematics can help
</li><li><code>False</code> if we know mathematically that such construction does not exist
</li></ul><p>
As the semantic changes, we will also turn the keyword <code>availability</code> into <code>existence</code> (or maybe have both).
</p>
<p>
In the same ticket, we will include some of the known non existence of transversal designs:
</p>
<ul><li>The <a class="ext-link" href="http://en.wikipedia.org/wiki/Bruck%E2%80%93Ryser%E2%80%93Chowla_theorem"><span class="icon"></span>Bruck Ryser Chowla Theorm</a> gives the non-existence of many TD(n+1,n)
</li><li>C. Lam in Lam, "The Search for a Finite Projective Plane of Order 10" (1991) proved that TD(11,10) cannot exist
</li></ul><p>
And we might see later
</p>
<ul><li>there is some work (?) that shows that if $k$ is large enough then the existence of TD(k,n) implies the existence of TD(n+1,n) and so the non-existence results can percolate downwards in k
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/16272
Trac 1.1.6vdelecroixWed, 30 Apr 2014 14:14:15 GMTdescription changed
https://trac.sagemath.org/ticket/16272#comment:1
https://trac.sagemath.org/ticket/16272#comment:1
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/16272?action=diff&version=1">diff</a>)
</li>
</ul>
TicketvdelecroixWed, 30 Apr 2014 14:16:22 GMTdependencies set
https://trac.sagemath.org/ticket/16272#comment:2
https://trac.sagemath.org/ticket/16272#comment:2
<ul>
<li><strong>dependencies</strong>
set to <em>#15310, #16227</em>
</li>
</ul>
TicketvdelecroixWed, 30 Apr 2014 14:51:43 GMTdescription changed
https://trac.sagemath.org/ticket/16272#comment:3
https://trac.sagemath.org/ticket/16272#comment:3
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/16272?action=diff&version=3">diff</a>)
</li>
</ul>
TicketbrettThu, 01 May 2014 09:45:26 GMT
https://trac.sagemath.org/ticket/16272#comment:4
https://trac.sagemath.org/ticket/16272#comment:4
<p>
In case "Unkown" did you mean that Sage does not know how to do it (as currently implemented) but possibly Mathematics knows the answer? "Unknown" should ideally mean that mathematics does not know the answer but Sage development is likely to lag behind mathematics and I don't want users to think that if Sage does not know the answer that this implies that mathematics does not know the answer. I think this is all what you meant but the word "nor" confused me.
</p>
<p>
The Bruck-Ryser-Chowla Theorem (from Theorem 2.13 in Stinson's Book) shows that if $n \equiv 1,2 \bmod 4$
and there exists a prime $p \equiv 3 \bmod 4$ such that the largest power of $p$ that divides $n$ is odd. Then a TD$(n+1,n)$ does not exist.
</p>
<p>
A reference for the non existence of TD$(11,10)$ is
</p>
<pre class="wiki">@article {MR1018454,
AUTHOR = {Lam, C. W. H. and Thiel, L. and Swiercz, S.},
TITLE = {The nonexistence of finite projective planes of order {$10$}},
JOURNAL = {Canad. J. Math.},
FJOURNAL = {Canadian Journal of Mathematics. Journal Canadien de Math\'ematiques},
VOLUME = {41},
YEAR = {1989},
NUMBER = {6},
PAGES = {1117--1123},
ISSN = {0008-414X},
CODEN = {CJMAAB},
MRCLASS = {51E15 (51-04)},
MRNUMBER = {1018454 (90j:51008)},
MRREVIEWER = {J. C. Fisher},
DOI = {10.4153/CJM-1989-049-4},
}
</pre><p>
For the results I mentioned about sufficiently high $k$ implying TD$(n+1,n)$ exists, I will have to dig up the papers by Metz and review them to remember exactly how this works.
</p>
TicketbrettThu, 01 May 2014 09:45:57 GMT
https://trac.sagemath.org/ticket/16272#comment:5
https://trac.sagemath.org/ticket/16272#comment:5
<p>
Why did that BibTeX reference format so badly?
</p>
TicketncohenThu, 01 May 2014 09:54:10 GMT
https://trac.sagemath.org/ticket/16272#comment:6
https://trac.sagemath.org/ticket/16272#comment:6
<p>
Yo !
</p>
<blockquote class="citation">
<p>
In case "Unkown" did you mean that Sage does not know how to do it (as currently implemented) but possibly Mathematics knows the answer?
</p>
</blockquote>
<p>
The parameter will be called "existence". When you type <code>designs.transversal_design(k,n,existence=True)</code>
</p>
<p>
it will answer
1) <code>True</code> meaning "Sage can build it"
2) <code>Unknown</code> meaning "Sage can't build it, but it may exist"
3) <code>False</code> meaning "it does not exist"
</p>
<p>
Thus there is nothing there to mean "Mathematics knows it exists but Sage does not know how to build it". THouuuuuuugh if you know of something like that, the only responsible way to solve the problem is to implement the mathematical result <code>:-P</code>
</p>
<blockquote class="citation">
<p>
"Unknown" should ideally mean that mathematics does not know the answer but Sage development is likely to lag behind mathematics and I don't want users to think that if Sage does not know the answer that this implies that mathematics does not know the answer. I think this is all what you meant but the word "nor" confused me.
</p>
</blockquote>
<p>
Well, the word "unknown" means "Sage has no idea" <code>^^;</code>
</p>
<blockquote class="citation">
<p>
For the results I mentioned about sufficiently high $k$ implying TD$(n+1,n)$ exists, I will have to dig up the papers by Metz and review them to remember exactly how this works.
</p>
</blockquote>
<p>
Okayyyyyyyyy !
</p>
<blockquote class="citation">
<p>
Why did that BibTeX reference format so badly?
</p>
</blockquote>
<p>
Because some characters that you used are interpreted as typographic instructions. Wrap what you want to say with <code>{{{</code> and <code>}}}</code> to avoid that. I edited your comment above, so it looks fine now <code>;-)</code>
</p>
<p>
Nathann
</p>
TicketbrettThu, 01 May 2014 10:21:17 GMT
https://trac.sagemath.org/ticket/16272#comment:7
https://trac.sagemath.org/ticket/16272#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16272#comment:6" title="Comment 6">ncohen</a>:
</p>
<blockquote class="citation">
<p>
Yo !
</p>
<blockquote class="citation">
<p>
In case "Unkown" did you mean that Sage does not know how to do it (as currently implemented) but possibly Mathematics knows the answer?
</p>
</blockquote>
<p>
The parameter will be called "existence". When you type <code>designs.transversal_design(k,n,existence=True)</code>
</p>
<p>
it will answer
1) <code>True</code> meaning "Sage can build it"
2) <code>Unknown</code> meaning "Sage can't build it, but it may exist"
3) <code>False</code> meaning "it does not exist"
</p>
</blockquote>
<p>
Perfect, this is what I thought, but "nor" threw me off
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Why did that BibTeX reference format so badly?
</p>
</blockquote>
<p>
Because some characters that you used are interpreted as typographic instructions. Wrap what you want to say with <code>{{{</code> and <code>}}}</code> to avoid that. I edited your comment above, so it looks fine now <code>;-)</code>
</p>
</blockquote>
<p>
thanks
</p>
<p>
You put it in some kind of box, a "code block"?
</p>
TicketncohenThu, 01 May 2014 10:22:41 GMT
https://trac.sagemath.org/ticket/16272#comment:8
https://trac.sagemath.org/ticket/16272#comment:8
<blockquote class="citation">
<p>
You put it in some kind of box, a "code block"?
</p>
</blockquote>
<p>
Yep !
</p>
TicketncohenFri, 02 May 2014 13:32:30 GMTstatus changed; branch set
https://trac.sagemath.org/ticket/16272#comment:9
https://trac.sagemath.org/ticket/16272#comment:9
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>branch</strong>
set to <em>public/16272</em>
</li>
</ul>
<p>
Here it is ! I implemented the same thing so many times that even I am trying to see that unifying it all may help... <code>:-P</code>
</p>
<p>
Nathann
</p>
TicketncohenFri, 02 May 2014 13:32:39 GMTstatus changed
https://trac.sagemath.org/ticket/16272#comment:10
https://trac.sagemath.org/ticket/16272#comment:10
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
TicketgitFri, 02 May 2014 13:39:40 GMTcommit set
https://trac.sagemath.org/ticket/16272#comment:11
https://trac.sagemath.org/ticket/16272#comment:11
<ul>
<li><strong>commit</strong>
set to <em>d678326f182432584dd0aaf99308038aa70961e5</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3fc79ba5be0b99e99593ff54bfa075a52cc5937c"><span class="icon"></span>3fc79ba</a></td><td><code>trac #15310: Merged into 6.2.rc1</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=054d2a2ffa441f675594a09b95cf943f41121b4c"><span class="icon"></span>054d2a2</a></td><td><code>trac #16227: Product construction of Transversal Designs</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a512ab154b21caaf74d6ccef2cf32bf67179eee9"><span class="icon"></span>a512ab1</a></td><td><code>trac #16227: Merged with updated #15310</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e62fae83fadf558f57548c6034e7ef8c48397c0a"><span class="icon"></span>e62fae8</a></td><td><code>corrected doctests + new doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4d6e964775e4d6734e677a9f281f56aee11bb696"><span class="icon"></span>4d6e964</a></td><td><code>trac #16227: Replace exception with booleans in the doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a46446f3b29690aa1245583639bb6b78b3d50dca"><span class="icon"></span>a46446f</a></td><td><code>trac #16231: Equivalence between OA/TD/MOLS</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a9dce705211f5593b926c0512a40ada05d978ff3"><span class="icon"></span>a9dce70</a></td><td><code>trac #16231: Merged with updated #16227</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8d8b928c909f4282903cf4bd561797cf736b6c3e"><span class="icon"></span>8d8b928</a></td><td><code>more documentation to orthogonal_arrays.py</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8257178acf8691b62a4092538bcaed47e73b3953"><span class="icon"></span>8257178</a></td><td><code>remove MOLS construction for prime powers + doc</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d678326f182432584dd0aaf99308038aa70961e5"><span class="icon"></span>d678326</a></td><td><code>trac #16272: Replacing availability by existence and forwarding the results between design constructors</code>
</td></tr></table>
TicketvdelecroixFri, 02 May 2014 13:54:31 GMT
https://trac.sagemath.org/ticket/16272#comment:12
https://trac.sagemath.org/ticket/16272#comment:12
<p>
Hi,
</p>
<p>
I would like to have a <code>verbose</code> or <code>message</code> option:
</p>
<pre class="wiki">sage: designs.transversal_design(7,6,existence=True)
False
sage: designs.transversal_design(7,6,existence=True,message=True)
(False, "By the Bruck-Ryser-Chowla theorem, no projective plane of order 6 exists")
</pre><p>
But it looks like we are implementing ourself the propagation of errors... What do you think?
</p>
<p>
Vincent
</p>
TicketncohenFri, 02 May 2014 13:58:25 GMT
https://trac.sagemath.org/ticket/16272#comment:13
https://trac.sagemath.org/ticket/16272#comment:13
<p>
Yo !
</p>
<blockquote class="citation">
<p>
But it looks like we are implementing ourself the propagation of errors... What do you think?
</p>
</blockquote>
<p>
HMmm... Well, to me this is similar to the "construction path" feature. You know, the feature to tell you how to prove formally that a design exists with theorems ? There you want a "formal proof" that it cannot be done.
</p>
<p>
The thing is that right now we have no recursive proof of non-existence... But I still believe that it is too early to implement stuff like that.
</p>
<p>
Nathann
</p>
TicketncohenFri, 02 May 2014 21:04:12 GMTcomponent changed
https://trac.sagemath.org/ticket/16272#comment:14
https://trac.sagemath.org/ticket/16272#comment:14
<ul>
<li><strong>component</strong>
changed from <em>PLEASE CHANGE</em> to <em>combinatorics</em>
</li>
</ul>
TicketvdelecroixFri, 02 May 2014 22:54:33 GMT
https://trac.sagemath.org/ticket/16272#comment:15
https://trac.sagemath.org/ticket/16272#comment:15
<p>
Hello,
</p>
<p>
Here it comes!
</p>
<p>
Ok. I mainly did two things
</p>
<ul><li>implement my own <code>projective_space_as_OA</code> inside <code>orthogonal_arrays.py</code>. This might sound weird but the current implementation of <code>ProjectiveSpaceDesign</code> is slow and scarry me (I plan to clean it soon in a ticket).
</li><li>implement a beautiful <code>TD_existence</code> (I do not like the name, so I aks for suggestions here):
<pre class="wiki">sage: from sage.combinat.designs.orthogonal_arrays import TD_existence
sage: TD_existence(6)
TD(k,6)
exist for k in [ 2, 3]
unknown to exist for k in [ 4, 6]
not exist for k in [ 7,+infini]
</pre></li></ul><p>
I also correct some of the errors raised to get mainly <code>EmptySetError</code> and <code>NotImplementedError</code>.
</p>
<p>
Vincent
</p>
TicketgitFri, 02 May 2014 22:54:54 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:16
https://trac.sagemath.org/ticket/16272#comment:16
<ul>
<li><strong>commit</strong>
changed from <em>d678326f182432584dd0aaf99308038aa70961e5</em> to <em>ba058dd515c38084b4cdeacf530eda7b4563d211</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=840382927ec2232ef1d3ee430dac5bdfeb23790e"><span class="icon"></span>8403829</a></td><td><code>16272: more functionalities</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ba058dd515c38084b4cdeacf530eda7b4563d211"><span class="icon"></span>ba058dd</a></td><td><code>more doctests</code>
</td></tr></table>
TicketvdelecroixFri, 02 May 2014 22:55:57 GMTstatus changed
https://trac.sagemath.org/ticket/16272#comment:17
https://trac.sagemath.org/ticket/16272#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
As I do not have the reference for the third point mentioned in the ticket, I did not do it...
</p>
<p>
Vincent
</p>
TicketvdelecroixFri, 02 May 2014 22:57:08 GMTdescription changed
https://trac.sagemath.org/ticket/16272#comment:18
https://trac.sagemath.org/ticket/16272#comment:18
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/16272?action=diff&version=18">diff</a>)
</li>
</ul>
TicketncohenSat, 03 May 2014 06:46:19 GMT
https://trac.sagemath.org/ticket/16272#comment:19
https://trac.sagemath.org/ticket/16272#comment:19
<p>
Yo !
</p>
<blockquote class="citation">
<ul><li>implement my own <code>projective_space_as_OA</code> inside <code>orthogonal_arrays.py</code>. This might sound weird but the current implementation of <code>ProjectiveSpaceDesign</code> is slow and scarry me (I plan to clean it soon in a ticket).
</li></ul></blockquote>
<p>
THis thing has *NOTHING* to do here. Use the current <code>ProjectivePlaneDesigns</code>, and write another ticket to change the code if you don't like it. I agree that it is slow, but implementing the function TWICE (one that is slow, one that is hidden in an unrelated module) is not a way to solve it.
</p>
<blockquote class="citation">
<ul><li>implement a beautiful <code>TD_existence</code> (I do not like the name, so I aks for suggestions here):
</li></ul></blockquote>
<p>
Beautiful ? Is that a joke ? A function that PRINTS text ?
</p>
<p>
Get this thing out of here until a proper interface design is found. A function like that should RETURN values, not print stuff. Plus we have almost no negative result to advertise for the moment. Plus how the hell is an user going to find that ? We write Sage code here, not our own scripts !!!
</p>
<p>
If such a function is to ever be implemented, it will return a pair "lower bound, upper bound". The only interesting data in your three lines are TWO NUMBERS, and wrapping them in intervals for nothing, adding a 2 there and an infinity there looks fancy, but once more we work on a library here, not on your own code.
</p>
<blockquote class="citation">
<p>
I also correct some of the errors raised to get mainly <code>EmptySetError</code> and <code>NotImplementedError</code>.
</p>
</blockquote>
<p>
I will look at that when the problems above are solved....
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 May 2014 07:23:15 GMT
https://trac.sagemath.org/ticket/16272#comment:20
https://trac.sagemath.org/ticket/16272#comment:20
<p>
By the way... When we will have merged all constructions, there is a doctest that I would like to include in the doc, and it may partly do what you built this "TD_existence" thing for.
</p>
<p>
This is the table that I am trying to copy :
<a class="ext-link" href="http://books.google.fr/books?id=S9FA9rq1BgoC&lpg=PA175&ots=Do3xYTkMox&dq=%22mols%22%20of%20side%20%3C%2010000&pg=PA176#v=onepage&q=%22mols%22%20of%20side%20%3C%2010000&f=false"><span class="icon"></span>http://books.google.fr/books?id=S9FA9rq1BgoC&lpg=PA175&ots=Do3xYTkMox&dq=%22mols%22%20of%20side%20%3C%2010000&pg=PA176#v=onepage&q=%22mols%22%20of%20side%20%3C%2010000&f=false</a>
</p>
<p>
It is somehow "the reference". It gives you the maximum number of MOLS that are known to exist for a given order.
</p>
<p>
aaaaaaaand I have had the following code for a while to see where I was going:
</p>
<pre class="wiki">print " ",
for i in range(20):
print str(i)+" "*(4-len(str(i))),
print ""
print " "*4+"_"*20*5+"__"
for i in range(20*10):
if i%20==0:
print ""
print str(i)+" "*(4-len(str(i)))+"| ",
if i < 2:
k = 1
else:
for k in range(i+1):
if not designs.mutually_orthogonal_latin_squares(i,k,availability=True):
break
k = str(k-1)
print k+" "*(4-len(k)),
</pre><p>
This function is not meant to be called by users of course, I thought I would just make it a private function and call it just once, in a "# long" doctest. Here is what it produces, with <a class="closed ticket" href="https://trac.sagemath.org/ticket/16241" title="enhancement: New MOLS shared by Ian Wanless (closed: fixed)">#16241</a> (and all its dependencies) applied.
</p>
<pre class="wiki"> 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19
______________________________________________________________________________________________________
0 | 0 0 1 2 3 4 1 6 7 8 2 10 4 12 4 4 15 16 3 18
20 | 4 5 3 22 7 24 4 26 5 28 4 30 31 5 4 5 8 36 4 5
40 | 7 40 5 42 5 6 4 46 8 48 6 5 5 52 5 6 7 3 2 58
60 | 5 60 5 6 63 4 2 66 4 4 6 70 7 72 3 7 3 6 3 78
80 | 9 80 8 82 6 6 6 3 7 88 3 6 3 4 3 6 7 96 6 8
100 | 8 100 6 102 7 4 4 106 4 108 4 6 7 112 3 7 4 8 3 6
</pre><p>
It will be a nice way to advertise what Sage can do for TD/OA/MOLS <code>:-P</code>
</p>
<p>
Nathann
</p>
TicketgitSat, 03 May 2014 07:53:55 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:21
https://trac.sagemath.org/ticket/16272#comment:21
<ul>
<li><strong>commit</strong>
changed from <em>ba058dd515c38084b4cdeacf530eda7b4563d211</em> to <em>bcf917589a03c3e71a800a51d181ed24a96834f9</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=bcf917589a03c3e71a800a51d181ed24a96834f9"><span class="icon"></span>bcf9175</a></td><td><code>16272: reviewer comment 1</code>
</td></tr></table>
TicketvdelecroixSat, 03 May 2014 07:59:04 GMT
https://trac.sagemath.org/ticket/16272#comment:22
https://trac.sagemath.org/ticket/16272#comment:22
<p>
Hi Nathann,
</p>
<p>
You are right. I moved <code>TD_existence</code> as a doctest in <code>transversal_design</code> and now <code>projective_plane_as_OA</code> belongs to the module <code>block_design</code>.
</p>
<p>
The function <code>projective_plane_as_OA</code> is not similar to <code>ProjectivePlaneDesign</code> as it returns an OA (moreover, possibly not a complete OA(n+1,n,2)). To write it, I just pick the code from what was in OA and put it in an independent function. To my mind, we should keep the <code>ProjectivePlaneDesign</code> function as it was before.
</p>
<p>
Is it better like that?
</p>
<p>
Vincent
</p>
TicketncohenSat, 03 May 2014 08:29:54 GMT
https://trac.sagemath.org/ticket/16272#comment:23
https://trac.sagemath.org/ticket/16272#comment:23
<p>
Yo !
</p>
<blockquote class="citation">
<p>
You are right. I moved <code>TD_existence</code> as a doctest in <code>transversal_design</code> and now <code>projective_plane_as_OA</code> belongs to the module <code>block_design</code>.
</p>
</blockquote>
<p>
Okay, I really think that you are wrongly assuming that it is "always cool" to add tests, but I always check the long tests when I work on a file and adding "a minute there, a minute there" really is painful.
</p>
<p>
Let's talk about the code : you screamed when I removed the feature "give me the largest k possible" from the MOLS. It is now time to add it. To add the feature in all 3 constructors, because it costs absolutely nothing
</p>
<pre class="wiki">def OA(...):
if k is None:
all(OA(k+1,n,existence=True) for k in range(2,n+4))
return k
</pre><p>
And for all others
</p>
<pre class="wiki">def TD(...):
if k is None:
k = OA(None, n,existence = True)
</pre><p>
Then your doctest will be lighter and we will have the feature back. I can do it if you don't want to.
</p>
<p>
Also : would you mind not constructing the design for values of k which are smaller than the best possible ? Really, please don't make doctests too long. It takes times when you work on code, and you really over-check stuff. Remember that all results are checked before being returned anyway, so really this is a waste of (developer) time.
</p>
<blockquote class="citation">
<p>
The function <code>projective_plane_as_OA</code> is not similar to <code>ProjectivePlaneDesign</code> as it returns an OA (moreover, possibly not a complete OA(n+1,n,2)). To write it, I just pick the code from what was in OA and put it in an independent function. To my mind, we should keep the <code>ProjectivePlaneDesign</code> function as it was before.
</p>
</blockquote>
<p>
If you need to rewrite a different version of ProjectivePlaneDesign becaause it is to slow, is it safe to assume that ProjectivePlaneDesign should be changed ?
</p>
<p>
It does not make any sense to implement the same thing twice, Vincent, really. Of course if you have to call the ProjectivePlaneDesign you may have some conversion to do, but is that really a problem ? We must not implement this thing twice, really ....
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 03 May 2014 08:42:24 GMT
https://trac.sagemath.org/ticket/16272#comment:24
https://trac.sagemath.org/ticket/16272#comment:24
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16272#comment:23" title="Comment 23">ncohen</a>:
</p>
<blockquote class="citation">
<p>
Yo !
</p>
<blockquote class="citation">
<p>
You are right. I moved <code>TD_existence</code> as a doctest in <code>transversal_design</code> and now <code>projective_plane_as_OA</code> belongs to the module <code>block_design</code>.
</p>
</blockquote>
<p>
Okay, I really think that you are wrongly assuming that it is "always cool" to add tests, but I always check the long tests when I work on a file and adding "a minute there, a minute there" really is painful.
</p>
<p>
Let's talk about the code : you screamed when I removed the feature "give me the largest k possible" from the MOLS. It is now time to add it. To add the feature in all 3 constructors, because it costs absolutely nothing
</p>
<pre class="wiki">def OA(...):
if k is None:
all(OA(k+1,n,existence=True) for k in range(2,n+4))
return k
</pre><p>
And for all others
</p>
<pre class="wiki">def TD(...):
if k is None:
k = OA(None, n,existence = True)
</pre><p>
Then your doctest will be lighter and we will have the feature back. I can do it if you don't want to.
</p>
<p>
Also : would you mind not constructing the design for values of k which are smaller than the best possible ? Really, please don't make doctests too long. It takes times when you work on code, and you really over-check stuff. Remember that all results are checked before being returned anyway, so really this is a waste of (developer) time.
</p>
</blockquote>
<p>
Sounds good to me. I will do it and simplify the doctests accordingly.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
The function <code>projective_plane_as_OA</code> is not similar to <code>ProjectivePlaneDesign</code> as it returns an OA (moreover, possibly not a complete OA(n+1,n,2)). To write it, I just pick the code from what was in OA and put it in an independent function. To my mind, we should keep the <code>ProjectivePlaneDesign</code> function as it was before.
</p>
</blockquote>
<p>
If you need to rewrite a different version of ProjectivePlaneDesign because it is too slow, is it safe to assume that ProjectivePlaneDesign should be changed ?
</p>
</blockquote>
<p>
It is not only about conversion. ProjectivePlaneDesign is buggy: there is crazy argument "type" which makes specification a bit more complicated:
</p>
<ul><li>if type is None: do what you know is the best and return <strong>a</strong> projective plane
</li><li>if type is something: do whatever the construction tells you and return <strong>the</strong> corresponding projective plane
</li></ul><p>
Now if you incorporate the <code>who_asked</code> feature. It becomes crazy! I feel like <code>type</code> and <code>who_asked</code> are rather incompatible. To my mind, we should simply remove <code>ProjectivePlaneDesign</code> and have different functions (which raise only <code>ValueError</code> on bad entries) for each kind of projective planes we know about.
</p>
<blockquote class="citation">
<p>
It does not make any sense to implement the same thing twice, Vincent, really. Of course if you have to call the ProjectivePlaneDesign you may have some conversion to do, but is that really a problem ? We must not implement this thing twice, really ....
</p>
</blockquote>
<p>
What? I only moved <strong>your</strong> code from <code>orthogonal_array</code> to <code>projective_plane_as_OA</code>? Just kidding :-P
</p>
TicketvdelecroixSat, 03 May 2014 08:46:04 GMT
https://trac.sagemath.org/ticket/16272#comment:25
https://trac.sagemath.org/ticket/16272#comment:25
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16272#comment:24" title="Comment 24">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16272#comment:23" title="Comment 23">ncohen</a>:
</p>
<blockquote class="citation">
<p>
Yo !
</p>
<blockquote class="citation">
<p>
You are right. I moved <code>TD_existence</code> as a doctest in <code>transversal_design</code> and now <code>projective_plane_as_OA</code> belongs to the module <code>block_design</code>.
</p>
</blockquote>
<p>
Okay, I really think that you are wrongly assuming that it is "always cool" to add tests, but I always check the long tests when I work on a file and adding "a minute there, a minute there" really is painful.
</p>
<p>
Let's talk about the code : you screamed when I removed the feature "give me the largest k possible" from the MOLS. It is now time to add it. To add the feature in all 3 constructors, because it costs absolutely nothing
</p>
<pre class="wiki">def OA(...):
if k is None:
all(OA(k+1,n,existence=True) for k in range(2,n+4))
return k
</pre><p>
And for all others
</p>
<pre class="wiki">def TD(...):
if k is None:
k = OA(None, n,existence = True)
</pre><p>
Then your doctest will be lighter and we will have the feature back. I can do it if you don't want to.
</p>
<p>
Also : would you mind not constructing the design for values of k which are smaller than the best possible ? Really, please don't make doctests too long. It takes times when you work on code, and you really over-check stuff. Remember that all results are checked before being returned anyway, so really this is a waste of (developer) time.
</p>
</blockquote>
<p>
Sounds good to me. I will do it and simplify the doctests accordingly.
</p>
</blockquote>
<p>
Arrrrrrggggghhhhhhh. The <code>k</code> comes before the <code>n</code>. So there is no way to make <code>k</code> optional without making <code>n</code> optional... do you feel like having something like
</p>
<pre class="wiki">def transversal_design(k,n=None):
if n is None:
n = k
k = best_possible(n)
...
</pre>
TicketgitSat, 03 May 2014 09:08:14 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:26
https://trac.sagemath.org/ticket/16272#comment:26
<ul>
<li><strong>commit</strong>
changed from <em>bcf917589a03c3e71a800a51d181ed24a96834f9</em> to <em>ba89d70cda40900f7410574f2d0c3f987f5f47b8</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ba89d70cda40900f7410574f2d0c3f987f5f47b8"><span class="icon"></span>ba89d70</a></td><td><code>shorten doctest in orthogonal_arrays.py</code>
</td></tr></table>
TicketvdelecroixSat, 03 May 2014 09:10:48 GMT
https://trac.sagemath.org/ticket/16272#comment:27
https://trac.sagemath.org/ticket/16272#comment:27
<p>
Now long doctest takes 25 secs on orthogonal_arrays.py, must be good enough.
</p>
TicketncohenSat, 03 May 2014 09:12:38 GMT
https://trac.sagemath.org/ticket/16272#comment:28
https://trac.sagemath.org/ticket/16272#comment:28
<blockquote class="citation">
<p>
It is not only about conversion. ProjectivePlaneDesign is buggy: there is crazy argument "type" which makes specification a bit more complicated:
</p>
<ul><li>if type is None: do what you know is the best and return <strong>a</strong> projective plane
</li><li>if type is something: do whatever the construction tells you and return <strong>the</strong> corresponding projective plane
</li></ul></blockquote>
<p>
This ticket is no place to fix that.
</p>
<blockquote class="citation">
<p>
Now if you incorporate the <code>who_asked</code> feature. It becomes crazy! I feel like <code>type</code> and <code>who_asked</code> are rather incompatible. To my mind, we should simply remove <code>ProjectivePlaneDesign</code> and have different functions (which raise only <code>ValueError</code> on bad entries) for each kind of projective planes we know about.
</p>
</blockquote>
<p>
Vincent, the "type" keyword is not even USED. Remove it if it makes it easier for you.
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 03 May 2014 09:31:08 GMT
https://trac.sagemath.org/ticket/16272#comment:29
https://trac.sagemath.org/ticket/16272#comment:29
<p>
I can not use the current ProjectivePlaneDesign... here are the timings:
</p>
<pre class="wiki">sage: %timeit designs.ProjectivePlaneDesign(9)
1 loops, best of 3: 2.63 s per loop
sage: %timeit designs.ProjectivePlaneDesign(16) # WTF?
1 loops, best of 3: 42.3 s per loop
</pre><p>
2 secs are reasonable but 42 secs are not and I do not want to go further. Now with the straigthforward one I used it takes less than 1 sec:
</p>
<pre class="wiki">sage: from sage.combinat.designs.block_design import projective_plane_as_OA
sage: %timeit projective_plane_as_OA(9)
100 loops, best of 3: 5.95 ms per loop
sage: %timeit projective_plane_as_OA(16)
10 loops, best of 3: 39.5 ms per loop
</pre><p>
I do not want either to rewrite the whole ProjectivePlaneDesign as it is not the purpose of that ticket. So please find a reasonable solution.
</p>
TicketvdelecroixSat, 03 May 2014 09:46:29 GMTstatus, dependencies changed
https://trac.sagemath.org/ticket/16272#comment:30
https://trac.sagemath.org/ticket/16272#comment:30
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#15310, #16227</em> to <em>#15310, #16227, #16281</em>
</li>
</ul>
<p>
I agreed with Nathann that the best solution is to rewrite the projective planes... so here it is: <a class="closed ticket" href="https://trac.sagemath.org/ticket/16281" title="enhancement: redesign projective plane (closed: fixed)">#16281</a>
</p>
TicketgitSat, 03 May 2014 18:47:52 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:31
https://trac.sagemath.org/ticket/16272#comment:31
<ul>
<li><strong>commit</strong>
changed from <em>ba89d70cda40900f7410574f2d0c3f987f5f47b8</em> to <em>8830bb57327e1ca370287ca60b33e6d1b790a2d3</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=1ded990808f2a7903e56e0305de73082604e0984"><span class="icon"></span>1ded990</a></td><td><code>trac #16272: report appropriate timings for the long doctest</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=429d815568a0e4a90e4968e55d45548f2e0c7262"><span class="icon"></span>429d815</a></td><td><code>16281: rewrite projective planes (as 2-designs)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3e5628a2be9e09a15962dafba7a235d72b222888"><span class="icon"></span>3e5628a</a></td><td><code>16281: reviewer's comment 1</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0812a739febb7dcf70226bfa3c9009902b5f10a3"><span class="icon"></span>0812a73</a></td><td><code>trac #16281: Simplification</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=61dc86b759c59bbeb707bb6a35ce3e7b6b5d36aa"><span class="icon"></span>61dc86b</a></td><td><code>16281: long comment for the construction of the projective plane</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=51daa7fe6e225714fa666aa778ef08cc207264fb"><span class="icon"></span>51daa7f</a></td><td><code>trac #16281: correct a doctest</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=01722c48007a3e89de647d3717be4b2323c6ff17"><span class="icon"></span>01722c4</a></td><td><code>trac #16272: merge #16281 (first part)</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8830bb57327e1ca370287ca60b33e6d1b790a2d3"><span class="icon"></span>8830bb5</a></td><td><code>trac #16272: merge #16281 (part 2)</code>
</td></tr></table>
TicketvdelecroixSat, 03 May 2014 18:49:37 GMTstatus changed
https://trac.sagemath.org/ticket/16272#comment:32
https://trac.sagemath.org/ticket/16272#comment:32
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Hi Nathann,
</p>
<p>
Projective planes from <a class="closed ticket" href="https://trac.sagemath.org/ticket/16281" title="enhancement: redesign projective plane (closed: fixed)">#16281</a> is merged and now the code is shorter/cleaner.
</p>
<p>
There is no real check that a projective plane is valid as the ouptut of the function <code>projective_plane</code>... but that is not a real problem.
</p>
<p>
Vincent
</p>
TicketncohenSat, 03 May 2014 18:51:11 GMT
https://trac.sagemath.org/ticket/16272#comment:33
https://trac.sagemath.org/ticket/16272#comment:33
<p>
Well, not only we don't care if some step of the total computation is not checked, but none of them should be checked except the last one. We should never call design constructors from inside of the library without deacivating the checks othewise we really waste time.
</p>
<p>
Okay... Now I am going to play with git to see what exactly is being implemented in this ticket <code>T_T</code>
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 May 2014 18:58:29 GMT
https://trac.sagemath.org/ticket/16272#comment:34
https://trac.sagemath.org/ticket/16272#comment:34
<p>
Please Vincent. In the future, do not do ANYTHING in a merge commit except fix the conflicts.
</p>
<p>
It is HELL to know what else you changed.
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 May 2014 19:04:26 GMT
https://trac.sagemath.org/ticket/16272#comment:35
https://trac.sagemath.org/ticket/16272#comment:35
<p>
Okay, it is hell to know what exactly you did here. Can you tell me ?
</p>
<p>
For instance, what is the point of this who_asked in projective plane ?
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 May 2014 19:05:57 GMT
https://trac.sagemath.org/ticket/16272#comment:36
https://trac.sagemath.org/ticket/16272#comment:36
<p>
Okay Vincent. Unless you know a way for me to see exactly what you implemented which is not already in the other tickets, could you :
</p>
<p>
1) GO back to the history before the merge
</p>
<p>
2) Do the merge and only the merge
</p>
<p>
3) Implement stuff
</p>
<p>
Otherwise I just can't see what you did.
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 03 May 2014 19:15:59 GMT
https://trac.sagemath.org/ticket/16272#comment:37
https://trac.sagemath.org/ticket/16272#comment:37
<p>
Hi,
</p>
<p>
Before the merge, there <strong>was</strong> a <code>who_asked</code> argument in <code>projective_plane_as_OA</code>. Now it is an argument of <code>projective_plane</code>. In the second part of the merge I only fix some typo and doctests.
</p>
<p>
Perhaps I should go much before in history and then do the merge... let me try.
</p>
<p>
Vincent
</p>
TicketncohenSat, 03 May 2014 19:17:30 GMT
https://trac.sagemath.org/ticket/16272#comment:38
https://trac.sagemath.org/ticket/16272#comment:38
<p>
Oh. I found a way. Cool <code>O_O;;</code>
</p>
<p>
Vincent, I think there is absolutely no point to the implementation of the <code>who_asked</code> argument in projective plane, nor in projective plane calling OA back. No projective plane is known to exist for orders which are not prime powers, so this is going too far. We will adapt the code if this ever happens
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 May 2014 19:20:18 GMT
https://trac.sagemath.org/ticket/16272#comment:39
https://trac.sagemath.org/ticket/16272#comment:39
<p>
It seems I still cannot see all changes. I saw that though :
</p>
<pre class="wiki">+ #TODO: here there is no need to test everything as the function
+ # projective_plane answers it for you
</pre><p>
If you want this to be removed, you need an existence flag on <code>projective_plane</code>.
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 May 2014 19:21:17 GMT
https://trac.sagemath.org/ticket/16272#comment:40
https://trac.sagemath.org/ticket/16272#comment:40
<blockquote class="citation">
<p>
If you want this to be removed, you need an existence flag on <code>projective_plane</code>.
</p>
</blockquote>
<p>
Oh, we have it already. Cool. Well, you can replace it if you want, then.
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 03 May 2014 19:27:11 GMT
https://trac.sagemath.org/ticket/16272#comment:41
https://trac.sagemath.org/ticket/16272#comment:41
<p>
Time to eat... will have a look later tonight.
</p>
<p>
Vincent
</p>
TicketvdelecroixSat, 03 May 2014 19:50:19 GMT
https://trac.sagemath.org/ticket/16272#comment:42
https://trac.sagemath.org/ticket/16272#comment:42
<p>
It will be much cleaner if I start the merge from d678326 (your last commit)... let's go!
</p>
<p>
Vincent
</p>
TicketncohenSat, 03 May 2014 20:06:22 GMT
https://trac.sagemath.org/ticket/16272#comment:43
https://trac.sagemath.org/ticket/16272#comment:43
<blockquote class="citation">
<p>
It will be much cleaner if I start the merge from d678326 (your last commit)... let's go!
</p>
</blockquote>
<p>
+1
</p>
<p>
Nathann
</p>
TicketgitSat, 03 May 2014 20:19:22 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:44
https://trac.sagemath.org/ticket/16272#comment:44
<ul>
<li><strong>commit</strong>
changed from <em>8830bb57327e1ca370287ca60b33e6d1b790a2d3</em> to <em>5074eee1d802ef44c251252581684bf1dc1dc7d6</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e090f924519e899f5575f5fe8c178d44bf6530ef"><span class="icon"></span>e090f92</a></td><td><code>trac #16272: merge #16281</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9a221beebfda5fb2697481dad839b2cf7535a98d"><span class="icon"></span>9a221be</a></td><td><code>trac #16272: fix doctests</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=5074eee1d802ef44c251252581684bf1dc1dc7d6"><span class="icon"></span>5074eee</a></td><td><code>trac #16272: finer doctest to test the output of transversal_design</code>
</td></tr></table>
TicketvdelecroixSat, 03 May 2014 20:21:21 GMT
https://trac.sagemath.org/ticket/16272#comment:45
https://trac.sagemath.org/ticket/16272#comment:45
<p>
Hi Nathann,
</p>
<p>
This is a non fast forward push from your last commit. I integrated your remarks and the long doctest I created to check the output of <code>transversal_design</code>.
</p>
<p>
Vincent
</p>
TicketncohenSat, 03 May 2014 20:22:13 GMT
https://trac.sagemath.org/ticket/16272#comment:46
https://trac.sagemath.org/ticket/16272#comment:46
<p>
Thanks ! It will be easier to follow this way <code>:-)</code>
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 03 May 2014 20:25:29 GMT
https://trac.sagemath.org/ticket/16272#comment:47
https://trac.sagemath.org/ticket/16272#comment:47
<p>
Note that the doc fails to build
</p>
<pre class="wiki">[combinat ] /opt/sage/local/lib/python2.7/site-packages/sage/combinat/designs/orthogonal_arrays.py:docstring of sage.combinat.designs.orthogonal_arrays.find_wilson_decomposition:14: ERROR: Unexpected indentation.
[combinat ] :0: WARNING: Block quote ends without a blank line; unexpected unindent.
Error building the documentation.
Traceback (most recent call last):
File "/opt/sage/src/doc/common/builder.py", line 1477, in <module>
getattr(get_builder(name), type)()
File "/opt/sage/src/doc/common/builder.py", line 276, in _wrapper
getattr(get_builder(document), 'inventory')(*args, **kwds)
File "/opt/sage/src/doc/common/builder.py", line 487, in _wrapper
x.get(99999)
File "/opt/sage/local/lib/python/multiprocessing/pool.py", line 554, in get
raise self._value
OSError: [combinat ] /opt/sage/local/lib/python2.7/site-packages/sage/combinat/designs/orthogonal_arrays.py:docstring of sage.combinat.designs.orthogonal_arrays.find_wilson_decomposition:14: ERROR: Unexpected indentation.
</pre>
TicketncohenSat, 03 May 2014 20:39:13 GMT
https://trac.sagemath.org/ticket/16272#comment:48
https://trac.sagemath.org/ticket/16272#comment:48
<p>
Yooooooooooo !!
</p>
<p>
I understand that you do not want to call the same function twice, but would you mind behaving as if boolean results were already cached ? It really makes the code's flow easier to follow <code>:-/</code>
</p>
<p>
Otherwise everything looks right... If I may ask, I think it would also be better to not have this <code>TD = designs.transversal_design</code> in the doctest. Keeping the full writing also helps explaining to the users that all the TD can be built from <code>designs.transversal_design</code>.
</p>
<p>
Nathann
</p>
TicketncohenSat, 03 May 2014 21:26:35 GMT
https://trac.sagemath.org/ticket/16272#comment:49
https://trac.sagemath.org/ticket/16272#comment:49
<p>
Besides it's good to go.
</p>
<p>
Nathann
</p>
TicketgitSat, 03 May 2014 23:02:10 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:50
https://trac.sagemath.org/ticket/16272#comment:50
<ul>
<li><strong>commit</strong>
changed from <em>5074eee1d802ef44c251252581684bf1dc1dc7d6</em> to <em>d81f265637099f93495dafac54e2d354be0d66d5</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d81f265637099f93495dafac54e2d354be0d66d5"><span class="icon"></span>d81f265</a></td><td><code>trac #16272: ultimate doctest</code>
</td></tr></table>
TicketvdelecroixSat, 03 May 2014 23:07:58 GMT
https://trac.sagemath.org/ticket/16272#comment:51
https://trac.sagemath.org/ticket/16272#comment:51
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16272#comment:48" title="Comment 48">ncohen</a>:
</p>
<blockquote class="citation">
<p>
Yooooooooooo !!
</p>
<p>
I understand that you do not want to call the same function twice, but would you mind behaving as if boolean results were already cached ? It really makes the code's flow easier to follow <code>:-/</code>
</p>
</blockquote>
<p>
I do not care too much about calling twice the function. My main trouble was that in <code>OA</code> with parameters <code>n,k=n+1,t=2</code> we have equivalence with projective planes. So we need to let the projective plane do what it has to do (even if the answer is Unknown). Whereas for general <code>n,k,t=2</code> we might have something better but it still worth it to ask about a projective plane... If you have an idea to make it neat I let it to you.
</p>
<blockquote class="citation">
<p>
Otherwise everything looks right... If I may ask, I think it would also be better to not have this <code>TD = designs.transversal_design</code> in the doctest. Keeping the full writing also helps explaining to the users that all the TD can be built from <code>designs.transversal_design</code>.
</p>
</blockquote>
<p>
Cool (Note that the doc problem is fixed).
</p>
<p>
We leave the idea of having an optional <code>k=None</code>? Might be an independent ticket anyway.
</p>
<p>
Vincent
</p>
TicketncohenSun, 04 May 2014 06:40:36 GMT
https://trac.sagemath.org/ticket/16272#comment:52
https://trac.sagemath.org/ticket/16272#comment:52
<blockquote class="citation">
<p>
I do not care too much about calling twice the function.
</p>
</blockquote>
<p>
Cool. Then I changed the code to have a nice "if/elif" structure. In some very specific cases the projective plane function is called three times, but
</p>
<p>
1) This is only when the answer has been found, so it does not happens often per call
</p>
<p>
2) A tricky replacement is possible too, as
</p>
<pre class="wiki"> elif (t == 2 and
(projective_plane(n, existence=True) or
(k == n+1 and projective_plane(n, existence=True) is False))):
</pre><p>
is equivalent to:
</p>
<pre class="wiki"> elif (t == 2 and
(projective_plane(n, existence=True) in (True,k<n+1))):
</pre><p>
This being said the code of these things compared to the cost of building the actual array ...
</p>
<blockquote class="citation">
<p>
We leave the idea of having an optional <code>k=None</code>? Might be an independent ticket anyway.
</p>
</blockquote>
<p>
Yep, let's close this one and get <a class="closed ticket" href="https://trac.sagemath.org/ticket/16277" title="enhancement: MOLS constructions rom the Handbook of Combinatorial Designs (closed: fixed)">#16277</a> in. I will merge it in a second.
</p>
<p>
Nathann
</p>
TicketgitSun, 04 May 2014 06:41:46 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:53
https://trac.sagemath.org/ticket/16272#comment:53
<ul>
<li><strong>commit</strong>
changed from <em>d81f265637099f93495dafac54e2d354be0d66d5</em> to <em>47798d2d54221c0bd0ce6aa75d1900496a091e2f</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=47798d2d54221c0bd0ce6aa75d1900496a091e2f"><span class="icon"></span>47798d2</a></td><td><code>trac #16272: simplifying the structure of orthogonal_array</code>
</td></tr></table>
TicketvdelecroixSun, 04 May 2014 09:03:53 GMT
https://trac.sagemath.org/ticket/16272#comment:54
https://trac.sagemath.org/ticket/16272#comment:54
<p>
Hi,
</p>
<p>
This is what I did not want to do... if <code>k=n+1</code> and <code>projective_plane</code> answers <code>Unknown</code> you are going to try other constructions. In <a class="ticket" href="https://trac.sagemath.org/ticket/16272#comment:38" title="Comment 38">comment:38</a> you said that it was useless... But anyway, it does not cost too much and if by chance we got one we would be happy!
</p>
<p>
Who is the author? who is the reviewer? who sets to positive review?
</p>
<p>
Vincent
</p>
TicketncohenSun, 04 May 2014 09:06:39 GMT
https://trac.sagemath.org/ticket/16272#comment:55
https://trac.sagemath.org/ticket/16272#comment:55
<p>
Yo !
</p>
<blockquote class="citation">
<p>
This is what I did not want to do... if <code>k=n+1</code> and <code>projective_plane</code> answers <code>Unknown</code> you are going to try other constructions. In <a class="ticket" href="https://trac.sagemath.org/ticket/16272#comment:38" title="Comment 38">comment:38</a> you said that it was useless...
</p>
</blockquote>
<p>
Oh right. But that was to avoid having a <code>who_asks</code> parameter in <code>projective_plane</code>. But anyway, it does not cost too much and if by chance we got one we would be happy! Ahahaha. Yeah. Perhaps somebody overlooked a very simple case of recursive decomposition <code>;-)</code>
</p>
<blockquote class="citation">
<p>
Who is the author? who is the reviewer? who sets to positive review?
</p>
</blockquote>
<p>
Well, let's do half/half. Positive review with both reviewers and both authors. By the way I just finished the patch for <code>k=None</code> and I added the MOLS table from my dreams. The last dependency of the constructions will also be reviewed in a second <code>;-)</code>
</p>
<p>
Nathann
</p>
TicketncohenSun, 04 May 2014 09:07:01 GMTstatus, author changed; reviewer set
https://trac.sagemath.org/ticket/16272#comment:56
https://trac.sagemath.org/ticket/16272#comment:56
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Nathann Cohen, Vincent Delecroix</em>
</li>
<li><strong>author</strong>
changed from <em>Vincent Delecroix</em> to <em>Nathann Cohen, Vincent Delecroix</em>
</li>
</ul>
TicketgitSun, 04 May 2014 16:13:34 GMTstatus, commit changed
https://trac.sagemath.org/ticket/16272#comment:57
https://trac.sagemath.org/ticket/16272#comment:57
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
changed from <em>47798d2d54221c0bd0ce6aa75d1900496a091e2f</em> to <em>d34b012758319ebcea5c0a0e17e6481ad5ded481</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e86d26cb8126f3e557cc6a27b1727e2055f67321"><span class="icon"></span>e86d26c</a></td><td><code>trac #15310: Merged with 6.2.rc2</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=5cfee91760bd3b73d19ebc1318a94861d62b226f"><span class="icon"></span>5cfee91</a></td><td><code>trac #16227: Merged with updated #15310</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d34b012758319ebcea5c0a0e17e6481ad5ded481"><span class="icon"></span>d34b012</a></td><td><code>trac #16272: Merged with updated #16227</code>
</td></tr></table>
TicketncohenSun, 04 May 2014 16:13:49 GMTstatus changed
https://trac.sagemath.org/ticket/16272#comment:58
https://trac.sagemath.org/ticket/16272#comment:58
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Still merging ...
</p>
<p>
Nathann
</p>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/16272#comment:59
https://trac.sagemath.org/ticket/16272#comment:59
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
TicketvbraunThu, 08 May 2014 08:04:17 GMTstatus changed
https://trac.sagemath.org/ticket/16272#comment:60
https://trac.sagemath.org/ticket/16272#comment:60
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Merge conflict
</p>
TicketgitThu, 08 May 2014 08:22:36 GMTcommit changed
https://trac.sagemath.org/ticket/16272#comment:61
https://trac.sagemath.org/ticket/16272#comment:61
<ul>
<li><strong>commit</strong>
changed from <em>d34b012758319ebcea5c0a0e17e6481ad5ded481</em> to <em>c127a6d52c126483966dbc5faa6436d699a0d4a4</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d051cf4f127aa7c23c0b4a09ca12d4ecaf2caee6"><span class="icon"></span>d051cf4</a></td><td><code>ValueError -> EmptySetError in latin_squares</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9e5e94f648113d6b824039651bcb8d98144e01b2"><span class="icon"></span>9e5e94f</a></td><td><code>trac #16231: Merged with updated #16227</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c127a6d52c126483966dbc5faa6436d699a0d4a4"><span class="icon"></span>c127a6d</a></td><td><code>trac #16272: Merged with updated #16231</code>
</td></tr></table>
TicketncohenThu, 08 May 2014 08:23:04 GMTstatus, dependencies changed
https://trac.sagemath.org/ticket/16272#comment:62
https://trac.sagemath.org/ticket/16272#comment:62
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#15310, #16227, #16281</em> to <em>#16231</em>
</li>
</ul>
TicketgitSat, 10 May 2014 21:28:29 GMTstatus, commit changed
https://trac.sagemath.org/ticket/16272#comment:63
https://trac.sagemath.org/ticket/16272#comment:63
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
changed from <em>c127a6d52c126483966dbc5faa6436d699a0d4a4</em> to <em>e2749b3db4cd78fc5e9ea8219d4cdf895b283465</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e2749b3db4cd78fc5e9ea8219d4cdf895b283465"><span class="icon"></span>e2749b3</a></td><td><code>trac #16272: Merged with 6.3.beta0</code>
</td></tr></table>
TicketncohenSat, 10 May 2014 21:28:50 GMTstatus changed
https://trac.sagemath.org/ticket/16272#comment:64
https://trac.sagemath.org/ticket/16272#comment:64
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketvbraunMon, 12 May 2014 11:34:39 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/16272#comment:65
https://trac.sagemath.org/ticket/16272#comment:65
<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>branch</strong>
changed from <em>public/16272</em> to <em>e2749b3db4cd78fc5e9ea8219d4cdf895b283465</em>
</li>
</ul>
Ticket