Opened 7 years ago
Closed 7 years ago
#16272 closed enhancement (fixed)
redesign transversal designs
Reported by:  vdelecroix  Owned by:  Vincent Delecroix 

Priority:  major  Milestone:  sage6.3 
Component:  combinatorics  Keywords:  designs, orthogona arrays 
Cc:  ncohen, brett  Merged in:  
Authors:  Nathann Cohen, Vincent Delecroix  Reviewers:  Nathann Cohen, Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  e2749b3 (Commits, GitHub, GitLab)  Commit:  e2749b3db4cd78fc5e9ea8219d4cdf895b283465 
Dependencies:  #16231  Stopgaps: 
Description (last modified by )
The tickets #15310 and #16227 introduce a nice availability
keywords to the function transversal_design
. With availability=True
the return value is the answer to the question "Does Sage know how to build a TD(k,n)"? Using Unknown
from sage.misc.unknown
we can turn the question into "Do we know mathematically that a TD(k,n) exist?" whose answer would be:
True
if Sage knows how to do itUnknwon
if neither Sage nor mathematics can helpFalse
if we know mathematically that such construction does not exist
As the semantic changes, we will also turn the keyword availability
into existence
(or maybe have both).
In the same ticket, we will include some of the known non existence of transversal designs:
 The Bruck Ryser Chowla Theorm gives the nonexistence of many TD(n+1,n)
 C. Lam in Lam, "The Search for a Finite Projective Plane of Order 10" (1991) proved that TD(11,10) cannot exist
And we might see later
 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 nonexistence results can percolate downwards in k
Change History (65)
comment:1 Changed 7 years ago by
 Description modified (diff)
comment:2 Changed 7 years ago by
 Dependencies set to #15310, #16227
comment:3 Changed 7 years ago by
 Description modified (diff)
comment:4 followup: ↓ 6 Changed 7 years ago by
comment:5 Changed 7 years ago by
Why did that BibTeX reference format so badly?
comment:6 in reply to: ↑ 4 ; followup: ↓ 7 Changed 7 years ago by
Yo !
In case "Unkown" did you mean that Sage does not know how to do it (as currently implemented) but possibly Mathematics knows the answer?
The parameter will be called "existence". When you type designs.transversal_design(k,n,existence=True)
it will answer
1) True
meaning "Sage can build it"
2) Unknown
meaning "Sage can't build it, but it may exist"
3) False
meaning "it does not exist"
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 :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.
Well, the word "unknown" means "Sage has no idea" ^^;
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.
Okayyyyyyyyy !
Why did that BibTeX reference format so badly?
Because some characters that you used are interpreted as typographic instructions. Wrap what you want to say with {{{
and }}}
to avoid that. I edited your comment above, so it looks fine now ;)
Nathann
comment:7 in reply to: ↑ 6 ; followup: ↓ 8 Changed 7 years ago by
Replying to ncohen:
Yo !
In case "Unkown" did you mean that Sage does not know how to do it (as currently implemented) but possibly Mathematics knows the answer?
The parameter will be called "existence". When you type
designs.transversal_design(k,n,existence=True)
it will answer 1)
True
meaning "Sage can build it" 2)Unknown
meaning "Sage can't build it, but it may exist" 3)False
meaning "it does not exist"
Perfect, this is what I thought, but "nor" threw me off
Why did that BibTeX reference format so badly?
Because some characters that you used are interpreted as typographic instructions. Wrap what you want to say with
{{{
and}}}
to avoid that. I edited your comment above, so it looks fine now;)
thanks
You put it in some kind of box, a "code block"?
comment:8 in reply to: ↑ 7 Changed 7 years ago by
You put it in some kind of box, a "code block"?
Yep !
comment:9 Changed 7 years ago by
 Branch set to public/16272
 Status changed from new to needs_review
Here it is ! I implemented the same thing so many times that even I am trying to see that unifying it all may help... :P
Nathann
comment:10 Changed 7 years ago by
 Status changed from needs_review to needs_work
comment:11 Changed 7 years ago by
 Commit set to d678326f182432584dd0aaf99308038aa70961e5
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
3fc79ba  trac #15310: Merged into 6.2.rc1

054d2a2  trac #16227: Product construction of Transversal Designs

a512ab1  trac #16227: Merged with updated #15310

e62fae8  corrected doctests + new doctests

