Opened 6 years ago
Closed 6 years ago
#16528 closed enhancement (fixed)
OA(9,120)
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.3 
Component:  combinatorial designs  Keywords:  
Cc:  vdelecroix, knsam, brett  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  2bd16e0 (Commits)  Commit:  2bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf 
Dependencies:  #16524  Stopgaps: 
Description
As the title says ! Another one of Julian R. Abel's tricks ;)
This one was... Painful to implement :P
Nathann
Change History (6)
comment:1 Changed 6 years ago by
 Branch set to u/ncohen/16528
 Commit set to a6003407cd2489f24541f418c778911b39331b9b
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Commit changed from a6003407cd2489f24541f418c778911b39331b9b to 3487f093a5c6f8ee2f5454357276125a142c95d7
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
e948cf6  trac #16423: Aligning the alignment

0a7d853  trac #16423: Broken doctests

b329351  trac #16499: Cheap speedup in the OA recursive constructions

a67c04f  trac #16500: New recursive constructions of Orthogonal Arrays

41c50d5  trac #16500: Simplified find_recursive_construction

e1992ce  trac #16500: doc + speedup

697dd0c  trac #16500: Typoes in the doc

71dad5d  trac #16503: qx construction of Orthogonal Arrays

002ceee  trac #16524: OA(9,135)

3487f09  trac #16528: OA(9,120)

comment:3 followup: ↓ 4 Changed 6 years ago by
 Status changed from needs_review to needs_info
Just comments about my commit (at u/vdelecroix/16528
)
Why if it is precomputed you do the extra check
for B in BIBD: len_trace = sum(x in hyperoval for x in B) assert len_trace == 0 or len_trace == 2
(commented in my commit)
I replaced the ugly Matrix[x].dict().keys()
by M.nonzero_positions_in_row(x)
.
In two places it is not needed to build a list, Python iterator are useful for that
 len_trace = len([x for x in B if x in hyperoval]) + len_trace = sum(x in hyperoval for x in B)  r = {v:i for i,v in enumerate([x for x in range(n) if x not in hyperoval])} + r = {v:i for i,v in enumerate(x for x in range(n) if x not in hyperoval)}
That's all before the positive review.
Vincent
comment:4 in reply to: ↑ 3 Changed 6 years ago by
 Branch changed from u/ncohen/16528 to u/vdelecroix/16528
 Commit changed from 3487f093a5c6f8ee2f5454357276125a142c95d7 to c80b7f464e197ad15ed239ef1246b197d12e4a69
 Reviewers set to Vincent Delecroix
 Status changed from needs_info to positive_review
Yo !
Why if it is precomputed you do the extra check
for B in BIBD: len_trace = sum(x in hyperoval for x in B) assert len_trace == 0 or len_trace == 2(commented in my commit)
Because it costs nothing, and because, given that the constructions are not always obvious, it is good to explain what is happening all along. It works the same way if it is commented.
I replaced the ugly
Matrix[x].dict().keys()
byM.nonzero_positions_in_row(x)
.
Thanks ! I did not know this function before you used it in another ticket.
In two places it is not needed to build a list, Python iterator are useful for that
If you believe that it makes any difference ... :P
That's all before the positive review.
Okayyyyyy ! Thanks, then :)
Nathann
P.S. : Something weird is happening. That's the last construction in needs_review
! I will fix that as soon as I get Julian's next mail ;)
Nathann
New commits:
c80b7f4  trac #16528: review

comment:5 Changed 6 years ago by
 Branch changed from u/vdelecroix/16528 to u/ncohen/16528
 Commit changed from c80b7f464e197ad15ed239ef1246b197d12e4a69 to 2bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf
New commits:
2bd16e0  trac #16528: Broken doctest

comment:6 Changed 6 years ago by
 Branch changed from u/ncohen/16528 to 2bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #16528: OA(9,120)