Opened 6 years ago
Closed 6 years ago
#16868 closed enhancement (fixed)
A real difference matrix has k columns
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.4 
Component:  combinatorial designs  Keywords:  
Cc:  vdelecroix  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  d6a2615 (Commits)  Commit:  d6a2615ffe4e4be01f6270ada7c450af250ad23c 
Dependencies:  #16846  Stopgaps: 
Description
Change Sage's standard to be coherent with OA which have k columns. This is much easier to work with on a computer, for it is much easier to add a row to a matrix than to add a column. And this is what we have to do most of the time.
Nathann
Change History (10)
comment:1 Changed 6 years ago by
 Branch set to public/16868
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Commit set to 361c4034247f4b3944dbb52a1259b184dfc6a250
comment:3 followup: ↓ 4 Changed 6 years ago by
I thought it was better standardization but in OA_from_quasi_difference_matrix
you have
# Each line is expanded by [g+x for x in line for g in G] then relabeled # with integers. Missing values are also handled. new_M = [] for line in izip(*M): ... ... new_M = zip(*new_M)
what's the point?
Vincent
comment:4 in reply to: ↑ 3 Changed 6 years ago by
Replying to vdelecroix:
I thought it was better standardization but in
OA_from_quasi_difference_matrix
you have# Each line is expanded by [g+x for x in line for g in G] then relabeled # with integers. Missing values are also handled. new_M = [] for line in izip(*M): ... ... new_M = zip(*new_M)what's the point?
Well, first if you only have one "zip*" in the whole library then the point is reached. I looked at that part of the code and I felt that the best way to implement it required two calls to zip, but for all others applications (functions in database.py
) it is easier to work on this version of the matrix, for one constantly add rows, not columns.
So even if there is a "zip*" somewhere, the rest of the code is easier to write/read.
Nathann
comment:5 Changed 6 years ago by
 Commit changed from 361c4034247f4b3944dbb52a1259b184dfc6a250 to d6a2615ffe4e4be01f6270ada7c450af250ad23c
Branch pushed to git repo; I updated commit sha1. New commits:
d6a2615  trac #16868: Mb was initialized twice

comment:6 Changed 6 years ago by
 Status changed from needs_review to positive_review
A small commit where I removed a line.
Vincent
comment:7 Changed 6 years ago by
I have non strong opposition against that :P
Thanks !
Nathann
comment:9 Changed 6 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_work to positive_review
Oh yeah!
comment:10 Changed 6 years ago by
 Branch changed from public/16868 to d6a2615ffe4e4be01f6270ada7c450af250ad23c
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
trac #16864: designs/database.py: autogenerated doc index
trac #16864: use Python string .format() + doc
trac #16817: OA for n=205,254,469,520,522,524,1262
trac #16817: docfix
trac #16846: a difference_matrices module
trac #16846: Remove obsolete functions
trac #16846: Broken doctests
trac #16846: Merge with updated #16817
trac #16846: review
trac #16868: A real difference matrix has k columns