4d6e964  trac #16227: Replace exception with booleans in the doctests

a46446f  trac #16231: Equivalence between OA/TD/MOLS

a9dce70  trac #16231: Merged with updated #16227

8d8b928  more documentation to orthogonal_arrays.py

8257178  remove MOLS construction for prime powers + doc

d678326  trac #16272: Replacing availability by existence and forwarding the results between design constructors

comment:12 followup: ↓ 13 Changed 7 years ago by
Hi,
I would like to have a verbose
or message
option:
sage: designs.transversal_design(7,6,existence=True) False sage: designs.transversal_design(7,6,existence=True,message=True) (False, "By the BruckRyserChowla theorem, no projective plane of order 6 exists")
But it looks like we are implementing ourself the propagation of errors... What do you think?
Vincent
comment:13 in reply to: ↑ 12 Changed 7 years ago by
Yo !
But it looks like we are implementing ourself the propagation of errors... What do you think?
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.
The thing is that right now we have no recursive proof of nonexistence... But I still believe that it is too early to implement stuff like that.
Nathann
comment:14 Changed 7 years ago by
 Component changed from PLEASE CHANGE to combinatorics
comment:15 followup: ↓ 19 Changed 7 years ago by
Hello,
Here it comes!
Ok. I mainly did two things
 implement my own
projective_space_as_OA
insideorthogonal_arrays.py
. This might sound weird but the current implementation ofProjectiveSpaceDesign
is slow and scarry me (I plan to clean it soon in a ticket).  implement a beautiful
TD_existence
(I do not like the name, so I aks for suggestions here):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]
I also correct some of the errors raised to get mainly EmptySetError
and NotImplementedError
.
Vincent
comment:16 Changed 7 years ago by
 Commit changed from d678326f182432584dd0aaf99308038aa70961e5 to ba058dd515c38084b4cdeacf530eda7b4563d211
comment:17 Changed 7 years ago by
 Status changed from needs_work to needs_review
As I do not have the reference for the third point mentioned in the ticket, I did not do it...
Vincent
comment:18 Changed 7 years ago by
 Description modified (diff)
comment:19 in reply to: ↑ 15 Changed 7 years ago by
Yo !
 implement my own
projective_space_as_OA
insideorthogonal_arrays.py
. This might sound weird but the current implementation ofProjectiveSpaceDesign
is slow and scarry me (I plan to clean it soon in a ticket).
THis thing has *NOTHING* to do here. Use the current ProjectivePlaneDesigns
, 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.
 implement a beautiful
