Opened 5 years ago

Closed 5 years ago

#15286 closed enhancement (fixed)

Latin squares

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.2
Component: combinatorics Keywords:
Cc: Merged in:
Authors: Nathann Cohen Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 68cdedf (Commits) Commit: 68cdedfb785eb077df4d6872c20b8de0749e09d0
Dependencies: #15285 Stopgaps:

Description

This patch creates a file latin_squares.py in combinat/designs/, and implements several constructions of mutually orthogonal latin squares, as found in Douglas Stinson's book "Combinatorial Designs: Constructions and Analysis".

Nathann

Change History (26)

comment:1 Changed 5 years ago by ncohen

  • Branch changed from #15285 to u/ncohen/15286
  • Dependencies set to #15285
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by git

  • Commit set to d93abcd13bd5230bafce89a73f553bd60e4c7aa0

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

[changeset:d93abcd]Latin Squares
[changeset:2ec76c7]Bug in AffineGeometryDesign?
[changeset:cf71d58]Rebase on 5.13.beta0
[changeset:9fcfb13]Rename the method from ProjectivePlaneDesign? to DesarguesianProjectivePlaneDesign?
[changeset:363badb]trac 15107 -- reviewer's comments
[changeset:ee6d412]Projective Plane designs constructor

comment:3 Changed 5 years ago by git

  • Commit changed from d93abcd13bd5230bafce89a73f553bd60e4c7aa0 to a8a2a719d1f0fc4aedf24f24fc2c87e407182da4

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

[changeset:a8a2a71]Todo notes for further constructions

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

Hi,

Why this is not included to sage.combinat.matrix.latin_squares ? If you do not want to merge, you could at least make a reference.

  • The first line in the documentation is too large (146 columns).
  • line 112: "(cite theorem)" is not enough for the ValueError message
  • optimization line 123: replace l == [] by not l
  • could you put the references lines 117 and 140 that are in comments and I would like better to see them in the documentation

I will have a look at the mathematics next.

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

Helloooooo !!

Why this is not included to sage.combinat.matrix.latin_squares ? If you do not want to merge, you could at least make a reference.

Because I did not know that this module existed when I wrote this one. "fortunately" they are unrelated : the sage.combinat.matrix.latin_squares module deals with operations specific to a given latin square, while this module builds "mutually orthogonal latin squares". I added a comment to the top of the file.

  • The first line in the documentation is too large (146 columns).

Fixed.

  • line 112: "(cite theorem)" is not enough for the ValueError message

T_T

Sorry 'bout that. It is fixed. I thought this theorem has a name, but it is just a nameless one in the references I have, so I just replaced this text by There exist at most n-1 MOLS of size n.. And I added a doctest.

  • optimization line 123: replace l == [] by not l

Fixed. Assuming it was p == [].

  • could you put the references lines 117 and 140 that are in comments and I would like better to see them in the documentation

I do not understand your sentence very well O_o This being said, [Stinson2004] does appear in the documentation.

I will have a look at the mathematics next.

Thaaaaaaaaaaaaaaaaaaaaanks. I'm very very thankful fo that, 'cause I love all this designs code and there does not seem to be many design lovers among Sage's devs' ^^;

Nathann

comment:6 Changed 5 years ago by git

  • Commit changed from a8a2a719d1f0fc4aedf24f24fc2c87e407182da4 to e02d42c390b8f3841cea5548abb1588efb6d99a3

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

e02d42ctrac #15286: Reviewer's comments
941d5a0trac #15268: Rebase on 6.1.beta2

comment:7 follow-up: Changed 5 years ago by vdelecroix

As far as I understand, when you raise the ValueError? at line 162, it means that the code does not exist. Not that the entries are not consistent, right ? (I guess this is the purpose of your Warning in the documentation). I would then prefer a NotImplementedError?.

Could you give the definition of your product (in the doc) ? Note that there is also a different product in sage.combinat.matrices.latin_square called direct_product and needs four latin squares as entries.

