#20938: Decoder for Reed Muller Codes
-------------------------------------+-------------------------------------
Reporter: panda314 | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-7.3
Component: coding theory | Resolution:
Keywords: | Merged in:
Authors: Parthasarathi | Reviewers:
Panda |
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/dlucas/decoder_for_reed_muller_codes|
86c1629afaa869dd2086b6dd569939a66e3e4ad4
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by dlucas):
* status: new => needs_review
* commit: => 86c1629afaa869dd2086b6dd569939a66e3e4ad4
Comment:
Hello,
I pushed a new version of your branched merged with latest version (there
was a conflict and I
allowed me to fix it myself).
I know this ticket was ready for review because you told me so by email,
but would you please
use ticket states from now on? It makes the process smoother for everyone.
Here are my comments on reviewing your work:
- a lot of lines are not in PEP8-compliance: for instance, you write `a=b`
where it should be `a = b`. You can check the rules regarding whitespaces
and operators
[http://doc.sagemath.org/html/en/developer/coding_basics.html#python-code-
style here].
- There are some very long lines (95+ characters) in your file. See lines
1212-1215 for instance.
While I'm not adamant that the 80 characters rule should be followed
everywhere (I tend to accept 85 characters), I think 95+ is too much.
- In RSDecoder's constructor (line 1241), you copy-pasted the sentence
illustrating your `TESTS` block from the class above. It should not
mention Binary Reed-Muller code, but RM code, as it works for any RM code.
- While I'm at it, you wrote ` if not isinstance(code,
QAryReedMullerCode)`, but this works with any `q`, even `2`, doesn't it?
Because if you try to build a RSDecoder with a binary RM code, it fails...
- Same decoder: is there a reason to enforce the use of Gao decoder? I
might be wrong,
but I think that as long as you manage to build your GRS code, you can
use any decoder over it.
Hence, I suggest to use the same kind of mechanism I used with
`PuncturedCode`, `SubfieldSubcode`,
`ExtendedCode` etc:
allow the user to pass an instance of a decoder over the GRS code as
input to RM code's decoder.
Then use this decoder to perform decoding. This triggers some changes in
your design: first, you
have to implement a function to get the GRS code from a RM code, instead
of computing it inplace
in `decode_to_code`. I think it's nice to have such a feature anyway.
Then, you have to add some input sanitization checks at construction time
(which will ensure the
decoder provided by the user is a decoder over the GRS code). You will
also have to change
`decoding_radius`, which now returns the result of `GRS's
decoder.decoding_radius()`. And it also
means `decoder_type` is now `{dynamic}`.
I still need to read carefully your implementation of the majority vote
decoder.
Otherwise, it looks quite good. Your documentation, especially, is well
written and explains
very well to the user how each decoder works.
Best,
David
----
New commits:
||[https://git.sagemath.org/sage.git/commit?id=b1642f8070adddf5d1a76e580b9b72faeea69794
b1642f8]||{{{adding decoders for reed muller code}}}||
||[https://git.sagemath.org/sage.git/commit?id=699b2e7496def5c155c53dbcc3b88a5c570631a3
699b2e7]||{{{removing some unecessary imports}}}||
||[https://git.sagemath.org/sage.git/commit?id=86c1629afaa869dd2086b6dd569939a66e3e4ad4
86c1629]||{{{Updated to latest version and fixed conflict}}}||
--
Ticket URL: <https://trac.sagemath.org/ticket/20938#comment:3>
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.