TD_existence
(I do not like the name, so I aks for suggestions here):
Beautiful ? Is that a joke ? A function that PRINTS text ?
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 !!!
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.
I also correct some of the errors raised to get mainly
EmptySetError
andNotImplementedError
.
I will look at that when the problems above are solved....
Nathann
comment:20 Changed 7 years ago by
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.
This is the table that I am trying to copy : 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
It is somehow "the reference". It gives you the maximum number of MOLS that are known to exist for a given order.
aaaaaaaand I have had the following code for a while to see where I was going:
print " ", for i in range(20): print str(i)+" "*(4len(str(i))), print "" print " "*4+"_"*20*5+"__" for i in range(20*10): if i%20==0: print "" print str(i)+" "*(4len(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(k1) print k+" "*(4len(k)),
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 #16241 (and all its dependencies) applied.
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
It will be a nice way to advertise what Sage can do for TD/OA/MOLS :P
Nathann
comment:21 Changed 7 years ago by
 Commit changed from ba058dd515c38084b4cdeacf530eda7b4563d211 to bcf917589a03c3e71a800a51d181ed24a96834f9
Branch pushed to git repo; I updated commit sha1. New commits:
bcf9175  16272: reviewer comment 1

comment:22 followup: ↓ 23 Changed 7 years ago by
Hi Nathann,
You are right. I moved TD_existence
as a doctest in transversal_design
and now projective_plane_as_OA
belongs to the module block_design
.
The function projective_plane_as_OA
is not similar to ProjectivePlaneDesign
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 ProjectivePlaneDesign
function as it was before.
Is it better like that?
Vincent
comment:23 in reply to: ↑ 22 ; followup: ↓ 24 Changed 7 years ago by
Yo !
You are right. I moved
TD_existence
as a doctest intransversal_design
and nowprojective_plane_as_OA
belongs to the moduleblock_design
.
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.
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
def OA(...): if k is None: all(OA(k+1,n,existence=True) for k in range(2,n+4)) return k
And for all others
def TD(...): if k is None: k = OA(None, n,existence = True)
Then your doctest will be lighter and we will have the feature back. I can do it if you don't want to.
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 overcheck stuff. Remember that all results are checked before being returned anyway, so really this is a waste of (developer) time.
The function
projective_plane_as_OA
is not similar toProjectivePlaneDesign
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 theProjectivePlaneDesign
function as it was before.
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 ?
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 ....
Nathann
comment:24 in reply to: ↑ 23 ; followups: ↓ 25 ↓ 28 Changed 7 years ago by
Replying to ncohen:
Yo !
You are right. I moved
TD_existence
as a doctest intransversal_design
and nowprojective_plane_as_OA
belongs to the moduleblock_design
.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.
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
def OA(...): if k is None: all(OA(k+1,n,existence=True) for k in range(2,n+4)) return kAnd for all others
def TD(...): if k is None: k = OA(None, n,existence = True)Then your doctest will be lighter and we will have the feature back. I can do it if you don't want to.
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 overcheck stuff. Remember that all results are checked before being returned anyway, so really this is a waste of (developer) time.
Sounds good to me. I will do it and simplify the doctests accordingly.
The function
projective_plane_as_OA
is not similar toProjectivePlaneDesign
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 theProjectivePlaneDesign
function as it was before.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 ?
It is not only about conversion. ProjectivePlaneDesign is buggy: there is crazy argument "type" which makes specification a bit more complicated:
 if type is None: do what you know is the best and return a projective plane
 if type is something: do whatever the construction tells you and return the corresponding projective plane
Now if you incorporate the who_asked
feature. It becomes crazy! I feel like type
and who_asked
are rather incompatible. To my mind, we should simply remove ProjectivePlaneDesign
and have different functions (which raise only ValueError
on bad entries) for each kind of projective planes we know about.
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 ....
What? I only moved your code from orthogonal_array
to projective_plane_as_OA
? Just kidding :P
comment:25 in reply to: ↑ 24 Changed 7 years ago by
Replying to vdelecroix:
Replying to ncohen:
Yo !
You are right. I moved
TD_existence
as a doctest intransversal_design
and nowprojective_plane_as_OA
belongs to the moduleblock_design
.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.
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
def OA(...): if k is None: all(OA(k+1,n,existence=True) for k in range(2,n+4)) return kAnd for all others
def TD(...): if k is None: k = OA(None, n,existence = True)Then your doctest will be lighter and we will have the feature back. I can do it if you don't want to.
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 overcheck stuff. Remember that all results are checked before being returned anyway, so really this is a waste of (developer) time.
Sounds good to me. I will do it and simplify the doctests accordingly.
Arrrrrrggggghhhhhhh. The k
comes before the n
. So there is no way to make k
optional without making n
optional... do you feel like having something like
def transversal_design(k,n=None): if n is None: n = k k = best_possible(n) ...
comment:26 Changed 7 years ago by
 Commit changed from bcf917589a03c3e71a800a51d181ed24a96834f9 to ba89d70cda40900f7410574f2d0c3f987f5f47b8
Branch pushed to git repo; I updated commit sha1. New commits:
ba89d70  shorten doctest in orthogonal_arrays.py

comment:27 Changed 7 years ago by
Now long doctest takes 25 secs on orthogonal_arrays.py, must be good enough.
comment:28 in reply to: ↑ 24 Changed 7 years ago by
It is not only about conversion. ProjectivePlaneDesign is buggy: there is crazy argument "type" which makes specification a bit more complicated:
 if type is None: do what you know is the best and return a projective plane
 if type is something: do whatever the construction tells you and return the corresponding projective plane
This ticket is no place to fix that.
Now if you incorporate the
who_asked
feature. It becomes crazy! I feel liketype
andwho_asked
are rather incompatible. To my mind, we should simply removeProjectivePlaneDesign
and have different functions (which raise onlyValueError
on bad entries) for each kind of projective planes we know about.
Vincent, the "type" keyword is not even USED. Remove it if it makes it easier for you.
Nathann
comment:29 Changed 7 years ago by
I can not use the current ProjectivePlaneDesign... here are the timings:
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
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:
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
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.
comment:30 Changed 7 years ago by
 Dependencies changed from #15310, #16227 to #15310, #16227, #16281
 Status changed from needs_review to needs_work
I agreed with Nathann that the best solution is to rewrite the projective planes... so here it is: #16281
comment:31 Changed 7 years ago by
 Commit changed from ba89d70cda40900f7410574f2d0c3f987f5f47b8 to 8830bb57327e1ca370287ca60b33e6d1b790a2d3
Branch pushed to git repo; I updated commit sha1. New commits:
1ded990  trac #16272: report appropriate timings for the long doctest

429d815  16281: rewrite projective planes (as 2designs)

3e5628a  16281: reviewer's comment 1

0812a73  trac #16281: Simplification

61dc86b  16281: long comment for the construction of the projective plane

51daa7f  trac #16281: correct a doctest

01722c4  trac #16272: merge #16281 (first part)

8830bb5  trac #16272: merge #16281 (part 2)

comment:32 Changed 7 years ago by
 Status changed from needs_work to needs_review
Hi Nathann,
Projective planes from #16281 is merged and now the code is shorter/cleaner.
There is no real check that a projective plane is valid as the ouptut of the function projective_plane
... but that is not a real problem.
Vincent
comment:33 Changed 7 years ago by
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.
Okay... Now I am going to play with git to see what exactly is being implemented in this ticket T_T
Nathann
comment:34 Changed 7 years ago by
Please Vincent. In the future, do not do ANYTHING in a merge commit except fix the conflicts.
It is HELL to know what else you changed.
Nathann
comment:35 Changed 7 years ago by
Okay, it is hell to know what exactly you did here. Can you tell me ?
For instance, what is the point of this who_asked in projective plane ?
Nathann
comment:36 Changed 7 years ago by
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 :
1) GO back to the history before the merge
2) Do the merge and only the merge
3) Implement stuff
Otherwise I just can't see what you did.
Nathann
comment:37 Changed 7 years ago by
Hi,
Before the merge, there was a who_asked
argument in projective_plane_as_OA
. Now it is an argument of projective_plane
. In the second part of the merge I only fix some typo and doctests.
Perhaps I should go much before in history and then do the merge... let me try.
Vincent
comment:38 Changed 7 years ago by
Oh. I found a way. Cool O_O;;
Vincent, I think there is absolutely no point to the implementation of the who_asked
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
Nathann
comment:39 followup: ↓ 40 Changed 7 years ago by
It seems I still cannot see all changes. I saw that though :
+ #TODO: here there is no need to test everything as the function + # projective_plane answers it for you
If you want this to be removed, you need an existence flag on projective_plane
.
Nathann
comment:40 in reply to: ↑ 39 Changed 7 years ago by
If you want this to be removed, you need an existence flag on
projective_plane
.
Oh, we have it already. Cool. Well, you can replace it if you want, then.
Nathann
comment:41 Changed 7 years ago by
Time to eat... will have a look later tonight.
Vincent
comment:42 followup: ↓ 43 Changed 7 years ago by
It will be much cleaner if I start the merge from d678326 (your last commit)... let's go!
Vincent
comment:43 in reply to: ↑ 42 Changed 7 years ago by
It will be much cleaner if I start the merge from d678326 (your last commit)... let's go!
+1
Nathann
comment:44 Changed 7 years ago by
 Commit changed from 8830bb57327e1ca370287ca60b33e6d1b790a2d3 to 5074eee1d802ef44c251252581684bf1dc1dc7d6
