Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

q-x construction of Orthogonal Arrays #16503

Closed
nathanncohen mannequin opened this issue Jun 20, 2014 · 22 comments
Closed

q-x construction of Orthogonal Arrays #16503

nathanncohen mannequin opened this issue Jun 20, 2014 · 22 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jun 20, 2014

Another construction of orthogonal arrays. New OA for n=158, 329, 334, 355, 517, 574, 745, 979, 1926.

Thanks to Julian R. Abel's pointers, as usual :-)

Nathann

Depends on #16500

CC: @videlec

Component: combinatorial designs

Author: Nathann Cohen

Branch/Commit: 8da2c73

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/16503

@nathanncohen nathanncohen mannequin added this to the sage-6.3 milestone Jun 20, 2014
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 20, 2014

Branch: u/ncohen/16503

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 20, 2014

Dependencies: #16500

@nathanncohen nathanncohen mannequin added the s: needs review label Jun 20, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 20, 2014

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

004833atrac #16347: Wilson's construction with two truncated groups
f5069f0trac #16347: Merged with 6.3.beta4
f182d36trac #16499: Cheap speedup in the OA recursive constructions
70f7046trac #16500: New recursive constructions of Orthogonal Arrays
9128badtrac #16503: q-x construction of Orthogonal Arrays

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 20, 2014

Commit: 9128bad

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 25, 2014

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

828ff22trac #16437: cut the branches in W. dec. with two trunc. blocks
8ebd21btrac #16347: use is_sum_of_squares_pyx instead of two_squares
0175134trac #16347: doc + simplifications
9ff5062trac #16423: Table of MOLS from the handbook and comparison with Sage
e64be98trac #16423: tiny code improvement and alignment
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
6087843trac #16503: q-x construction of Orthogonal Arrays

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 25, 2014

Changed commit from 9128bad to 6087843

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 26, 2014

comment:4

Updated on top of #16500 !

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 26, 2014

Changed commit from 6087843 to 71dad5d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 26, 2014

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

41c50d5trac #16500: Simplified find_recursive_construction
e1992cetrac #16500: doc + speedup
697dd0ctrac #16500: Typoes in the doc
71dad5dtrac #16503: q-x construction of Orthogonal Arrays

@videlec
Copy link
Contributor

videlec commented Jun 27, 2014

comment:6

Hi Nathann,

  1. One lines 656-659 you have an import followed by a commented check!!
from sage.combinat.designs.bibd import _check_pbd
PBD = [[relabel[xx] for xx in B if not xx in points_to_delete] for B in TD]

# _check_pbd(PBD,n,[q,q-x-1,q-x+1,x+2])

why is that?

  1. This is wrong (lines 730-731)
# The next is always True, because q is a prime power
# orthogonal_array(k+1,q,existence=True) and

if k+1 > q... instead we can start the loop at k+1 instead of 3.

  1. I bounced into something bad: I wanted to change in find_recursive_construction the
assert k >= 3

into a

assert k > 3, "you do not need recursion for k<4"

since it is trivial to build one latin square. But it appears that the recursive constructions is called with k=3 many times! so bad! This has to be corrected... but hopefully, not here. I guess that #16535 would be the good place.

Vincent

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 27, 2014

comment:7

Hello !

  1. One lines 656-659 you have an import followed by a commented check!!
from sage.combinat.designs.bibd import _check_pbd
PBD = [[relabel[xx] for xx in B if not xx in points_to_delete] for B in TD]

# _check_pbd(PBD,n,[q,q-x-1,q-x+1,x+2])

why is that?

It is something I used while implementing the construction and as it said something about the PBD I left it. I updated that comment as that troubled you.

  1. This is wrong (lines 730-731)
# The next is always True, because q is a prime power
# orthogonal_array(k+1,q,existence=True) and

The line before that one is

            orthogonal_array(k+1,q-x+1,existence=True) and
  1. I bounced into something bad: I wanted to change in find_recursive_construction the
assert k >= 3

into a

assert k > 3, "you do not need recursion for k<4"

since it is trivial to build one latin square. But it appears that the recursive constructions is called with k=3 many times! so bad! This has to be corrected... but hopefully, not here. I guess that #16535 would be the good place.

Why in #16535 ? What we need to do is add a case for k=3 in the orthogonal_array constructor.

I fixed the things above, plus an apparently incorrect 1<x.

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 27, 2014

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

91eb3d2trac #16503: Review

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 27, 2014

Changed commit from 71dad5d to 91eb3d2

@videlec
Copy link
Contributor

videlec commented Jun 27, 2014

comment:9

Hi Nathann,

You can actually cut the loop if x >= q as x is increasing along the loop. It is done at public/16503 (it does not matter too much right now, but it will if you want to compute a big MOLS table).

Beyond that, you can set to positive review.

Vincent

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jun 27, 2014

comment:11

HMmmmm... I don't like much to differentiate over integer divisions.. Do you mind if we keep it like that ?

Nathann

@videlec
Copy link
Contributor

videlec commented Jun 27, 2014

comment:12

Replying to @nathanncohen:

HMmmmm... I don't like much to differentiate over integer divisions.. Do you mind if we keep it like that ?

Me neither, but the floor function is increasing... so it is fine (as the composition of increasing functions is increasing)! But we can keep it as it is, since we cut very little with that: (q^2-n)/(q-2) ~ q + cte for large q.

Vincent

@videlec
Copy link
Contributor

videlec commented Jun 27, 2014

Reviewer: Vincent Delecroix

@videlec
Copy link
Contributor

videlec commented Jun 28, 2014

comment:13

As reported in #16524 there is a problem building the doc. Solution (line 622)

-    REFERENCES::
+    REFERENCES:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2014

Changed commit from 91eb3d2 to 8da2c73

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2014

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

8da2c73trac #16503: Broken doc

@videlec
Copy link
Contributor

videlec commented Jun 28, 2014

comment:16

... building the doc...

@vbraun
Copy link
Member

vbraun commented Jun 29, 2014

Changed branch from u/ncohen/16503 to 8da2c73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants