Opened 7 years ago
Closed 7 years ago
#16780 closed enhancement (fixed)
Brouwer's separable design construction of OA
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  combinatorial designs  Keywords:  
Cc:  vdelecroix, knsam, dimpase, brett  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  63ca3d7 (Commits, GitHub, GitLab)  Commit:  63ca3d79103ceb8e5493e5c8becbc255b09c8b7d 
Dependencies:  #16863  Stopgaps: 
Description
Once more, this wouldn't have been possible without Julian R. Abel's help.
Oh, and...
This code is "commented". To say the least :P
Change History (21)
comment:1 Changed 7 years ago by
 Branch set to u/ncohen/16780
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
 Commit set to dca31d38cccf754176010b45cd2835676ef682f2
comment:3 Changed 7 years ago by
 Commit changed from dca31d38cccf754176010b45cd2835676ef682f2 to 5400e2a44b2444c8b661e0e6acbd7c0b6f252f71
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5400e2a  trac #16780: Brouwer's separable design construction of OA

comment:4 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:5 Changed 7 years ago by
 Dependencies changed from #16763 to #16863
comment:6 Changed 7 years ago by
 Commit changed from 5400e2a44b2444c8b661e0e6acbd7c0b6f252f71 to deee0699c23838922e5dc1d19c152b464ca1e302
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
9a57f13  trac #16757: Organize the V(m,t) vectors into a dictionary

57c00f0  trac #16757: doctest simplication

424e229  trac #16763: New OA for n=189, plus some others through Vmt vectors

5ba165e  trac #16763: Complete bibliographical references

f3f644d  trac #16763: code simplification

04936aa  trac #16802: database of difference family

4bd8d69  trac #16802: Review

4434d61  trac #16802: review the review

33b4171  trac #16863: twin prime difference set

deee069  trac #16780: Brouwer's separable design construction of OA

comment:7 Changed 7 years ago by
Hi,
A first commit at u/vdelecroix/16780
.
In the documentation of brouwer_separable_design
case i) it would be nice to say before that you want to build a resolvable OA(k1,N)
.
I let you do the rebase over #16859.
Vincent
comment:8 Changed 7 years ago by
comment:9 Changed 7 years ago by
 Branch changed from u/ncohen/16780 to u/vdelecroix/16780
 Commit changed from deee0699c23838922e5dc1d19c152b464ca1e302 to fac1b83b48242a2abbdfd7e7abf647e0acc89373
New commits:
fac1b83  trac #16780: review 1

comment:10 Changed 7 years ago by
 Branch changed from u/vdelecroix/16780 to public/16780
 Commit changed from fac1b83b48242a2abbdfd7e7abf647e0acc89373 to 52c4fa0e328d0ccf666506d8a8b2d89ae528ccd3
Last 10 new commits:
2297d31  trac #16780: Brouwer's separable design construction of OA

9b449fb  trac #16780: review 1

b2f1c8f  trac #16655: resolvable OA/TD

9103ee9  trac #16655: Merged with 6.4.beta1

cffb31d  trac #16722: A note about GLPK's "performances", new arguments to change the solver and the verbosity level

5245ef6  trac #16655: review

0bad9ed  trac #16859: Resolvable incomplete orthogonal arrays

e6deb83  trac #16859: doc

db53470  trac #16780: Merged with #16859 (need the resolvable incomplete OA)

52c4fa0  trac #16780: resolvable incomplete OA are built with 2 lines

comment:11 followup: ↓ 14 Changed 7 years ago by
In my last commit (at public/16780
):
 added in the doc: in case i)
x=0
, we construct a resolvableOA(k1,N)
.  the
l.extend([f(x) for x in R])
are replaced byl.extend(f(x) for x in R)
(we win at least 2ms... incredible, isn't it?) Exception
>RuntimeError
 the size of a matrix is given as
nb_rows x nb_cols
and not the contrary
I would not say that I carefully checked all constructions, but I am confident since there is a lot of doctest. I think it could go to positive review.
Vincent
comment:12 Changed 7 years ago by
 Commit changed from 52c4fa0e328d0ccf666506d8a8b2d89ae528ccd3 to 7fe173a1a3fbbaac5de6174d6c18db26d55886e8
Branch pushed to git repo; I updated commit sha1. New commits:
7fe173a  trac #16780: review 2

comment:13 Changed 7 years ago by
 Commit changed from 7fe173a1a3fbbaac5de6174d6c18db26d55886e8 to 931a33fe5720312ec1b109e2a6d1de29472a8256
Branch pushed to git repo; I updated commit sha1. New commits:
931a33f  trac #16780: repeat "parallel classes"

comment:14 in reply to: ↑ 11 Changed 7 years ago by
Hello !
 added in the doc: in case i)
x=0
, we construct a resolvableOA(k1,N)
.
Good, good !
 the
l.extend([f(x) for x in R])
are replaced byl.extend(f(x) for x in R)
(we win at least 2ms... incredible, isn't it?)
Perhaps even twice that. Crazy.
Exception
>RuntimeError
Good good !
 the size of a matrix is given as
nb_rows x nb_cols
and not the contrary
I object, but I have everybody against me. So I will not object aloud.
I would not say that I carefully checked all constructions, but I am confident since there is a lot of doctest. I think it could go to positive review.
Well, each case is tested and the code cannot return anything wrong anyway as all results are checked before that. Somehow while the design code is always very tricky there is no risk to return anything wrong.
I added a small commit to repeat "parallel classes" in a sentence, but short of this everything is cool, and it can go. Thank you very much for this review !
Nathann
comment:15 Changed 7 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to positive_review
comment:16 followup: ↓ 17 Changed 7 years ago by
 Status changed from positive_review to needs_work
Merge conflict in src/sage/combinat/designs/database.py
comment:17 in reply to: ↑ 16 Changed 7 years ago by
comment:18 Changed 7 years ago by
Conflict is 424e229a (#16763)
comment:19 Changed 7 years ago by
 Commit changed from 931a33fe5720312ec1b109e2a6d1de29472a8256 to 63ca3d79103ceb8e5493e5c8becbc255b09c8b7d
comment:21 Changed 7 years ago by
 Branch changed from public/16780 to 63ca3d79103ceb8e5493e5c8becbc255b09c8b7d
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
trac #16662: OA for n=1046,1059,2164,3992,3994
trac #16665: New OA for n=408,600,792,856,1368,2328,...
trac #16673: Three factors construction of MOLS
trac #16716: OA for n=262,950
trac #16722: OA(17,560)
trac #16722: Merged with 6.3.beta8
trac #16757: Organize the V(m,t) vectors into a dictionary
trac #16763: New OA for n=189, plus some others through Vmt vectors
trac #16763: Merged with 6.3.rc1
trac #16780: Brouwer's separable design construction of OA