Opened 10 years ago
Closed 8 years ago
#12418 closed enhancement (fixed)
adding Delsarte bound for codes
Reported by:  dimpase  Owned by:  wdj 

Priority:  major  Milestone:  sage5.12 
Component:  coding theory  Keywords:  
Cc:  jpang, kini, wdj, ncohen, ppurka, ptrrsn_1  Merged in:  sage5.12.beta1 
Authors:  Dmitrii Pasechnik  Reviewers:  Frédéric Chapoton, Punarbasu Purkayastha 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  #12533, #13650  Stopgaps: 
Description (last modified by )
Delsarte bound for codes, aka Linear Programming bound, is easy to implement in Sage.
To work well in big dimensions, one needs an arbitrary precision LP solver. We use an LP backend to PPL, which is available in Sage since #12533.
Apply
Attachments (6)
Change History (47)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
 Cc kini added
comment:3 Changed 10 years ago by
 Cc wdj added
comment:4 Changed 10 years ago by
 Cc ncohen added
 Description modified (diff)
comment:5 Changed 10 years ago by
comment:6 Changed 10 years ago by
 Cc ppurka added
comment:7 Changed 10 years ago by
The function named delsarte_bound
should be renamed to something like delsarte_bound_hamming_space
. This is so that in future other functions like delsarte_bound_johnson_space
, delsarte_bound_permutation_space
, etc can be added easily, without having inconsistencies in naming.
comment:8 Changed 10 years ago by
 Cc ptrrsn_1 added
comment:9 Changed 9 years ago by
 Description modified (diff)
comment:10 Changed 9 years ago by
 Dependencies set to #12533
 Description modified (diff)
comment:11 Changed 9 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:12 followups: ↓ 14 ↓ 20 Changed 9 years ago by
I think the Krawtchouk
polynomial could be computed explicitly by not making repeated calls to binomial
. This should speed it up. I have something like this in mind:
def Krawtchouk2(n,q,l,i): # Use the expression in equation (55) of MacWilliams & Sloane, pg 151 # We write jth term = some_factor * (j1)th term kraw = jth_term = (q1)**l * binomial(n, l) # j=0 for j in range(1,l+1): jth_term *= q*(lj+1)*(ij+1)/((q1)*j*(nj+1)) kraw += jth_term return kraw n,q,l,i = 10,8,7,5 timeit('Krawtchouk2(n,q,l,i)') timeit('Krawtchouk (n,q,l,i)') print Krawtchouk2(n,q,l,i) == Krawtchouk(n,q,l,i) 625 loops, best of 3: 53.3 µs per loop 625 loops, best of 3: 205 µs per loop True
I noticed that sage handles nonintegral components in the binomial, so the expression for the Krawtchouk
already works with nonintegral n
and x
.
n,q,l,i = 10.6,8,7,5.4 #timeit('Krawtchouk3(n,q,l,i)') timeit('Krawtchouk2(n,q,l,i)') timeit('Krawtchouk (n,q,l,i)') print Krawtchouk2(n,q,l,i) == Krawtchouk(n,q,l,i) print Krawtchouk2(n,q,l,i), Krawtchouk(n,q,l,i) 625 loops, best of 3: 382 µs per loop 125 loops, best of 3: 4.74 ms per loop False 93582.0160001147 93582.0159999999
comment:13 Changed 9 years ago by
Can you mention when it guarantees a weight spectrum? Would doing an ILP
make it a proper weight spectrum?
 ``return_data``  if ``True``, return a weights vector, which actually need not be a proper weight enumerator, or even have integer entries, and the LP.
Also, I think the term weight enumerator
refers to the weight enumerator polynomial
. Perhaps using weight distribution
or distance distribution
is more appropriate here.
comment:14 in reply to: ↑ 12 ; followup: ↓ 16 Changed 9 years ago by
Replying to ppurka:
I think the
Krawtchouk
polynomial could be computed explicitly by not making repeated calls tobinomial
. This should speed it up.
It's probably even faster to compute by using recurrence relations, but I don't think it's important here: LP solving timing clearly dominates the rest.
By the way, would it be interesting to include an option to compute bounds on codes with a prescribed forbidden set of distances, rather than just [1..d] ? It's a trivial addon. I did this in a prototype code for Johnson schemes, here.
Any other interesting schemes to include? (Johnson scheme takes care of constant weight binary codes, as you know.)
comment:15 Changed 9 years ago by
 Dependencies changed from #12533 to #12533, #13650
 Description modified (diff)
 Milestone changed from sage5.4 to sage5.5
