#20705: Classes for Reed Muller Codes
-------------------------------------+-------------------------------------
Reporter: panda314 | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-7.3
Component: coding theory | Resolution:
Keywords: | Merged in:
Authors: | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/panda314/classes_for_reed_muller_codes|
5129c915c146098f2c9c03ce89ca992b32e86609
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by panda314):
Replying to [comment:16 dlucas]:
Hi,
So should we keep BinaryReedMullerCode in codes_catalog.py in keeping with
the BinaryReedMullerCode() function from guava.py? Or should i just keep
the guava.py implementation with a warning regarding it's depreciation?
> Hello,
>
> Some more comments:
>
> - I noticed you add both `ReedMullerCode`, `BinaryReedMullerCode` and
`QAryReedMullerCode` to `codes_catalog.py`. But with our design, only
`ReedMullerCode` should be in this file, as the user should never access
the classes themselves.
>
>
> == `_multivariate_polynomial_interpolation` ==
>
> - You left a reference to `_R` in the docstring while it no longer
exists
> - Is the base field (and the cardinality) really needed? I would rather
remove them from the list of inputs and recover them from `evaluation`.
This way, one has to provide `evaluation` as a vector over a finite field,
you get this finite field by doing `F = evaluation.base_ring()`, you check
that `F` is the base ring of `polynomial_ring` and that's it. You can get
`q` with the appropriate call on `F`.
> - As my previous comment implies, I would only allow `evaluation` to be
a vector (and not a list). After all, it's a private method you only use
to recover the original message as a polynomial from a codeword, and the
latter is necessarily a vector, isn't it?
> - l. 105: is there a way to avoid a call to `base_field.list()`? I know
that in most cases, this list will be rather small, but it's not really
efficient to build a full list (especially in a recursive pattern, because
you will build it a lot of times...). This time again, I'd suggest an
iterator over the finite field.
>
> == `ReedMullerCode` ==
>
> - the `WARNING` block should be added in the module documentation as
well. The syntax on this block is the following
> {{{
> .. WARNING::
> body of the block
> }}}
> See `linear_code.py`, line 682 for an example.
> I would also change the message. I propose `For now, this implementation
only supports Reed-Muller codes whose order is less than q`. Such a
message implies it will be extended later, I think it's more positive this
way ;)
>
> == Q-ary and Binary classes ==
>
> - I think it's not necessary to save `q` as a class parameter. The user
can access it by typing `C.base_field().cardinality()`, it's not something
specific to the class (or something hard to "compute"), so it should not
be saved.
>
> - I prefer full names for user-accessed methods. I'm okay with `num_var`
as an internal variable, but its getter should be called
`number_of_variables` instead.
>
> == Vectorial encoder ==
>
> - In `generator_matrix`, you're calling `self.code()` a lot of times. I
think you should store it in a local variable which will reduce the amount
of calls to this method.
>
> Best,
>
> David
--
Ticket URL: <http://trac.sagemath.org/ticket/20705#comment:17>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
