Opened 5 years ago

Closed 5 years ago

#16868 closed enhancement (fixed)

A real difference matrix has k columns

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: 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 5 years ago by ncohen

  • Branch set to public/16868
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by git

  • Commit set to 361c4034247f4b3944dbb52a1259b184dfc6a250

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

5b8500btrac #16864: designs/database.py: auto-generated doc index
bfbad46trac #16864: use Python string .format() + doc
3ca48dctrac #16817: OA for n=205,254,469,520,522,524,1262
f9e6569trac #16817: docfix
c92e9bftrac #16846: a difference_matrices module
fdab06etrac #16846: Remove obsolete functions
f7afe6ftrac #16846: Broken doctests
fd1bbc6trac #16846: Merge with updated #16817
c66c19btrac #16846: review
361c403trac #16868: A real difference matrix has k columns

comment:3 follow-up: Changed 5 years ago by 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?

Vincent

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

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 5 years ago by git

  • Commit changed from 361c4034247f4b3944dbb52a1259b184dfc6a250 to d6a2615ffe4e4be01f6270ada7c450af250ad23c

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

d6a2615trac #16868: Mb was initialized twice

comment:6 Changed 5 years ago by vdelecroix

  • Status changed from needs_review to positive_review

A small commit where I removed a line.

Vincent

comment:7 Changed 5 years ago by ncohen

I have non strong opposition against that :-P

Thanks !

Nathann

comment:8 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name

comment:9 Changed 5 years ago by vdelecroix

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

Oh yeah!

comment:10 Changed 5 years ago by vbraun

  • Branch changed from public/16868 to d6a2615ffe4e4be01f6270ada7c450af250ad23c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.