comment:16 in reply to: ↑ 14 ; followup: ↓ 18 Changed 9 years ago by
Replying to dimpase:
Replying to ppurka:
I think the
Krawtchouk
polynomial could be computed explicitly by not making repeated calls tobinomial
. This should speed it up.It's probably even faster to compute by using recurrence relations, but I don't think it's important here: LP solving timing clearly dominates the rest.
The point is that someone might try to use these polynomials more generally in a separate context. They are not defined anywhere else in Sage, so anyone who tries to use them will use this one.
By the way, would it be interesting to include an option to compute bounds on codes with a prescribed forbidden set of distances, rather than just [1..d] ? It's a trivial addon. I did this in a prototype code for Johnson schemes, here.
Wow! You have the Johnson scheme too?! Sure, add them all in!! Do you use the polynomials used by Aaltonen?
Any other interesting schemes to include? (Johnson scheme takes care of constant weight binary codes, as you know.)
LP for permutation codes would be interesting. There are not too many good results known there. IIRC, it uses Charlier polynomials.
EDIT: FWIW, it is Charlier polynomials.
comment:17 Changed 9 years ago by
Oh, I forgot to add. Forbidden distances will be nice as well. I think only some special cases achieve the closed form solutions. In general, still not much is known. It looks like you only need to drop distances d_1,...,d_m
instead of 1,...,d
, right?
How about introducing an extra keyword called forbidden_distances
or exclude_distances
, which defaults to 1,...,d
?
comment:18 in reply to: ↑ 16 ; followup: ↓ 19 Changed 9 years ago by
Replying to ppurka:
Replying to dimpase:
Replying to ppurka:
I think the
Krawtchouk
polynomial could be computed explicitly by not making repeated calls tobinomial
. This should speed it up.It's probably even faster to compute by using recurrence relations, but I don't think it's important here: LP solving timing clearly dominates the rest.
The point is that someone might try to use these polynomials more generally in a separate context. They are not defined anywhere else in Sage, so anyone who tries to use them will use this one.
Actually, I have most discrete orthogonal polynomials arising in the classical P and Q polynomial schemes implemented, although it's neither polished nor optimized.
[
By the way, would it be interesting to include an option to compute bounds on codes with a prescribed forbidden set of distances, rather than just [1..d] ? It's a trivial addon. I did this in a prototype code for Johnson schemes, here.
Wow! You have the Johnson scheme too?! Sure, add them all in!! Do you use the polynomials used by Aaltonen?
I use the descriptions in the book "Algebraic Combinatorics I" by E.Bannai and T.Ito. Something known as Eberlein polynomials.
Any other interesting schemes to include? (Johnson scheme takes care of constant weight binary codes, as you know.)
LP for permutation codes would be interesting. There are not too many good results known there. IIRC, it uses Chebychev polynomials(?).
yes, this should be perfectly doable.
comment:19 in reply to: ↑ 18 Changed 9 years ago by
Replying to dimpase:
I use the descriptions in the book "Algebraic Combinatorics I" by E.Bannai and T.Ito. Something known as Eberlein polynomials.
That's for the binary case. For the qary case it is a product of Krawtchouk and Hahn, if I recall properly. Let me fish out the paper; I will send it to you.
comment:20 in reply to: ↑ 12 ; followup: ↓ 21 Changed 9 years ago by
Replying to ppurka:
I think the
Krawtchouk
polynomial could be computed explicitly by not making repeated calls tobinomial
. This should speed it up. I have something like this in mind:
This can be further optimized by using Horner's rule. I'll do this, and leave the rest (other schemes) for another ticket, OK?
comment:21 in reply to: ↑ 20 Changed 9 years ago by
Replying to dimpase:
This can be further optimized by using Horner's rule. I'll do this, and leave the rest (other schemes) for another ticket, OK?
Yes, yes. One space/polynomial at a time. Just Hamming space in this ticket is OK.
comment:22 Changed 9 years ago by
 Description modified (diff)
Please review. I added a Kravchouck speedup, and cleaned up docstrings as requested.
comment:23 Changed 9 years ago by
improved docstrings for return_data
comment:24 followups: ↓ 25 ↓ 26 Changed 9 years ago by
Thanks for the update. I have some general comments. Will look into this patch in more detail too.
 There are lot of trailing whitespaces. The patchbot will complain. :)
 What is the point of this portion of the code? Can't it be replaced by