comment:45 Changed 7 years ago by
Hi Nathann,
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 transversal_design
.
Vincent
comment:46 Changed 7 years ago by
Thanks ! It will be easier to follow this way :)
Nathann
comment:47 Changed 7 years ago by
Note that the doc fails to build
[combinat ] /opt/sage/local/lib/python2.7/sitepackages/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/sitepackages/sage/combinat/designs/orthogonal_arrays.py:docstring of sage.combinat.designs.orthogonal_arrays.find_wilson_decomposition:14: ERROR: Unexpected indentation.
comment:48 followup: ↓ 51 Changed 7 years ago by
Yooooooooooo !!
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 :/
Otherwise everything looks right... If I may ask, I think it would also be better to not have this TD = designs.transversal_design
in the doctest. Keeping the full writing also helps explaining to the users that all the TD can be built from designs.transversal_design
.
Nathann
comment:49 Changed 7 years ago by
Besides it's good to go.
Nathann
comment:50 Changed 7 years ago by
 Commit changed from 5074eee1d802ef44c251252581684bf1dc1dc7d6 to d81f265637099f93495dafac54e2d354be0d66d5
Branch pushed to git repo; I updated commit sha1. New commits:
d81f265  trac #16272: ultimate doctest

comment:51 in reply to: ↑ 48 ; followup: ↓ 52 Changed 7 years ago by
Replying to ncohen:
Yooooooooooo !!
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
:/
I do not care too much about calling twice the function. My main trouble was that in OA
with parameters n,k=n+1,t=2
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 n,k,t=2
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.
Otherwise everything looks right... If I may ask, I think it would also be better to not have this
TD = designs.transversal_design
in the doctest. Keeping the full writing also helps explaining to the users that all the TD can be built fromdesigns.transversal_design
.
Cool (Note that the doc problem is fixed).
We leave the idea of having an optional k=None
? Might be an independent ticket anyway.
Vincent
comment:52 in reply to: ↑ 51 Changed 7 years ago by
I do not care too much about calling twice the function.
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
1) This is only when the answer has been found, so it does not happens often per call
2) A tricky replacement is possible too, as
elif (t == 2 and (projective_plane(n, existence=True) or (k == n+1 and projective_plane(n, existence=True) is False))):
is equivalent to:
elif (t == 2 and (projective_plane(n, existence=True) in (True,k<n+1))):
This being said the code of these things compared to the cost of building the actual array ...
We leave the idea of having an optional
k=None
? Might be an independent ticket anyway.
Yep, let's close this one and get #16277 in. I will merge it in a second.
Nathann
comment:53 Changed 7 years ago by
 Commit changed from d81f265637099f93495dafac54e2d354be0d66d5 to 47798d2d54221c0bd0ce6aa75d1900496a091e2f
