Opened 8 years ago

Closed 8 years ago

#16528 closed enhancement (fixed)

OA(9,120)

Reported by: Nathann Cohen Owned by:
Priority: major Milestone: sage-6.3
Component: combinatorial designs Keywords:
Cc: Vincent Delecroix, Kannappan Sampath, brett stevens Merged in:
Authors: Nathann Cohen Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 2bd16e0 (Commits, GitHub, GitLab) Commit: 2bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf
Dependencies: #16524 Stopgaps:

Status badges

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 8 years ago by Nathann Cohen

Branch: u/ncohen/16528
Commit: a6003407cd2489f24541f418c778911b39331b9b
Status: newneeds_review

New commits:

a600340trac #16528: OA(9,120)

comment:2 Changed 8 years ago by git

Commit: a6003407cd2489f24541f418c778911b39331b9b3487f093a5c6f8ee2f5454357276125a142c95d7

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

e948cf6trac #16423: Aligning the alignment
0a7d853trac #16423: Broken doctests
b329351trac #16499: Cheap speedup in the OA recursive constructions
a67c04ftrac #16500: New recursive constructions of Orthogonal Arrays
41c50d5trac #16500: Simplified find_recursive_construction
e1992cetrac #16500: doc + speedup
697dd0ctrac #16500: Typoes in the doc
71dad5dtrac #16503: q-x construction of Orthogonal Arrays
002ceeetrac #16524: OA(9,135)
3487f09trac #16528: OA(9,120)

comment:3 Changed 8 years ago by Vincent Delecroix

Status: needs_reviewneeds_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

Last edited 8 years ago by Vincent Delecroix (previous) (diff)

comment:4 in reply to:  3 Changed 8 years ago by Nathann Cohen

Branch: u/ncohen/16528u/vdelecroix/16528
Commit: 3487f093a5c6f8ee2f5454357276125a142c95d7c80b7f464e197ad15ed239ef1246b197d12e4a69
Reviewers: Vincent Delecroix
Status: needs_infopositive_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() by M.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:

c80b7f4trac #16528: review
Last edited 8 years ago by Nathann Cohen (previous) (diff)

comment:5 Changed 8 years ago by Nathann Cohen

Branch: u/vdelecroix/16528u/ncohen/16528
Commit: c80b7f464e197ad15ed239ef1246b197d12e4a692bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf

New commits:

2bd16e0trac #16528: Broken doctest

comment:6 Changed 8 years ago by Volker Braun

Branch: u/ncohen/165282bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.