kk = ZZ(log(q, q_base))
?kk = 0 while q_base**kk < q: kk += 1
 There is another bit further down:
m = 1 while q_base**(m+1) < bd: m += 1 if q_base**(m+1) == bd: m += 1
comment:25 in reply to: ↑ 24 Changed 9 years ago by
Replying to ppurka:
Thanks for the update. I have some general comments. Will look into this patch in more detail too.
 There are lot of trailing whitespaces. The patchbot will complain. :)
I've just uploaded an update with all the trailing spaces removed.
 What is the point of this portion of the code? Can't it be replaced by
kk = ZZ(log(q, q_base))
?kk = 0 while q_base**kk < q: kk += 1 There is another bit further down:
m = 1 while q_base**(m+1) < bd: m += 1 if q_base**(m+1) == bd: m += 1
this came from an older piece of plain Python. Then I struggled with log() quite a bit, and finally gave up on it and rolled my own.
 Also, I don't think this deprecation is necessary any more. The ticket you cited is over 2 years old.
def dimension_upper_bound(n,d,q): +@rename_keyword(deprecation=6094, method="algorithm") +def dimension_upper_bound(n,d,q,algorithm=None):
I blindly copied from sage/coding/code_bounds.py
Should the whole file be cleaned out of these?
By the way, plural methods
slipped through this decorator...
Edit: Sorry. It seems
ZZ
doesn't work butint(log(..))
does work.
comment:26 in reply to: ↑ 24 Changed 9 years ago by
Replying to ppurka:
Thanks for the update. I have some general comments. Will look into this patch in more detail too.
 What is the point of this portion of the code? Can't it be replaced by
kk = ZZ(log(q, q_base))
?
yes, it is basically what it does; this is also needed to do a Gomorystyle cut which might be available due to the corresponding rounding.
comment:27 followup: ↓ 29 Changed 9 years ago by
Ok. I am finally getting some time to look into this again. Here are some comments.
Krawtchouk
is missing a doctest. There is no need of
\
indef delsarte_bound_...
 There are still many trailing whitespaces. If you use vim then you can try this command
:%s/[ ]\+$//
 ``q``  the length of the alphabet
 this should be "the size of the alphabet"
Also I think the the options
q
andd
can be swapped to be in the order in which they appear in the function definition.
 ``solver``  the LP/ILP solved
 this should be solver. What other solvers are present? They should be listed as options in this variable.The bound on the size of the F_2codes
 this should be`F_2`

 ``return_data``
 As it is currently written in the description, it is unclear what this is returning. It looks like the first component is an MIP variable (and not a vector), and the second component is the MILP itself. At a first glance in your doctests, it looks like we need to understand how the MILP works in order to get the values of the weight distribution. Should we just return the weight distribution itself? A weight distribution vector can be returned by doingp.get_values(a).values()
.  The backend should automatically handle "isinteger=True" at least so that it is functional. Currently I get a very generic Exception (why is this only "Exception"?). Maybe it is automatically handled in a later version of sage? I will have to check against 5.6.beta1 and higher:
Exception: This backend does not handle integer variables ! Read the doc !
 Why do we need the second function? We already discussed this offticket  I will wait for your generic function.