Branch pushed to git repo; I updated commit sha1. New commits:
47798d2  trac #16272: simplifying the structure of orthogonal_array

comment:54 followup: ↓ 55 Changed 7 years ago by
Hi,
This is what I did not want to do... if k=n+1
and projective_plane
answers Unknown
you are going to try other constructions. In comment:38 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!
Who is the author? who is the reviewer? who sets to positive review?
Vincent
comment:55 in reply to: ↑ 54 Changed 7 years ago by
Yo !
This is what I did not want to do... if
k=n+1
andprojective_plane
answersUnknown
you are going to try other constructions. In comment:38 you said that it was useless...
Oh right. But that was to avoid having a who_asks
parameter in projective_plane
. 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 ;)
Who is the author? who is the reviewer? who sets to positive review?
Well, let's do half/half. Positive review with both reviewers and both authors. By the way I just finished the patch for k=None
and I added the MOLS table from my dreams. The last dependency of the constructions will also be reviewed in a second ;)
Nathann
comment:56 Changed 7 years ago by
 Reviewers set to Nathann Cohen, Vincent Delecroix
 Status changed from needs_review to positive_review
comment:57 Changed 7 years ago by
 Commit changed from 47798d2d54221c0bd0ce6aa75d1900496a091e2f to d34b012758319ebcea5c0a0e17e6481ad5ded481
 Status changed from positive_review to needs_review
comment:58 Changed 7 years ago by
 Status changed from needs_review to positive_review
Still merging ...
Nathann
comment:59 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:61 Changed 7 years ago by
 Commit changed from d34b012758319ebcea5c0a0e17e6481ad5ded481 to c127a6d52c126483966dbc5faa6436d699a0d4a4
comment:62 Changed 7 years ago by
 Dependencies changed from #15310, #16227, #16281 to #16231
 Status changed from needs_work to positive_review
comment:63 Changed 7 years ago by
 Commit changed from c127a6d52c126483966dbc5faa6436d699a0d4a4 to e2749b3db4cd78fc5e9ea8219d4cdf895b283465
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
e2749b3  trac #16272: Merged with 6.3.beta0

comment:64 Changed 7 years ago by
 Status changed from needs_review to positive_review
comment:65 Changed 7 years ago by
 Branch changed from public/16272 to e2749b3db4cd78fc5e9ea8219d4cdf895b283465
 Resolution set to fixed
 Status changed from positive_review to closed
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.
The BruckRyserChowla 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.
A reference for the non existence of TD$(11,10)$ is
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.