Because of your rebase on 6.1.beta2 I need some time to compile ;-(

comment:8 in reply to: ↑ 7 ; follow-up: Changed 5 years ago by ncohen

Hellooooooooo !!

As far as I understand, when you raise the ValueError? at line 162, it means that the code does not exist. Not that the entries are not consistent, right ? (I guess this is the purpose of your Warning in the documentation). I would then prefer a NotImplementedError?.

Well, it means that we have no idea whether it exists or not, and in particular that Sage cannot built it. I don't really think it matters, but I can change it if you want.

For instance, this paper was published not very long ago : http://onlinelibrary.wiley.com/doi/10.1002/jcd.21384/abstract;jsessionid=6D04BFDDEA53369FE1ACDF6839FB7BD5.f01t01

So you can see what these MOLS mean : some guys are actually fighting to know the maximum order of MOLS of a given size.

Could you give the definition of your product (in the doc) ? Note that there is also a different product in sage.combinat.matrices.latin_square called direct_product and needs four latin squares as entries.

O_o

Well, the product I use is also called a direct product in Stinson's book. Only his actually takes two latin squares of sizes n,m and returns a latin square of size nxm, which makes infinitely more sense than taking 4 squares as input with disjoints sets of symbols and concatenating them in a larger matrix. Sigh.. Combinat code..

Anyway. Done, too.

Because of your rebase on 6.1.beta2 I need some time to compile ;-(

Yeah, that's one of the problems with git.. If you just checkout an old patch, you recompile everything. Well, at least now your Sage is up to date ^^;

Nathann

comment:9 Changed 5 years ago by git

  • Commit changed from e02d42c390b8f3841cea5548abb1588efb6d99a3 to 47df907cf1b7491b4ed2365e939101d9c6d15581

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

47df907trac #15286: Reviewer's comments 2

comment:10 Changed 5 years ago by ncohen

(this patch's only dependency is now positively reviewed)

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

Hi Nathann,

I am not able to access the documentation in the reference manual. Does it work for you ? Where is it linked ?

comment:12 in reply to: ↑ 11 Changed 5 years ago by ncohen

Yo !

I am not able to access the documentation in the reference manual. Does it work for you ? Where is it linked ?

Well, I was able to read the doc when I wrote this patch but I cannot build it at the moment.

[combinat ] building [html]: targets for 247 source files that are out of date
[combinat ] updating environment: [config changed] 255 added, 0 changed, 0 removed
[combinat ] Sphinx error:
[combinat ] 'ascii' codec can't decode byte 0xe2 in position 939: ordinal not in range(128)
Build finished. The built documents can be found in /home/ncohen/.Sage/src/doc/output/html/en/reference/combinat

This, however, is not this ticket's branch but... beta4. I get the same bug with this ticket's branch. So it has to be fixed.

Nathann

comment:13 Changed 5 years ago by ncohen

Hello !

I ended up being able to build the doc. Some accent. And Sphinx not helping much either. The doc can be obtained from "Combinatorics -> Designs and Incidence Structures -> Mutually Orthogonal Latin Squares (MOLS)".

Nathann

comment:14 Changed 5 years ago by git

  • Commit changed from 47df907cf1b7491b4ed2365e939101d9c6d15581 to e0e44f1d789d43e7d60186cd435b334928aeb8b6

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

49b55b3trac #15286: Rebase on 6.1.beta4
e0e44f1trac #15286: Fix an encoding problem in the doc

comment:15 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:16 Changed 5 years ago by ncohen

The dependency of this ticket has been reviewed.

Nathann

comment:17 Changed 5 years ago by git

  • Commit changed from e0e44f1d789d43e7d60186cd435b334928aeb8b6 to c67c37355de2e07cfe64988045d921c669c3cfbe

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

c67c373trac #15286: Rebase on 6.2.beta0

comment:18 Changed 5 years ago by chapoton

There is a useless "else" before the last return.

comment:19 Changed 5 years ago by ncohen

Indeed, but it works both ways so... :-P

comment:20 in reply to: ↑ 8 ; follow-up: Changed 5 years ago by rws

Replying to ncohen:

Because of your rebase on 6.1.beta2 I need some time to compile ;-(

Yeah, that's one of the problems with git.. If you just checkout an old patch, you recompile everything. Well, at least now your Sage is up to date ^^;

You must do a pull:

sage: dev.checkout(ticket='15286',base='develop')
...
(your local branch on top of develop is created)
...
sage: dev.pull('15286')
Merging the remote branch "u/ncohen/15286" into the local branch "ticket/15286".
Automatic merge successful.

#  (use "dev.commit()" to commit your merge)

comment:21 Changed 5 years ago by rws

  • Branch changed from u/ncohen/15286 to u/rws/latinsq
  • Created changed from 10/16/13 14:43:03 to 10/16/13 14:43:03
  • Modified changed from 02/24/14 14:59:57 to 02/24/14 14:59:57

comment:22 in reply to: ↑ 20 Changed 5 years ago by ncohen

  • Branch changed from u/rws/latinsq to u/ncohen/15286

You must do a pull:

Why did you change the branch ? If you need to do it, please give it a public/ name. And in particular, please set meaningful commit messages.

Nathann

comment:23 Changed 5 years ago by rws

Apologies and thanks---I didn't know about public/. BTW the docs built and look fine.

comment:24 Changed 5 years ago by vdelecroix

Hi there,

I finally was able to build the doc !

The branch cleanly merges on 6.2.beta4. I implemented a are_mutually_orthogonal_latin_squares. It is very naive but provide a small check to your function (if there is a better way tell me !). See u/vdelecroix/15286

I am happy with all what you did.

comment:25 Changed 5 years ago by ncohen

  • Branch changed from u/ncohen/15286 to u/vdelecroix/15286
  • Commit changed from c67c37355de2e07cfe64988045d921c669c3cfbe to 68cdedfb785eb077df4d6872c20b8de0749e09d0
  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

Wow !! Greaaaaaaaaaat, thank you so much !! I had almost forgotten about this long sequence of tickets.... :-)

Nathann


New commits:

dc84bd8Merge branch 'u/ncohen/15286' of git://trac.sagemath.org/sage into 15286
68cdedfremove the avoid section "Function" in the intial comment and implement

comment:26 Changed 5 years ago by vbraun

  • Branch changed from u/vdelecroix/15286 to 68cdedfb785eb077df4d6872c20b8de0749e09d0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.