#12418: adding Delsarte bound for codes
----------------------------------+-----------------------------------------
Reporter: dimpase | Owner: wdj
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-5.7
Component: coding theory | Resolution:
Keywords: | Work issues:
Report Upstream: N/A | Reviewers:
Authors: | Merged in:
Dependencies: #12533, #13650 | Stopgaps:
----------------------------------+-----------------------------------------
Comment (by ppurka):
Ok. I am finally getting some time to look into this again. Here are some
comments.
1. `Krawtchouk` is missing a doctest.
2. There is no need of `\` in `def delsarte_bound_...`
3. There are still many trailing whitespaces. If you use vim then you can
try this command `:%s/[ ]\+$//`
4. {{{ - ``q`` -- the length of the alphabet}}} -- this should be "the
''size'' of the alphabet"
Also I think the the options `q` and `d` can be swapped to be in the
order in which they appear
in the function definition.
5. {{{- ``solver`` -- the LP/ILP solved }}} --- this should be ''solver''.
What other solvers are present? They should be listed as options in
this
variable.
6. {{{The bound on the size of the F_2-codes}}} --- this should be
''`F_2`''
7. {{{ - ``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 doing
`p.get_values(a).values()`.
8. 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 !
}}}
9. Why do we need the second function? We already discussed this off-
ticket - I will wait for your generic function.
10. {{{(**this option is currenly disabled, cf. trac #13667**). }}} --- it
should be ''`:trac:13667`'' since it will be in the documentation.
11. {{{ Parameter "algorithm" has the same meaning as in
codesize_upper_bound() }}} --- This should be
''`:func:codesize_upper_bound`''.
12. {{{ print "Wrong q_base=", q_base, " for q=", q, kk}}} --- This
can be formatted python3 style as
{{{
print "Wrong q_base={} for q={} {}".format(q_base, q, kk)
}}}
13. 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.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12418#comment:27>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
