Opened 7 years ago
Closed 7 years ago
#15286 closed enhancement (fixed)
Latin squares
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.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 7 years ago by
 Branch changed from #15285 to u/ncohen/15286
 Dependencies set to #15285
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
 Commit set to d93abcd13bd5230bafce89a73f553bd60e4c7aa0
comment:3 Changed 7 years ago by
 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 followup: ↓ 5 Changed 7 years ago by
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 7 years ago by
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 n1 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 7 years ago by
 Commit changed from a8a2a719d1f0fc4aedf24f24fc2c87e407182da4 to e02d42c390b8f3841cea5548abb1588efb6d99a3
comment:7 followup: ↓ 8 Changed 7 years ago by
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 ; followup: ↓ 20 Changed 7 years ago by
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 7 years ago by
 Commit changed from e02d42c390b8f3841cea5548abb1588efb6d99a3 to 47df907cf1b7491b4ed2365e939101d9c6d15581
Branch pushed to git repo; I updated commit sha1. New commits:
47df907  trac #15286: Reviewer's comments 2

comment:10 Changed 7 years ago by
(this patch's only dependency is now positively reviewed)
comment:11 followup: ↓ 12 Changed 7 years ago by
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 7 years ago by
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 7 years ago by
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 7 years ago by
 Commit changed from 47df907cf1b7491b4ed2365e939101d9c6d15581 to e0e44f1d789d43e7d60186cd435b334928aeb8b6
comment:15 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:16 Changed 7 years ago by
The dependency of this ticket has been reviewed.
Nathann
comment:17 Changed 7 years ago by
 Commit changed from e0e44f1d789d43e7d60186cd435b334928aeb8b6 to c67c37355de2e07cfe64988045d921c669c3cfbe
Branch pushed to git repo; I updated commit sha1. New commits:
c67c373  trac #15286: Rebase on 6.2.beta0

comment:18 Changed 7 years ago by
There is a useless "else" before the last return.
comment:19 Changed 7 years ago by
Indeed, but it works both ways so... :P
comment:20 in reply to: ↑ 8 ; followup: ↓ 22 Changed 7 years ago by
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 7 years ago by
 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 7 years ago by
 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 7 years ago by
Apologies and thanksI didn't know about public/
. BTW the docs built and look fine.
comment:24 Changed 7 years ago by
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 7 years ago by
 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
comment:26 Changed 7 years ago by
 Branch changed from u/vdelecroix/15286 to 68cdedfb785eb077df4d6872c20b8de0749e09d0
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits: