#20970: Gabidulin Codes
-------------------------------------+-------------------------------------
Reporter: arpitdm | Owner:
Type: enhancement | Status: new
Priority: major | Milestone: sage-7.3
Component: coding theory | Resolution:
Keywords: | Merged in:
Authors: Arpit Merchant | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/arpitdm/gabidulin_codes | ddaef4138cf29d70428a80539d249461226fbc9d
Dependencies: #13215 | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by dlucas):
Hello,
To answer your question (`4.`), for now, yes, I guess you need to have
such a method.
Later on, when the abstract class to manage rank-metric codes will be
implemented we can
replicate the design used in `AbstractLinearCode`, but for now, copy-
pasting `generator_matrix`
from `AbstractLinearCode` and put it in `GabidulinCode` is fine.
Some remarks:
1. You don't have to use backslashes to jump lines when you're in a
parenthesis/square bracket block.
For instance, you can write:
{{{
% (self.length(), self.dimension(), \
self.minimum_distance(), self.base_field())
}}}
as
{{{
% (self.length(), self.dimension(),
self.minimum_distance(), self.base_field())
}}}
2. Lines 19-34 are more or less copy-pasted from
`relative_finite_field_extension.py`, and are not necessary. Take lines
19-20 for instance: it's a carbon copy of lines 116-117 of
`relative_finite_field_extension.py` (except you replaced `must be` by
`has to be` in the error message)... Which basically means you run the
same sanity check twice: first time on lines 19-20 and second time line 49
when calling the constructing a `RelativeFiniteFieldExtension`. I think
you should just remove all those tests and let
`RelativeFiniteFieldExtension` do its job on sanitizing.
3. In the same vein: you copy-pasted a lot of getter from
`relative_finite_field_extension.py`, and I'm not sure it makes sense. The
user might be a bit puzzled by `absolute_field_power` which returns
something of little relevance wrt. the Gabidulin code. Furthermore, if the
user desperately needs to access this value, they can still access it by
the `RelativeFiniteFieldExtension` returned by
`relative_finite_field_extension` (this one we should keep).
4. There's some kind of naming conflict: `evaluation_points` and
`linearly_independent_elements` are the same (see line 68), but while the
user has to write `linearly_independent_elements=stuff` at construction
time, they have to write `my_code.evaluation_points()` to access them from
the constructed code. I think you should choose one name and stick to it.
I vote for `evaluation_points` for consistency with `grs.py`.
5. Remember to not call methods in for loops except it's absolutely
necessary. You do this on lines 96-97, 180-184 and 212. E.g. line 182, as
return value of `p.coefficients()` never changes throughout the
iterations, I would store it in a local variable and use this local
variable instead.
Otherwise it seems good.
Best,
David
--
Ticket URL: <https://trac.sagemath.org/ticket/20970#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.
