MOLS constructions rom the Handbook of Combinatorial Designs

[ncohen]

Heeeeeere they are ! ;-)

follow up: #16235

[git]

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

​22db72b	trac #16277; Merged with #16269
​2642317	trac #16248: Some trivial Orthogonal Arrays
​2d84806	trac #16277: Merge with #16248
​8b8aea8	trac #16058: Organize the index of combinatorial modules
​e0d2b66	trac #16058: Two new categories
​0293c49	trac #16058: Another group
​a14057b	Some more groupings and separated root system types into separate list.
​4b5cb2b	trac #16058: Rebase on 6.2.rc0
​5dbc42f	trac #16277: Merged with #16058
​d4f445b	trac #16277: MOLS constructions rom the Handbook of Combinatorial Designs

[ncohen]

Only the last commit implements something. All others are dependencies.

God, I can't say how glad I am to see this ticket created :-P

Nathann

[ncohen]

By the way Vincent : the database takes 30s to test on my computer... Do we make those tests long tests ?

Nathann

[vdelecroix]

Hi Nathann,

Globally looks good. Documentation builds and all test pass.

1) Could we make it go after #16272? There is a moderately trivial rebase to do. I think it would be easier in that direction.

2) Could we also move the TD_6_12 into the database module?

3) In the doctests of database, I would rather call directly the function themselves followed by is_transversal_design. We have no idea (at least me) that the corresponding design is actually returned when we call designs.orthogonal_array with the good parameters. Nevertheless it is a good test to check that the latter actually return a value for those parameters (and to see whether or not it is the corresponding value).

4) (semi-stupid remark) For the 6 construction of OA from Vmt, the function is rather trivial. One way to avoid duplication is to modify the dictionnary into n -> (k, constructor, extra_args). For the Vmt the entries would look like

OA_constructions = {
...
46: (6, OA_from_Vmt, (4,9,[0, 1, 3, 2, 8])),
50: (8, OA_from_Vmt, (6,7,[0, 1, 3, 16, 35, 26, 36])),
...
}

Vincent

[ncohen]

Yo !

1) Could we make it go after #16272? There is a moderately trivial rebase to do. I think it would be easier in that direction.

Yeah, no problem I guess.

2) Could we also move the TD_6_12 into the database module?

Yepyep, I will do that.

3) In the doctests of database, I would rather call directly the function themselves followed by is_transversal_design. We have no idea (at least me) that the corresponding design is actually returned when we call designs.orthogonal_array with the good parameters. Nevertheless it is a good test to check that the latter actually return a value for those parameters (and to see whether or not it is the corresponding value).

'f you want ......

4) (semi-stupid remark) For the 6 construction of OA from Vmt, the function is rather trivial. One way to avoid duplication is to modify the dictionnary into n -> (k, constructor, extra_args). For the Vmt the entries would look like

OA_constructions = {
...
46: (6, OA_from_Vmt, (4,9,[0, 1, 3, 2, 8])),
50: (8, OA_from_Vmt, (6,7,[0, 1, 3, 16, 35, 26, 36])),
...
}

If we do that the OA will be built when Sage starts. Plus I would like to thank Julian R. Abel somewhere.

Nathann

[vdelecroix]

Replying to ncohen:

4) (semi-stupid remark) For the 6 construction of OA from Vmt, the function is rather trivial. One way to avoid duplication is to modify the dictionnary into n -> (k, constructor, extra_args). For the Vmt the entries would look like

OA_constructions = {
...
46: (6, OA_from_Vmt, (4,9,[0, 1, 3, 2, 8])),
50: (8, OA_from_Vmt, (6,7,[0, 1, 3, 16, 35, 26, 36])),
...
}

If we do that the OA will be built when Sage starts. Plus I would like to thank Julian R. Abel somewhere.

Agreed. And it is better to test the corresponding code in the docstring of the function.

[ncohen]

Here it is. Gosh, it takes a lifetime to update all the doctests....

For the moment it still uses the availability keyword, and this ticket has not been merged on tpo of #16272. This is because the #16272 is not finished and I don't want to merge it before that.

Nathann

[git]

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

​490aa7e

trac #16277: Reviewer's remarks

[vdelecroix]

Hi Nathann,

In the doctest, I would like to see:

sage: TD = TD_6_12()
sage: TDbis = designs.transversal_design(6,12)
sage: TD == TDbis
True