(**this option is currenly disabled, cf. trac #13667**).
 it should be:trac:`13667`
since it will be in the documentation.Parameter "algorithm" has the same meaning as in codesize_upper_bound()
 This should be:func:`codesize_upper_bound`
.print "Wrong q_base=", q_base, " for q=", q, kk
 This can be formatted python3 style asprint "Wrong q_base={} for q={} {}".format(q_base, q, kk)
 According to the patchbot this needs rebasing against some higher version of 5.6. I have only beta0 here; I will check it against rc1 after I have compiled that.
 EDIT: I forgot.. the deprecation notice should go. It has been two years. If code_bounds needs to be cleaned then I can do that.
EDIT: Update backticks in 6., 10., 11.
comment:28 Changed 9 years ago by
 Status changed from needs_review to needs_work
This patch needs to address referee's comments. I am changing this to needs_work
in the meanwhile.
comment:29 in reply to: ↑ 27 Changed 8 years ago by
Replying to ppurka:
rebased to Sage 5.10 and fixed the following:
Krawtchouk
is missing a doctest. There is no need of
\
indef delsarte_bound_...
 There are still many trailing whitespaces. If you use vim then you can try this command
:%s/[ ]\+$//
 ``q``  the length of the alphabet
 this should be "the size of the alphabet"Also I think the the options
q
andd
can be swapped to be in the order in which they appear in the function definition.
I swapped the docstrings instead.
 ``solver``  the LP/ILP solved
 this should be solver. What other solvers are present? They should be listed as options in this variable.The bound on the size of the F_2codes
 this should be`F_2`
(and other F_
)
 The backend should automatically handle "isinteger=True" at least so that it is functional. Currently I get a very generic Exception (why is this only "Exception"?). Maybe it is automatically handled in a later version of sage? I will have to check against 5.6.beta1 and higher:
The PPL backend does not handle ILP. One might want to improve the way it reports this error, but not on this ticket.
Exception: This backend does not handle integer variables ! Read the doc !
Parameter "algorithm" has the same meaning as in codesize_upper_bound()
 This should be:func:`codesize_upper_bound`
. According to the patchbot this needs rebasing against some higher version of 5.6. I have only beta0 here; I will check it against rc1 after I have compiled that.
Rebased.
comment:30 followup: ↓ 31 Changed 8 years ago by
 Work issues set to code refactoring
comment:31 in reply to: ↑ 30 Changed 8 years ago by
Replying to dimpase:
Work issues set to code refactoring
Oh good. I was wondering if you didn't want to do that in this patch.
comment:32 Changed 8 years ago by
 Status changed from needs_work to needs_review
 Work issues code refactoring deleted
comment:33 Changed 8 years ago by
instructions for the bot:
apply 12418_delsart_bounds.patch
comment:34 followup: ↓ 35 Changed 8 years ago by
 Status changed from needs_review to needs_work
 doctest covering is not 100%
 you can use the wikipedia role for example
:wikipedia:`Togo`
 it would be better to lazy_import the new functions, maybe ?
comment:35 in reply to: ↑ 34 Changed 8 years ago by
Replying to chapoton:
 doctest covering is not 100%
hmm, what function do you mean? there is an internal function which is not exported; I don't think it needs a doctest, does it?
 you can use the wikipedia role for example
:wikipedia:`Togo`
where?
 it would be better to lazy_import the new functions, maybe ?
I have no idea. Is there a stated policy on this?
Having said this, perhaps it's better to relazy_import the whole sage/coding
, something for another ticket?
comment:36 Changed 8 years ago by
 well, the bot complains about the missing doctest, so I guess that the internal function needs one indeed
 instead of
`en.wikipedia.org/wiki/Kravchuk_polynomials <http://en.wikipedia.org/wiki/Kravchuk_polynomials>`_
you can write :wikipedia:`Kravchuk_polynomials`
 the bot is not happy either on adding something new in the global namespace. It is better for the startup time of sage to try and make the bot happy on this point, imho.
comment:37 Changed 8 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:38 Changed 8 years ago by
 Status changed from needs_review to positive_review
The patch looks OK to me now. Though I would have really liked the Qmatrix to be passed to the linear program (for instance to the delsarte LP building function), this can be done in a future patch when LP for other spaces are introduced.
comment:39 Changed 8 years ago by
 Reviewers set to Frédéric Chapoton, Punarbasu Purkayastha
comment:40 Changed 8 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:41 Changed 8 years ago by
 Merged in set to sage5.12.beta1
 Resolution set to fixed
 Status changed from positive_review to closed
(GLPK can solve noninteger rational LP. It is not exposed, but may not be too hard either)