Opened 5 years ago

Closed 4 years ago

#16780 closed enhancement (fixed)

Brouwer's separable design construction of OA

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

  • Branch set to u/ncohen/16780
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by git

  • Commit set to dca31d38cccf754176010b45cd2835676ef682f2

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

355ac2atrac #16662: OA for n=1046,1059,2164,3992,3994
15b449ctrac #16665: New OA for n=408,600,792,856,1368,2328,...
a515deetrac #16673: Three factors construction of MOLS
845de7atrac #16716: OA for n=262,950
a9608c5trac #16722: OA(17,560)
e5fc881trac #16722: Merged with 6.3.beta8
698b704trac #16757: Organize the V(m,t) vectors into a dictionary
0598e46trac #16763: New OA for n=189, plus some others through Vmt vectors
792ed9dtrac #16763: Merged with 6.3.rc1
dca31d3trac #16780: Brouwer's separable design construction of OA

comment:3 Changed 5 years ago by git

  • Commit changed from dca31d38cccf754176010b45cd2835676ef682f2 to 5400e2a44b2444c8b661e0e6acbd7c0b6f252f71

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

5400e2atrac #16780: Brouwer's separable design construction of OA

comment:4 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:5 Changed 4 years ago by ncohen

  • Dependencies changed from #16763 to #16863

comment:6 Changed 4 years ago by git

  • Commit changed from 5400e2a44b2444c8b661e0e6acbd7c0b6f252f71 to deee0699c23838922e5dc1d19c152b464ca1e302

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

9a57f13trac #16757: Organize the V(m,t) vectors into a dictionary
57c00f0trac #16757: doctest simplication
424e229trac #16763: New OA for n=189, plus some others through Vmt vectors
5ba165etrac #16763: Complete bibliographical references
f3f644dtrac #16763: code simplification
04936aatrac #16802: database of difference family
4bd8d69trac #16802: Review
4434d61trac #16802: review the review
33b4171trac #16863: twin prime difference set
deee069trac #16780: Brouwer's separable design construction of OA

comment:7 Changed 4 years ago by vdelecroix

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(k-1,N).

I let you do the rebase over #16859.

Vincent

comment:8 Changed 4 years ago by ncohen

Yoooooo !

I agree with your modifications. This patch, however, will have to be rebased not on #16859 but on #16863 (which only needs a final check). Plus I will also have to merge it with #16859 before removing the "incomplete+resolvable OA" blocks.

Nathann

comment:9 Changed 4 years ago by ncohen

  • Branch changed from u/ncohen/16780 to u/vdelecroix/16780
  • Commit changed from deee0699c23838922e5dc1d19c152b464ca1e302 to fac1b83b48242a2abbdfd7e7abf647e0acc89373

New commits:

fac1b83trac #16780: review 1

comment:10 Changed 4 years ago by ncohen

  • Branch changed from u/vdelecroix/16780 to public/16780
  • Commit changed from fac1b83b48242a2abbdfd7e7abf647e0acc89373 to 52c4fa0e328d0ccf666506d8a8b2d89ae528ccd3

Last 10 new commits:

2297d31trac #16780: Brouwer's separable design construction of OA
9b449fbtrac #16780: review 1
b2f1c8ftrac #16655: resolvable OA/TD
9103ee9trac #16655: Merged with 6.4.beta1
cffb31dtrac #16722: A note about GLPK's "performances", new arguments to change the solver and the verbosity level
5245ef6trac #16655: review
0bad9edtrac #16859: Resolvable incomplete orthogonal arrays
e6deb83trac #16859: doc
db53470trac #16780: Merged with #16859 (need the resolvable incomplete OA)
52c4fa0trac #16780: resolvable incomplete OA are built with 2 lines

comment:11 follow-up: Changed 4 years ago by vdelecroix

In my last commit (at public/16780):

  • added in the doc: in case i) x=0, we construct a resolvable OA(k-1,N).
  • the l.extend([f(x) for x in R]) are replaced by l.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 4 years ago by git

  • Commit changed from 52c4fa0e328d0ccf666506d8a8b2d89ae528ccd3 to 7fe173a1a3fbbaac5de6174d6c18db26d55886e8

Branch pushed to git repo; I updated commit sha1. New commits:

7fe173atrac #16780: review 2

comment:13 Changed 4 years ago by git

  • Commit changed from 7fe173a1a3fbbaac5de6174d6c18db26d55886e8 to 931a33fe5720312ec1b109e2a6d1de29472a8256

Branch pushed to git repo; I updated commit sha1. New commits:

931a33ftrac #16780: repeat "parallel classes"

comment:14 in reply to: ↑ 11 Changed 4 years ago by ncohen

Hello !

  • added in the doc: in case i) x=0, we construct a resolvable OA(k-1,N).

Good, good !

  • the l.extend([f(x) for x in R]) are replaced by l.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 4 years ago by ncohen

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

comment:16 follow-up: Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict in src/sage/combinat/designs/database.py

comment:17 in reply to: ↑ 16 Changed 4 years ago by vdelecroix

Replying to vbraun:

Merge conflict in src/sage/combinat/designs/database.py

It is hardly believable... the combinatorial design tickets were linearly ordered! And I just check that it is up to date with #16802. Could you point the relevant ticket for the conflict?

Vincent

comment:18 Changed 4 years ago by vbraun

Conflict is 424e229a (#16763)

comment:19 Changed 4 years ago by git

  • Commit changed from 931a33fe5720312ec1b109e2a6d1de29472a8256 to 63ca3d79103ceb8e5493e5c8becbc255b09c8b7d

Branch pushed to git repo; I updated commit sha1. New commits:

bb21dc0trac #16763: Double citation
63ca3d7trac #16780: merge #16763

comment:20 Changed 4 years ago by vdelecroix

  • Status changed from needs_work to positive_review

done...

Vincent

comment:21 Changed 4 years ago by vbraun

  • Branch changed from public/16780 to 63ca3d79103ceb8e5493e5c8becbc255b09c8b7d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.