Opened 5 years ago

Closed 5 years ago

#16528 closed enhancement (fixed)

OA(9,120)

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.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 5 years ago by ncohen

  • Branch set to u/ncohen/16528
  • Commit set to a6003407cd2489f24541f418c778911b39331b9b
  • Status changed from new to needs_review

New commits:

a600340trac #16528: OA(9,120)

comment:2 Changed 5 years ago by git

  • Commit changed from a6003407cd2489f24541f418c778911b39331b9b to 3487f093a5c6f8ee2f5454357276125a142c95d7

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 follow-up: Changed 5 years ago by vdelecroix

  • 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

Last edited 5 years ago by vdelecroix (previous) (diff)

comment:4 in reply to: ↑ 3 Changed 5 years ago by ncohen

  • 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() 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 5 years ago by ncohen (previous) (diff)

comment:5 Changed 5 years ago by ncohen

  • Branch changed from u/vdelecroix/16528 to u/ncohen/16528
  • Commit changed from c80b7f464e197ad15ed239ef1246b197d12e4a69 to 2bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf

New commits:

2bd16e0trac #16528: Broken doctest

comment:6 Changed 5 years ago by vbraun

  • Branch changed from u/ncohen/16528 to 2bd16e0fafd5bca36bc5378bf4a59d62cc79d0bf
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.