Improved Efficiency of mutually_orthogonal_latin_squares function #39483
Conversation
pyrusbrawler64
changed the title
pyrusbrawler64
changed the title
|
Looks good, thanks. However, I think it would be good to add mention of the issue to your comment on line 386. I was thinking that you could add a new line something like "This fixes :issue:`26107`, which pointed out that the original method could be unacceptably slow in this case." Also, perhaps it would be good to add "for prime powers" at the end of the existing sentence. I will set to positive review on Thursday if |
|
|
|
Documentation preview for this PR (built with commit 739b9cd; changes) is ready! 🎉 |
|
There is a typo in line 386: change "primw" to "prime". Everything else looks good. |
|
Thanks. I'll set to positive review when the CI approves. |
|
The CI failure is I am seeing this failure elsewhere, so it should not be attributed to this PR. Therefore, I consider the CI to be successful. |
sagemathgh-39483: Improved Efficiency of mutually_orthogonal_latin_squares function <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Fixes sagemath#26107. When n is a prime power the construction used in the mutually_orthogonal_latin_squares function of latin_squares.py made all n-1 possible Mutually Orthogonal Latin Squares (MOLS) no matter how many were requested. This caused the function to take much longer than required when n was large and the number of MOLS requested was small. This was fixed by implementing a construction for prime powers n which only creates the number of MOLS requested by the user. A reference was added for this construction, and a doctest was added to verify it. Several more doctests were changed as the function now returns different MOLS when n is a prime power. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [X] The title is concise and informative. - [X] The description explains in detail what this PR is about. - [X] I have linked a relevant issue or discussion. - [X] I have created tests covering the changes. - [X] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39483 Reported by: Caleb Van't Land Reviewer(s):
Fixes #26107.
When n is a prime power the construction used in the mutually_orthogonal_latin_squares function of latin_squares.py made all n-1 possible Mutually Orthogonal Latin Squares (MOLS) no matter how many were requested. This caused the function to take much longer than required when n was large and the number of MOLS requested was small.
This was fixed by implementing a construction for prime powers n which only creates the number of MOLS requested by the user. A reference was added for this construction, and a doctest was added to verify it. Several more doctests were changed as the function now returns different MOLS when n is a prime power.
📝 Checklist
⌛ Dependencies