Prepare linear_code for inheritance

[dlucas]

For now, every family of linear code (eg: Hamming code) is a method which returns a LinearCode object. It would be nice to change this: every family of code should be an object. Besides, every linear codes needs to know its generator matrix to be constructed. This is fine for linear codes without a specific algebraic structure, but not for sub-families of linear codes. For instance, it is possible to encode & decode words in Reed-Solomon codes without the help of a generator matrix. With regards to this, the user should be free to build the generator matrix for sub-families of code (which can be both a time- and memory-consuming operation). This is also true for the dimension of a code (which can be long to compute for some sub-families).

However, some parameters (like the length of the code, or its base field) are mandatory to every linear code, and subfamilies. They need to work fine with the category framework as well.

We then propose the following design:

implement an abstract class , AbstractLinearCode, which will initialize parameters used in every linear code, and make linear codes properly interact as modules in the category framework in its constructor. Besides, as all methods that were previously in linear codes need to work for all subfamilies of codes, we propose to relocate them as methods of AbtractLinearCode. With this design, every linear code and subfamily will only need to inherit from this abstract class to get all the generic methods and parameters initialized.

Besides, linear codes get their base_ring method from their category. For better consistency, we propose to implement a base_field() method which will be specific to linear codes.

[dlucas]

New commits:

​4d7f14d	New method: _init_linear_code
​bf70359	New method: base_field

[ncohen]

Helloooooooooooooooooooo !

Several comments:

• I do not understand why you created a specific function _init_linear_code instead of writing what you need in the __init__ function. In particular the names __init__ and _init_linear_code do not indicate how their aims are different.
  • If you want to change the name of the function, could it mention categories explicitly? (Or is there a reason not to?)
  • If you decide to remove this function, please move the doctests that it contains to the __init__ method.
• You say in the ticket's description that you add a base_ring method while you implement base_field. What is the correct version?
• I do not understand why you implement this function while saying that it is 'inherited'.
  • If it is inherited, why do you need to implement it?
  • If it is *not* inherited, then shouldn't we fix that instead of implementing the function?

• We usually type '::' at the end of the line. Is there any reason why you write your code this way?

  We now create a member of our newly made class
  ::
  

Note that writing our newly made class:: will appear in the HTML as our newly made class: (only one ':'). If you do not want to see this terminal ':' then a space is sufficient, e.g.: This sentence ends here. ::

Nathann

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​a425a89

Changed structure of file: now contains an abstract class AbstractLinearCode for linear codes

[dlucas]

Okay then!

I changed the structure of the file: we now have an AbstractLinearCode class which has all methods that was previously in LinearCode. LinearCode now inherits from this new abstract class (and gets all its former methods that way). I fixed the :: stuff too.

David

[ncohen]

Helloooooooooooooooooooooo,

Several comments:

• (superficial) The way you moved code around makes the commit much more difficult to read than it should. It is a good thing to quickly read the commits before you push them, as usually the reviewer has to "reverse-engineer" what you want to do from the diff file. If you are forced to move code around, it is better to do so in a specific commit which *only* moves the code without changing anything in it.

As a reviewer, we have to make sure that all changes are correct, and as moving code around appears to us as 1) remove the code 2) add the code we have to re-read and check all of it, even if it was a no-brainer change on your side.

• I changed your toy example a bit so that the matrix is given as argument. I hope that you will not mind.

• While modifying minimum_distance() and gens() you interfered with its caching mechanism: answers which used to be cached are now forgotten. Could you fix that?

• The doctest you wrote for the abstract class takes a lot of time, and we try to not go over a couple of seconds if we can avoid it (even for long ones). As this one does not test a very fundamental feature, could you make it a bit faster by changing the figures?

Thanks,

I added a small commit at public/18099. The usual procedure is that, as I reviewed your changes, you can be the reviewer of mine. In particular, feel free to discuss/oppose any of the changes.

Nathann

[jsrn]

Also, remember to change the description of the ticket to reflect what it actually does now.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​3ac3d64	Merge branch 'u/dlucas/prepare_linear_code_for_inheritance' of git://trac.sagemath.org/sage into prepare_linear_code_for_inheritance
​3fb0f56	trac #18099: Merged with rc2
​f7a8d03	trac #18099: unimportant change in a doctest
​6587ad8	Merge with public branch
​357c4f6	Fixed too long doctest, put cached_method decorator over minimum_distance() and gens() methods
​8905552	Minor changes to documentation

[dlucas]

Replying to ncohen:

Helloooooooooooooooooooooo,

Hello!

Several comments:

• (superficial) The way you moved code around makes the commit much more difficult to read than it should. It is a good thing to quickly read the commits before you push them, as usually the reviewer has to "reverse-engineer" what you want to do from the diff file. If you are forced to move code around, it is better to do so in a specific commit which *only* moves the code without changing anything in it.

As a reviewer, we have to make sure that all changes are correct, and as moving code around appears to us as 1) remove the code 2) add the code we have to re-read and check all of it, even if it was a no-brainer change on your side.

Got it! Thanks for the advice :)

• I changed your toy example a bit so that the matrix is given as argument. I hope that you will not mind.

Not at all. I kept it in the new version of the code.

• While modifying minimum_distance() and gens() you interfered with its caching mechanism: answers which used to be cached are now forgotten. Could you fix that?

Done. These methods are now cached.

• The doctest you wrote for the abstract class takes a lot of time, and we try to not go over a couple of seconds if we can avoid it (even for long ones). As this one does not test a very fundamental feature, could you make it a bit faster by changing the figures?

Sure. Considering I picked some random methods to illustrate the inheritance mechanism with a dummy class, I just replaced it by something faster.

David

[vdelecroix]

Hello,

I just had a quick look.

1. What is the point of having two classes AbstractLinearCode and LinearCode. I would naturally name LinearCode the one from which I need to inherit from. Why not LinearCode and LinearCode_generic instead?

2. Why the ambient vector space is not part of the input of AbstractLinearCode?

3. What is the difference between base_ring provided by the Module and base_field?

4. Coud you include in the docstring of the former AbstractLinearCode:
   • the set of attributes that are provided by the class
   • what should be done to implement a linar code when inheriting from it

In particular, some methods of AbstractLinearCode assume the presence of the attribute ._generator_matrix (like __cmp__).

5. Could you remove

   + # sage: C.minimum_distance_upper_bound() # optional (net connection)
   + # 3
   + # sage: C.minimum_distance_why() # optional (net connection)
   + # Ub(7,4) = 3 follows by the Griesmer bound.
   

6. In the docstring of AbstractLinearCode replace from this abstract method. with from this abstract class..

Vincent

[ncohen]

Hello,

1. What is the point of having two classes AbstractLinearCode and LinearCode. I would naturally name LinearCode the one from which I need to inherit from. Why not LinearCode and LinearCode_generic instead?

I am not sure that I totally understand your question, but I may be able to help a bit.

The class which is currently named "LinearCode?" should (to me) be named "LinearCodeFromMatrix?" in the code [1]. It is meant to represent a linear code *defined* from a matrix. On the other hand, they will have some code which will *not* be defined from a matrix [2]. Thus, I thought that it made sense to have a class defining methods which apply to all kinds of codes in some astract class (I don't care much what the name of that class is).

Nathann

P.S.: As usual, when we discuss things "live", important information is left out from the ticket's comments.

[1] At user level, I don't see anything wrong with having it aliased as "LinearCode?", as it is the easiest way for users to define a code.

[2] Like Reed-Solomon codes. They apparently have better ways to compute things than to work on a matrix.