#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/panda314/decoder_for_reed_muller_codes|
d158aad633a3902c523957dcfa5d0b0b639dc813
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by dlucas):
Hello,
> Sorry for delay. Internet problems.
No worries, it's fine :)
I have a few remarks:
- I'd prefer to have `ReedSolomonSupercode` as a class method and not a
global method. I think it's a a bit more intuitive to write
`C.ReedSolomonSupercode()`. Also, the user will be able to find this
method by typing `C.<tab>` in a terminal, which is nice. But this raises
an issue: as we have *two* classes for Reed-Muller codes, there will be
code duplication... Thus I suggest to make the method you wrote a private
method for this module only. And then, in both `QAry` and `Binary` you add
a new method, let's say `reed_solomon_supercode(self, p = None)` which
returns `ReedSolomonSupercode(self, p)`. You still have some duplication,
but it's a bit better as you don't copy-paste the whole method twice.
- Still talking about `ReedSolomonSupercode`, I find its documentation
rather uninformative: could you please add a few more lines to explain how
this Reed-Solomon code is built, and what it is wrt. the Reed-Muller code?
Also, for `code`, you wrote `A Reed-Muller code of appropiate order.`.
What does `appropriate order` mean?
- And last one on `ReedSolomonSupercode`: you forgot to add `::` at the
end of you sentence in the `EXAMPLES` block.
- In the Supercode decoder, two methods (`reed_solomon_supercode` and
`reed_solomon_decoder`) are not documented.
- The Supercode decoder should change its types at construction time for
the types of the RS decoder.
You can copy-paste lines 347-349 from `extended_code.py` if you want
`:)`.
- `decoding_radius` in the Supercode decoder should take both `*args` and
`**kwargs` as input and pass these to the RS decoder's `decoding_radius`.
For example, RS error-erasure decoder requires to pass the number of
erasures as an argument of `decoding_radius`, so for now, calling
`decoding_radius` from the supercode decoder with error-erasure as RS
decoder fails.
- Since #20840, it's no longer necessary to manually add generic decoders
to the list of `_registered_decoders`. So you can remove the registration
lines related to Syndrome decoder.
- As it can be long to compute, and it might be called quite often, I
suggest to make `_list_polynomial` a cached method.
- Can you write a few more lines of documentation to `_set_to_mask`? I
think it's a bit hard to understand what it does just by reading the doc
for now.
- In `_list_polynomial`, I'd write `Return the list of all polynomials of
degree etc` instead of `Lists all polynomials of degree etc` to be
consistent with the usual way of writing these sentences.
> I have started my full-time employment. So i will not be able to work as
quickly.
Sure, don't worry! Take all the time you need.
David
--
Ticket URL: <https://trac.sagemath.org/ticket/20938#comment:8>
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.
