#18376: New encoding structure for linear codes
-------------------------+-------------------------------------------------
Reporter: | Owner:
dlucas | Status: needs_review
Type: | Milestone: sage-6.8
enhancement | Resolution:
Priority: major | Merged in:
Component: | Reviewers:
coding theory | Work issues:
Keywords: | Commit:
Authors: David | aa42238c6754cff63c5cc42e8d77e78b36074fbc
Lucas | Stopgaps:
Report Upstream: N/A |
Branch: |
u/dlucas/encoder |
Dependencies: |
-------------------------+-------------------------------------------------
Changes (by dlucas):
* status: needs_work => needs_review
Comment:
Hello!
>Here are my remarks/questions while I was looking for the first time at
your
>code. Let's get this thing in so that you are not blocked for the more
>mathematical parts.
Thanks a lot for this, and for being that thorough! It's exactly the kind
of comments we were hoping for.
I fixed the typos and small mistakes you noticed.
Here are some answers to your questions:
>To import these names into the global namespace, use: --> it sems that
the code is *already* in the >global namespace?...
It shouldn't be available in global namespace. I fixed that.
>`encoder_default_name` -> `default_encoder`?
I kept `encoder_default_name` as it's the name of the encoder (the string
that points to it), and not the object in itself.
>How is this
{{{
if hasattr(self.code(), "_generator_matrix"):
return self.code()._generator_matrix
else:
return self.code().generator_matrix()
}}}
>better than return `self.code().generator_matrix()`?
This encoder is designed to put generic linear codes (`LinearCode` class)
into the new framework. And in `LinearCode`, you have a class parameter
`_generator_matrix` because one has to provide a generator matrix when he
build a "structureless" linear code.
As `generator_matrix()` method in `AbstractLinearCode` returns the
generator matrix given by the default encoder, which is, for these
`LinearCode`s the one you pointed out, if this encoder had this
`self.code().generator_matrix()` in its `generator_matrix()` method, it
will lead to an infinite loop when calling
`LinearCode.generator_matrix()`.
>`information_set` takes nothing as argument but calls `generator_matrix`
which can depend on an encoder. Could you expose that flag and document
it? That's the price of exposing in `LinearCode` the methods which are
defined only in its encoder object.. It is a bit awkward `:-/`
Actually, the information set can be computed from any generator matrix,
and it will always return the same result whatever the generator matrix I
used to compute it. As the default encoder of a code has a valid
`generator_matrix` method, and as `C.generator_matrix()` calls the default
encoder, the code of `information_set()` will always return a valid
result.
>Really, these methods `encode` and `unencode` have nothing to do in
`LinearCode`. You should "accept your own design" and have all these
things be available only in the `Encoder` object, otherwise there will be
a crazy amount of copy/paste with those arguments.
I do think they should be there. A lot of users won't care about the kind
of encoding/decoding which will be used underneath as long as they get
what they expect. For these users, being able to directly call
`MyCode.encode(m)` or `MyCode.unencode(c)` instead of having to ask for
the list of available encoders, pick the one they want, create it, and
call `encode` or `unencode` on it is a really helpful feature.
>I do not understand why the `Encoder` class provides methods to
encode/decode linear codes from the matrix, given that you cannot be sure
that a matrix is available. Shouldn't this be in
`LinearCodeGeneratorMatrixEncoder`?
It actually provides a default implementation that will avoid spending
time (and repeating code) in case of you have a generator matrix for your
code. If you know a generator matrix for the encoder you're writing, you
just have to implement `generator_matrix` and you get immediately `encode`
and `unencode`.
If you don't have one, you have to override these methods in your encoder.
It's just a way to make developer's life easier `:)`
>Instead of `encoders_available`, what about simply encoders? I do not see
the added value of `_available` there. Furthermore, what about returning a
copy of the inner dictionary? Given that you can use a dictionary as you
would use a list `(list(my_dict)`, `for x in my_dict`, `if x in my_dict)`
why wouldn't we return everything at once?
In `AbstractLinearCode`, we already provide a method called `encode` and
another called `encoder`. I think adding a third one called `encoders`
might lead to some confusion amongst the users, especially as `encoder`
returns an `Encoder` object, one might think `encoders` will return a list
of all possible `Encoder`s objects available for the provided code. Plus,
I think the `_available` actually adds value, by meaning "this is the list
of all encoders you can access with your linear code".
About the dictionary, if I just want to build a specific encoder by using
`C.encoder(name='mySpecificName`), all I care about is actually the list
of names of all the encoders my `C` can access to. I think returning the
whole dictionary might be a bit confusing, as one will recover the objects
as well as the string names representing them, but he only needs the
string names to build them (thanks to the `encoder` method).
Now, about your idea of having a `set_encoder` method. There (imho)
several problems with that, namely:
- the existence of `_default_encoder_name` as a class argument allow all
the methods of `LinearCode` that rely on a generator matrix to work.
Indeed, as they call `generator_matrix()` without any argument in their
bodies, and as we always set an encoder which knows a generator matrix as
a default encoder, all these methods will always work. Now, if you allow
the user to dynamically change this, and if he changes the encoder to
another which does not has a generator matrix, all these methods will
fail... Which is a bit silly, because what we only care about when the
message space is not a vector space are the differences when it comes to
encoding/unencoding, we don't want that to change the behaviour of other
methods.
- also, it will lead to the deletion of these "name" arguments, as you
said. So if I want to test different encoding styles (or later on,
decoding algorithms) I will have to reset a new encoder everytime. Of
course it's only an extra line of code everytime, and of course encoders
are cached so it's not heavy (memory-wise), but it still adds a bit of
heaviness in the process.
So I'd prefer to keep it as is. And in that case, I also vote to keep
`add_encoder` method.
>I am now in Nantes. It took me a Nice->Nantes flight to work on this
review. I
>love this city. Plus the weather is good but not as hot as in the
>south. Cooooooool.
Sounds nice, enjoy your vacations!
David
--
Ticket URL: <http://trac.sagemath.org/ticket/18376#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 http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