It is worth it to see if the individual designs are actually used in the construction, isn't it? Even if the answer is False at some point, it still makes sense to keep the function and the doctest.

I do not mind too much as it is painful to edit all doctests in database.py.

Vincent

[ncohen]

In the doctest, I would like to see:

Hey man, what you "would like to see" takes me 20 minutes each time....

sage: TD = TD_6_12()
sage: TDbis = designs.transversal_design(6,12)
sage: TD == TDbis
True

WHAT IS THE POIIIIIIIIIIIIIIIINT ????

It is worth it to see if the individual designs are actually used in the construction, isn't it? Even if the answer is False at some point, it still makes sense to keep the function and the doctest.

Who cares ? The function checks that the designs returned are good, and it checks that they are available through the main entry point. It already takes 30 seconds to check this file ! You want to double it to check ... that ?

Nathann

[vdelecroix]

As I said, I don't mind as I know that it is painful to edit.

The point is that in the future, somebody might come and ask: for a given k,n how many different TD(k,n) do I have? But an equality test is far from an isomorphism test, so please leave it.

Vincent

[ncohen]

The point is that in the future, somebody might come and ask: for a given k,n how many different TD(k,n) do I have? But an equality test is far from an isomorphism test, so please leave it.

......

Yeah, I guess it is a bit too early to do things like that ...

Nathann

[git]

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

​14420b6

trac #16277: Yet another construction

[git]

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

​0812a73	trac #16281: Simplification
​61dc86b	16281: long comment for the construction of the projective plane
​51daa7f	trac #16281: correct a doctest
​e090f92	trac #16272: merge #16281
​9a221be	trac #16272: fix doctests
​5074eee	trac #16272: finer doctest to test the output of transversal_design
​d81f265	trac #16272: ultimate doctest
​47798d2	trac #16272: simplifying the structure of orthogonal_array
​ddda559	trac #16277: Merged with updated #16272
​7570b39	trac #16277: From availability to existence and broken doctests

[ncohen]

God. There *WERE* conflicts :-PPP

Nothing bad, but still, the file was messy at first ^^;

Nathann

[vdelecroix]

Hi Nathann,

In orthogonal_arrays.py in the function transversal_design you added a test

`TD(6,12)` ::

    sage: _ = designs.transversal_design(6,12)

which is actually already tested in the block above it and in database.py. So if you don't mind, you can simply remove it.

Otherwise, everything is fine and it could go to positive review after that!

Vincent

[git]

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

​5e8b2af

trac #16277: removes a doctests that appears several times already

[ncohen]

Well, then ...

[ncohen]

(by the way it was also tested in the TD_product docstring)

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​e86d26c	trac #15310: Merged with 6.2.rc2
​5cfee91	trac #16227: Merged with updated #15310
​d051cf4	ValueError -> EmptySetError in latin_squares
​9e5e94f	trac #16231: Merged with updated #16227
​acf8988	trac #16277: Merged with updated #16231

[ncohen]

Still updating ...

Nathann

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​d34b012	trac #16272: Merged with updated #16227
​c127a6d	trac #16272: Merged with updated #16231
​36a705b	trac #16248: Merged with #16231
​5dc4a25	trac #16248: Merged with #16272
​f019bd3	Merge branch 'u/ncohen/16248' of trac.sagemath.org:sage into 16231
​485e880	trac #16277: Merged with updated #16248

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

​411a759	trac #16286: Merged with updated #16277
​0f3a755	trac #16235: A pair of orthogonal latin squares of order 10
​cb2e272	trac #16235: Merged with updated #16286
​aeab437	trac #16235: merge the updated #16286
​daa3835	trac #16235: case n=1 + doc
​31a53f2	trac #16235: update the MOLS table
​11eff2c	trac #16235: Merged with 6.2
​5a0e3fe	trac #16235: Merged with #16231
​c0b13c4	trac #16235: Merged with updated #16286
​9eab3e3	Merge branch 'public/16235' of trac.sagemath.org:sage into tmp

[ncohen]

ARGGGGGGGGGGGGGG.... Wrong push T_T

[ncohen]

Back to the previous one T_T

[git]

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

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​e2749b3	trac #16272: Merged with 6.3.beta0
​6678079	trac #16277: Merged with updated #16272
​270c65b	trac #16277: A typo

[ncohen]

Better -_-