#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 dlucas):
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:16>
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.
