Opened 4 years ago

Closed 4 years ago

#16720 closed enhancement (fixed)

Speedup for OA_from_quasi_difference_matrix

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.4
Component: combinatorial designs Keywords:
Cc: vdelecroix Merged in:
Authors: Nathann Cohen Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 41676cf (Commits) Commit: 41676cf91c149c3de0b7c7f1e597ce3ac4d6addd
Dependencies: Stopgaps:

Description (last modified by ncohen)

Turns out that the use of a brain is underrated. I should really buy one someday.

This is what happens when you think before writing a loop:

sage -t --long database.py
    [340 tests, 3.20 s]

This is what happens when you do not

sage -t --long database.py
    [340 tests, 14.43 s]

Change History (11)

comment:1 Changed 4 years ago by ncohen

  • Branch set to u/ncohen/16720
  • Status changed from new to needs_review

comment:2 Changed 4 years ago by ncohen

  • Description modified (diff)

comment:3 Changed 4 years ago by git

  • Commit set to 26975dcef9a06284d32cd85b40bb1551dd2464f1

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

26975dctrac #16720: Speedup for OA_from_quasi_difference_matrix

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

Hello,

I do not understand that

-    M = M[:len(M)/2] # only develop w.r.t the last two coordinates
+    M = [M[i*18+j] for i in range(len(M)/18) for j in range(9)] # only develop w.r.t the last two coordinates

Vincent

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

Yoooooooo !

I do not understand that

-    M = M[:len(M)/2] # only develop w.r.t the last two coordinates
+    M = [M[i*18+j] for i in range(len(M)/18) for j in range(9)] # only develop w.r.t the last two coordinates

Ahem :-PPPPP

Thaaaaaaaaat's because the rows of the matrix were listed as 111222333 before and now it is 123123123. You don't care about that in an OA or a difference matrix, so it does not matter and it is easier to get this speedup.

The function which I had to modify above called this routine, and only needed to remember the first half of the matrix. Formerly. But now that the rows are listed differently, well getting that "half" of the matrix is a bit different :-P

I know it is not "semantically correct". Please don't make me rewrite the function for that T_T

Nathann

comment:6 Changed 4 years ago by git

  • Commit changed from 26975dcef9a06284d32cd85b40bb1551dd2464f1 to 41676cf91c149c3de0b7c7f1e597ce3ac4d6addd

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

f5ea70dtrac #16720: Merged with 6.3.rc1
41676cftrac #16720: A clearer line

comment:7 Changed 4 years ago by ncohen

This is clearer.

Nathann

comment:8 Changed 4 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

Ok, good!

Vincent

comment:9 Changed 4 years ago by ncohen

THaaaaaaaaaaaaaaaaaaaaaaanks !!

comment:10 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 Changed 4 years ago by vbraun

  • Branch changed from u/ncohen/16720 to 41676cf91c149c3de0b7c7f1e597ce3ac4d6addd
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.