#28073 closed enhancement (fixed)

Abstract Code Class

Reported by: gh-emes4 Owned by: gh-emes4
Priority: major Milestone: sage-9.0
Component: coding theory Keywords: gsoc19
Cc: dimpase, jsrn, caruso, gh-Adurand8 Merged in:
Authors: Marketa Slukova Reviewers: Dima Pasechnik, Durand Amaury
Report Upstream: N/A Work issues:
Branch: 4dbc878 (Commits, GitHub, GitLab) Commit: 4dbc8789f4ef1e173987df7ba6e13769eacff2ac
Dependencies: #27634 Stopgaps:

Status badges

Description (last modified by gh-emes4)

AbstractLinearCode is at the moment the most abstract representation of codes in Sage. This makes it very difficult to implement non-linear codes and also codes with a metric different than Hamming.

We propose to create AbstractCode class that will contain metric-agnostic methods, as well as the encoder/decoder framework. AbstractLinearCode will derive from this class.

Change History (59)

comment:1 Changed 21 months ago by gh-emes4

  • Authors set to Marketa Slukova
  • Cc dimpase jsrn added
  • Component changed from PLEASE CHANGE to coding theory
  • Description modified (diff)
  • Owner changed from (none) to gh-emes4
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 21 months ago by gh-emes4

  • Summary changed from Abstract Code to Abstract Code Class

comment:3 Changed 21 months ago by gh-emes4

  • Branch set to u/emes4/abstract_code

comment:4 Changed 21 months ago by gh-emes4

  • Branch changed from u/emes4/abstract_code to u/gh-emes4/coding/abstract_code
  • Commit set to ba4fc5394446919f9969b5df6bb5b4da82bf9876

New commits:

f4767e0AbstractCode class created, cut relevant methods from linear_code.py
2cc4c72Reverted base_field and length to be only in linear_code.py
64f446aMerge branch 'develop' of git://trac.sagemath.org/sage into abstract_code
53e445badded base_ring and length parameter to AbstractCode
5e6dffeFixed some dependencies. Category still set up wrong.
ba4fc53Merge branch 'abstract_code' into t/28073/abstract_code

comment:5 follow-up: Changed 21 months ago by dimpase

you've checked in uncleanly merged/rebased things, e.g.

+>>>>>>> 4f44acd853... Fixed some dependencies. Category still set up wrong.

please look at >>>>, <<<<, and ==== markers in these files, clean them up.

comment:6 Changed 21 months ago by git

  • Commit changed from ba4fc5394446919f9969b5df6bb5b4da82bf9876 to e8edfebc20568c2173cc4d2e9306edcff42fa85b

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

e8edfebFixed unclean merge.

comment:7 in reply to: ↑ 5 Changed 21 months ago by gh-emes4

Replying to dimpase:

Sorry about that, it showed me the cleaned up file in my editor.

comment:8 Changed 21 months ago by gh-emes4

I created the class AbstractCode and moved all the relevant methods from AbstractLinearCode.

Originally, I had default_encoder and default_decoder parameters in the initialisation of AbstractCode, however, I ran into an issue with dependencies - if I set a default_encoder for AbstractLinearCode, this would then be the default for all the codes inheriting from this. I am sure that this can be fixed, however maybe it makes sense for classes such as AbstractLinearCode to not have default decoders/encoders?

Finally, one of the doc tests fails. I tried to set up the category stuff (Parent.__init__()), but I don't think I did it correctly.

This is just a rough, first draft.

comment:9 Changed 21 months ago by git

  • Commit changed from e8edfebc20568c2173cc4d2e9306edcff42fa85b to 880aebb7258263aa73ac90763551aeccbce84c94

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

880aebbFixed default decoder/encoder dependencies. Set to None by default.

comment:10 Changed 21 months ago by dimpase

  • Keywords gsoc19 added

comment:11 Changed 20 months ago by jsrn

Thanks for working on this! I took a quick view over the new class, and it looks good. There are some copy/paste errors in the documentation where it refers to AbstractLinearCode or "this linear code" etc., but that's small stuff.

More importantly, I am worried about how AbstractCode should fit into the category framework. In particular, a non-linear code is not necessarily a module. Also, some objects people call codes are not even inside some free module like R^n for some ring R, e.g. polyalphabetic codes like Chinese Remainder Codes or Polynomial Remainder codes, which are inside A_1 x A_2 x ... x A_n for some rings A_i. Really, the most general notion of a code is just a subset of some set, which isn't saying much.

Perhaps, as we're now talking of a really base class for all codes, AbstractCode should therefore simply omit setting up anything in the category framework, leaving that to more concrete implementations. The task of AbstractCode would then basically only consist of setting up the encoder/decoder framework, as well as providing the metric method, which I like (I guess list() is fine too). It shouldn't even have a length I guess?

In this case, I don't know whether AbstractCode should somehow otherwise be made a parent in the category framework, or whether we should agree that this is a "contract" that sub-classes have to do themselves.

Best, Johan

comment:12 Changed 20 months ago by git

  • Commit changed from 880aebb7258263aa73ac90763551aeccbce84c94 to d11560039839bb16a6d85d9e4ed674afdabfc299

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

d115600No category set up and base_field in AbstractCode. No encoder/decoder error msgs. Documentation and tests.

comment:13 Changed 20 months ago by gh-emes4

I made all the changes we discussed, namely:

The category framework is no longer set up in AbstractCode. Instead of inheriting from Module, it now inherits from Parent. I tried a few things, e.g. SageObject, Set here and Parent seemed to do the trick. I added some documentation hopefully instructing the user on how to set the category framework up.

I took base_field away from the initiating parameters of AbstractCode. This should be the only change in AbstractLinearCode.

The decoder and encoder methods now instruct the user to add a decoder/encoder for the code if they try to use the encoder/decoder framework without having set these up. I added tests for these. If there are no default encoders/decoders, the methods decoders_available and encoders_available return an empty list. Let me know if all the checks for None are correctly set up.

I extended the documentation and following the example of AbstractLinearCode, added an example of how to use AbstractCode to create a subclass.

A lot of the documentation overlaps with AbstractLinearCode, however I don't think this is an issue.

comment:14 Changed 20 months ago by dimpase

please push your updates...

comment:15 Changed 20 months ago by git

  • Commit changed from d11560039839bb16a6d85d9e4ed674afdabfc299 to 487e9e2ce347f619114d52bb33e98ab042947dee

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

2bf73f8Merge branch 'develop' of git://trac.sagemath.org/sage into t/28073/abstract_code
487e9e2Category related methods added. Encoder/decoder documentation specified for linear codes.

comment:16 Changed 20 months ago by gh-emes4

Added methods __iter__ and __contains__ to AbstractCode instructing the user to override them.

Changed Encoder and Decoder class documentation to instruct the user to inherit from these when working with linear codes (over any metric).

comment:17 Changed 20 months ago by git

  • Commit changed from 487e9e2ce347f619114d52bb33e98ab042947dee to 40df01e3e1bc92c4d22ae9928f39c24d64117841

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

40df01eFinished up documentation.

comment:18 Changed 20 months ago by gh-emes4

  • Status changed from new to needs_review

comment:19 Changed 20 months ago by gh-emes4

  • Dependencies set to #28209

comment:20 Changed 20 months ago by git

  • Commit changed from 40df01e3e1bc92c4d22ae9928f39c24d64117841 to 01135cbfae1ff481ba243ca05eabf229e8c1432d

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

01135cbMerge branch 'develop' of git://trac.sagemath.org/sage into t/28073/abstract_code

comment:21 Changed 20 months ago by gh-emes4

  • Dependencies #28209 deleted

comment:22 Changed 20 months ago by jsrn

  • Status changed from needs_review to needs_work

This looks very promising! My comments may look long but they are not too heavy. Note: this is not a complete review, I didn't compile, run the code and build the documentation. I'm hoping Dima will do that :-)

  • Top of the class:

The only assumption we kept is that the code is enumerable.

change to

The abstract notion of "code" that is implicitly used for this class is any enumerable subset of a cartesian product `A_1 \times A_2 \times \ldots \times A_n for some sets A_i`. Note though that this class makes no attempt to directly represent the code in this fashion, allowing subclasses to make the appropriate choices. The notion of metric is also not mathematically enforced in any way, and is simply stored as a string value. ".

The line just after has a line break very early, doesn't it?

  • part of the framework category -> part of the category framework
  • any method that works on linear codes works for our -> coming from AbstractCode code
  • The example in AbstractCode.__init__ is not great, since it doesn't yield a working code class (it doesn't implement _list_ etc.). Can you make an small, but more meaningful example, e.g. the code consisting of the l words
{
  00...00,
  10...00,
  11...00,
  ...
  11...10,
  11...11
}

For this code, you could add a full working example with implementations of _list_, __iter__, __contains__.

The For #28209, the example could be expounded upon in a short thematic tutorial: adding an encoder/unencoder from the ring (ZZ mod (l+1)) into the code. The metric could be Hamming (though the code is of course quite bad), and that would allow attaching the LinearCodeNearestNeighbor? decoder, which actually works for any code under the Hamming metric.

That would be very nice documentation for anyone wanting to implement a new class of codes not fitting in linear/hamming codes.

  • The example for _repr_ and _latex_ is unnecessarily complicated because AbstractCode does not need supplying encoders and decoders. Use instead the smaller example from e.g. __contains__.
  • In doc for decode_to_code and decode_to_message, the description of word should say "an element in the ambient space as self". The code might not be vectorial in the classical sense.
  • For decoder() and encoder(), the exception thrown should be a NotImplementedError and the message could perhaps be "No encoder [resp. decoder] implemented for this code".
  • In doc for encode, then the description of word should say "an element of a message space of the code". The message space might not be vectorial. The note after the INPUT block is not appropriate anymore and should just be removed.
  • At this abstraction there's virtually no service we could put on an Encoder or Decoder class which would justify having an AbstractEncoder? or AbstractDecoder?, I think. In a strongly typed OOP language, like Java, we would of course have to have such a thing, but I believe in Python and SageMath in particular, the convention is not to have needless abstract classes. Therefore your oneline doc changes to encoder.py and decoder.py are spot-on: that's all the change we'll have for those files.

However, we are left with something of a documentation problem. For where do we document the precise interface requirements of an encoder and a decoder assumed by the framework in AbstractCode?? I think the best place for this is a (not too long) discussion at the top of the file abstract_code.py, i.e. just after "Any class inheriting ... can use the encode/decode framework". Here we can describe what that is, and what is meant and promised by an encoder/decoder.

I am OK with having a relatively short description of the purpose, and just mentioning the methods an encoder must have (encode, __call__ which is simply encode, unencode, message_space and code), and similarly for a decoder. And then pointing to Encoder and Decoder as examples.

Best, Johan

comment:23 Changed 19 months ago by dimpase

I'd like to wait for above to be implemented (at least in part) before doing my own review.

comment:24 Changed 19 months ago by git

  • Commit changed from 01135cbfae1ff481ba243ca05eabf229e8c1432d to 9608a237aeabcaf56416bc26475a6b3c0433e6c0

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

ef7b797Merge branch 'develop' of git://trac.sagemath.org/sage into t/28073/abstract_code
9608a23Documentation and example fixes.

comment:25 follow-up: Changed 19 months ago by gh-emes4

I implemented all the changes that Johan suggested.

One comment: In the new example in AbstractCode.__init__, I didn't make it a part of the category framework, partially because I was unsure as to which category to choose.

comment:26 in reply to: ↑ 25 ; follow-up: Changed 19 months ago by jsrn

Replying to gh-emes4:

I implemented all the changes that Johan suggested.

You are fast! :-)

One comment: In the new example in AbstractCode.__init__, I didn't make it a part of the category framework, partially because I was unsure as to which category to choose.

Putting it in the category framework would be great, but I think it is fine as it is. I'm happy with the example :-)

One new comment I thought of: AbstractLinearCode used to inherit from Module as well as being injected into the category framework under Modules. Now it inherits from AbstractCode which, of course, does not inherit from Module. My concern is whether some functionality from Module might now have gotten lost?

I haven't checked out your ticket and compiled, but this is the list of methods I get with dir(C) on a freshly created LinearCode C on Sage 8.8.rc3:

['CartesianProduct', 'Element', 'Hom', '__cached_methods', '__call__', '__class__', '__contains__', '__delattr__', '__dict__', '__dir__', '__div__', '__doc__', '__eq__', '__format__', '__gens_dict', '__getattribute__', '__getitem__', '__getstate__', '__hash__', '__init__', '__init_extra__', '__iter__', '__len__', '__make_element_class__', '__module__', '__mul__', '__ne__', '__new__', '__nonzero__', '__pari__', '__pyx_vtable__', '__rdiv__', '__reduce__', '__reduce_ex__', '__repr__', '__rmul__', '__setattr__', '__setstate__', '__sizeof__', '__slots__', '__str__', '__subclasshook__', '__temporarily_change_names', '_abstract_element_class', '_an_element_', '_apply_module_endomorphism', '_apply_module_morphism', '_ascii_art_', '_assign_names', '_axiom_', '_axiom_init_', '_base', '_cache_an_element', '_cache_key', '_canonize', '_coerce_map_from_', '_coerce_map_via', '_coercions_used', '_convert_map_from_', '_convert_method_map', '_convert_method_name', '_default_decoder_name', '_default_encoder_name', '_defining_names', '_dense_free_module', '_dimension', '_doccls', '_element_constructor_', '_element_constructor_from_element_class', '_facade_for', '_factory_data', '_first_ngens', '_fricas_', '_fricas_init_', '_from_dict', '_gap_', '_gap_init_', '_generator_matrix', '_generic_coerce_map', '_generic_convert_map', '_get_action_', '_giac_', '_giac_init_', '_gp_', '_gp_init_', '_init_category_', '_initial_action_list', '_initial_coerce_list', '_initial_convert_list', '_interface_', '_interface_init_', '_interface_is_cached_', '_internal_coerce_map_from', '_internal_convert_map_from', '_introspect_coerce', '_is_category_initialized', '_is_coercion_cached', '_is_conversion_cached', '_is_valid_homomorphism_', '_kash_', '_kash_init_', '_latex_', '_length', '_macaulay2_', '_macaulay2_init_', '_magma_init_', '_maple_', '_maple_init_', '_mathematica_', '_mathematica_init_', '_maxima_', '_maxima_init_', '_maxima_lib_', '_maxima_lib_init_', '_minimum_distance', '_minimum_weight_codeword', '_module_morphism', '_names', '_octave_', '_octave_init_', '_pari_init_', '_polymake_', '_polymake_init_', '_populate_coercion_lists_', '_punctured_form', '_r_init_', '_reduction', '_refine_category_', '_registered_decoders', '_registered_encoders', '_remove_from_coerce_cache', '_repr_', '_repr_option', '_sage_', '_set_element_constructor', '_singular_', '_singular_init_', '_sum_of_monomials', '_test_additive_associativity', '_test_an_element', '_test_cardinality', '_test_category', '_test_elements', '_test_elements_eq_reflexive', '_test_elements_eq_symmetric', '_test_elements_eq_transitive', '_test_elements_neq', '_test_eq', '_test_new', '_test_not_implemented_methods', '_test_pickling', '_test_some_elements', '_test_zero', '_tester', '_underlying_class', '_unicode_art_', '_unset_category', '_unset_coercions_used', '_unset_embedding', 'add_decoder', 'add_encoder', 'addition_table', 'algebra', 'ambient_space', 'an_element', 'annihilator', 'annihilator_basis', 'assmus_mattson_designs', 'automorphism_group_gens', 'base', 'base_extend', 'base_field', 'base_ring', 'basis', 'binomial_moment', 'canonical_representative', 'cardinality', 'cartesian_product', 'categories', 'category', 'change_ring', 'characteristic', 'characteristic_polynomial', 'chinen_polynomial', 'coerce', 'coerce_embedding', 'coerce_map_from', 'construction', 'convert_map_from', 'covering_radius', 'decode_to_code', 'decode_to_message', 'decoder', 'decoders_available', 'dimension', 'direct_sum', 'divisor', 'dual_code', 'dump', 'dumps', 'echelon_form', 'element_class', 'encode', 'encoder', 'encoders_available', 'endomorphism_ring', 'extended_code', 'facade_for', 'from_vector', 'galois_closure', 'generator_matrix', 'gens', 'gens_dict', 'gens_dict_recursive', 'genus', 'get_action', 'has_coerce_map_from', 'hom', 'information_set', 'inject_variables', 'is_empty', 'is_exact', 'is_finite', 'is_galois_closed', 'is_information_set', 'is_parent_of', 'is_permutation_automorphism', 'is_permutation_equivalent', 'is_projective', 'is_self_dual', 'is_self_orthogonal', 'is_subcode', 'latex_name', 'latex_variable_names', 'length', 'linear_combination', 'list', 'minimum_distance', 'module_composition_factors', 'module_morphism', 'monomial', 'monomial_or_zero_if_none', 'objgen', 'objgens', 'parent', 'parity_check_matrix', 'permutation_automorphism_group', 'permuted_code', 'punctured', 'quotient_module', 'random_element', 'rate', 'redundancy_matrix', 'register_action', 'register_coercion', 'register_conversion', 'register_embedding', 'relative_distance', 'rename', 'reset_name', 'save', 'shortened', 'some_elements', 'spectrum', 'standard_form', 'submodule', 'sum', 'sum_of_monomials', 'sum_of_terms', 'summation', 'summation_from_element_class_add', 'support', 'syndrome', 'systematic_generator_matrix', 'tensor', 'tensor_square', 'term', 'unencode', 'variable_name', 'variable_names', 'weight_distribution', 'weight_enumerator', 'zero', 'zeta_function', 'zeta_polynomial']

comment:27 Changed 19 months ago by jsrn

Oh yeah, Python has multiple inheritance, so perhaps AbstractLinearCode should just be declared as

class AbstractLinearCode(AbstractCode, Module)

and then everything should be fine?

comment:28 in reply to: ↑ 26 ; follow-up: Changed 19 months ago by gh-emes4

Replying to jsrn:

I haven't checked out your ticket and compiled, but this is the list of methods I get with dir(C) on a freshly created LinearCode C on Sage 8.8.rc3:

Without inheriting from Module, a fresh LinearCode on this branch (Sage 8.9.beta4) has more methods than your list, but is missing these: ['base_extend', 'change_ring', 'endomorphism_ring'].

I added the Module inheritance, will push it with some bigger changes.

Last edited 19 months ago by gh-emes4 (previous) (diff)

comment:29 in reply to: ↑ 28 ; follow-ups: Changed 19 months ago by jsrn

Replying to gh-emes4:

Replying to jsrn:

I haven't checked out your ticket and compiled, but this is the list of methods I get with dir(C) on a freshly created LinearCode C on Sage 8.8.rc3:

Without inheriting from Module, a fresh LinearCode on this branch (Sage 8.9.beta4) has more methods than your list, but is missing these: ['base_extend', 'change_ring', 'endomorphism_ring'].

None of those methods seem to do anything useful, and they don't even make much sense for a (linear) code to begin with. endomorphism_ring is particularly unfortunate, since we have automorphism_group_gens which does something very useful, and a user might confuse these.

Perhaps, then, it's better to not inherit from Module after all?

comment:30 Changed 19 months ago by gh-emes4

  • Status changed from needs_work to needs_review

comment:31 in reply to: ↑ 29 ; follow-up: Changed 19 months ago by dimpase

Replying to jsrn:

None of those methods seem to do anything useful, and they don't even make much sense for a (linear) code to begin with. endomorphism_ring is particularly unfortunate, since we have automorphism_group_gens which does something very useful, and a user might confuse these.

Perhaps, then, it's better to not inherit from Module after all?

IMHO, Module is too general, here we have free modules, a.k.a. ModulesWithBasis.

comment:32 in reply to: ↑ 31 ; follow-up: Changed 19 months ago by jsrn

Replying to dimpase:

Perhaps, then, it's better to not inherit from Module after all?

IMHO, Module is too general, here we have free modules, a.k.a. ModulesWithBasis.

That's absolutely true. If we inherit from that instead, do these useless methods then go away?

comment:33 in reply to: ↑ 32 Changed 19 months ago by gh-emes4

Replying to jsrn:

Replying to dimpase:

Perhaps, then, it's better to not inherit from Module after all?

IMHO, Module is too general, here we have free modules, a.k.a. ModulesWithBasis.

That's absolutely true. If we inherit from that instead, do these useless methods then go away?

When I add ModulesWithBasis to the inheritance of AbstractLinearCode, I get the following error: ValueError: base must be a ring or a subcategory of Rings(). I am not sure how to fix that?

comment:34 Changed 19 months ago by jsrn

I see, this is due to the structure of the category framework: ModulesWithBasis is a category (i.e. the set of all modules with a basis). We are standing with a single "module with basis", so we should put that into the category. But there is no class ModuleWithBasis (singular). So the way this is done is to inherit from Module and then use a magic incantation to tell the category framework that this is not just any module, it is a module with basis. That's exactly what is currently done in AbstractLinearCode.

Looking at the definition of sage.modules.modules.Module, I don't see anything useful for LinearCodes to inherit. On the other hand, it's probably not a good idea to put a parent object into the category of modules (with basis) without that parent object actually being a module. At least, any strong type system would cry ;-)

So I suggest just keeping the status quo by using multiple inheritance so AbstractLinearCode inherits from both AbstractCode and Module (in that order).

comment:35 Changed 19 months ago by jsrn

I've just been reviewing the cleanup-ticket #27634. This ticket might very well conflict with that, so I suggest merging in #27634 and making it a dependency, so we handle that up-front.

comment:36 Changed 19 months ago by gh-emes4

  • Dependencies set to #27634

comment:37 follow-up: Changed 19 months ago by jsrn

I propose adding a method ambient_space to AbstractCode which throws a NotImplementedError. It should be documented that it is recommended (but not required) to override this method. Then we can also move the __call__ method of AbstractLinearCode to AbstractCode.

comment:38 Changed 19 months ago by jsrn

  • Status changed from needs_review to needs_work

comment:39 in reply to: ↑ 37 ; follow-up: Changed 19 months ago by gh-emes4

Replying to jsrn:

I propose adding a method ambient_space to AbstractCode which throws a NotImplementedError. It should be documented that it is recommended (but not required) to override this method. Then we can also move the __call__ method of AbstractLinearCode to AbstractCode.

Is this method required by the encoder/decoder framework?

comment:40 in reply to: ↑ 39 ; follow-up: Changed 19 months ago by jsrn

Replying to gh-emes4:

Replying to jsrn:

I propose adding a method ambient_space to AbstractCode which throws a NotImplementedError. It should be documented that it is recommended (but not required) to override this method. Then we can also move the __call__ method of AbstractLinearCode to AbstractCode.

Is this method required by the encoder/decoder framework?

Well, __call__ is a natural method for any code that is encodable, so it naturally fits to AbstractCode. Generally in Sage for a parent P then P(e) for some value e does one of two things (which abstractly is maybe the same thing):

  1. It "coerces" e into being something that can be considered to be in P. So you convert e into the shape objects in P have. E.g. if F is a field then F(1) gives you the 1-element of that field.
  1. It uses e as an input to create an object in P. E.g. if P is a polynomial ring then P[1,2,3] creates the polynomial 1 + 2*x + 3*x^2.

If P is some linear code C, then Item 1 is supported in the sense that if e is a vector in the ambient space of C, then C(e) returns e if e is in C, otherwise it throws an error. Not the most useful function perhaps, but it follows this coercioen convention of SageMath.

Item 2, however, can naturally be considered to be encoding - after all, that's the canonical way you would "construct" a codeword. Due to the convention that the default encoder should always use F[x]^k as the message space, then if e is a vector of length k then C(e) returns the encoding of e as a codeword.

A funny (disturbing?) detail I never considered before is that if C is an [n,n] code, then the current implementation always uses Item 1, i.e. it doesn't encode.

Anyway, this convention absolutely makes sense for any type of code I could think of. So, it should be on AbstractCode. For that to work, ambient_space() has to be on AbstractCode as well. But since we don't really want to force a representation of the ambient space (since it would be some clumsy cartesian product in the general case), then we should just leave it to subclasses to fill out.

In any case, it is a natural function to expect a code to have, i.e. if I was writing in a strongly typed language like Java or C#, then AbstractCode would be an interface, and I would require ambient_space to be a method that had to be implemented in subclasses.

comment:41 Changed 19 months ago by git

  • Commit changed from 9608a237aeabcaf56416bc26475a6b3c0433e6c0 to 318b4441b0e2f7ee1b1275f733e5f43b64b3669e

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

c8706eeMerge branch 'develop'
fb35c2fFixes responding to reviewer comments
0b95c1dManual table for codes_catalog.py
1bca13cMerge branch 'u/jsrn/27634'
6bf9f1eRemove unwanted names under codes
24df329A minor fix in the main document
6c7012bA pyflakes fix in linear_code.py
c650b8aRename channel_constructions.py and goppa.py
5230b19Merge #27634
318b444Module inheritance. Ambient_space and __call__ changes.

comment:42 in reply to: ↑ 29 ; follow-up: Changed 19 months ago by dimpase

Replying to jsrn:

Replying to gh-emes4:

Replying to jsrn:

I haven't checked out your ticket and compiled, but this is the list of methods I get with dir(C) on a freshly created LinearCode C on Sage 8.8.rc3:

Without inheriting from Module, a fresh LinearCode on this branch (Sage 8.9.beta4) has more methods than your list, but is missing these: ['base_extend', 'change_ring', 'endomorphism_ring'].

None of those methods seem to do anything useful, and they don't even make much sense for a (linear) code to begin with. endomorphism_ring is particularly unfortunate,

endomorphism ring provides maps onto subcodes, which does not look as totally useless to me.

changing the ring (taking a bigger ring) produces an additive code.

comment:43 in reply to: ↑ 40 ; follow-up: Changed 19 months ago by gh-emes4

Replying to jsrn:

In any case, it is a natural function to expect a code to have, i.e. if I was writing in a strongly typed language like Java or C#, then AbstractCode would be an interface, and I would require ambient_space to be a method that had to be implemented in subclasses.

Thank you for the explanation, makes things much clearer!

I did all the changes, namely added Module to inheritance of AbstractLinearCode, merged #27634, added ambient_space method to AbstractCode with a recommendation to implement it, and moved __call__ from AbstractLinearCode to AbstractCode.

comment:44 Changed 19 months ago by gh-emes4

  • Status changed from needs_work to needs_review

comment:45 in reply to: ↑ 42 Changed 19 months ago by jsrn

Replying to dimpase:

endomorphism ring provides maps onto subcodes, which does not look as totally useless to me.

Quite true. I have never seen people study it, but probably some people have. And it does have the same type of meaning as automorphism_group_gens. It's just inconsistent and unfortunate that that method is not called automorphism_ring and wraps the returned generators in some appropriate algebraic object.

changing the ring (taking a bigger ring) produces an additive code.

OK, I'm not familiar with those. But of course you can take a linear code over some field F_q and then consider its generator matrix as part of F_{q^m} and look at the code there. That would even be trivial to implement (but does not belong in this ticket).

comment:46 in reply to: ↑ 43 Changed 19 months ago by jsrn

Replying to gh-emes4:

I did all the changes, namely added Module to inheritance of AbstractLinearCode, merged #27634, added ambient_space method to AbstractCode with a recommendation to implement it, and moved __call__ from AbstractLinearCode to AbstractCode.

Awesome! I'm happy now :-) Dima, your turn ;-)

comment:47 follow-up: Changed 19 months ago by dimpase

  • Status changed from needs_review to needs_work

I am getting

sage -t --warn-long 47.5 src/doc/en/thematic_tutorials/structures_in_coding_theory.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/structures_in_coding_theory.rst", line 450, in doc.en.thematic_tutorials.structures_in_coding_theory
Failed example:
    from sage.coding.channel_constructions import Channel
Exception raised:
    Traceback (most recent call last):
      File "/home/dimpase/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/dimpase/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1105, in compile_and_execute
        exec(compiled, globs)
      File "<doctest doc.en.thematic_tutorials.structures_in_coding_theory[0]>", line 1, in <module>
        from sage.coding.channel_constructions import Channel
    ImportError: No module named channel_constructions
**********************************************************************
File "src/doc/en/thematic_tutorials/structures_in_coding_theory.rst", line 451, in doc.en.thematic_tutorials.structures_in_coding_theory
Failed example:
    class BinaryStaticErrorRateChannel(Channel):
        def __init__(self, space, number_errors):
            if space.base_ring() is not GF(2):
                raise ValueError("Provided space must be a vector space over GF(2)")
            if number_errors > space.dimension():
                raise ValueErrors("number_errors cannot be bigger than input space's dimension")
            super(BinaryStaticErrorRateChannel, self).__init__(space, space)
            self._number_errors = number_errors
Exception raised:
    Traceback (most recent call last):
      File "/home/dimpase/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/dimpase/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1105, in compile_and_execute
        exec(compiled, globs)
      File "<doctest doc.en.thematic_tutorials.structures_in_coding_theory[1]>", line 1, in <module>
        class BinaryStaticErrorRateChannel(Channel):
    NameError: name 'Channel' is not defined
**********************************************************************
1 item had failures:
   2 of  45 in doc.en.thematic_tutorials.structures_in_coding_theory
    [28 tests, 2 failures, 0.11 s]
Version 0, edited 19 months ago by dimpase (next)

comment:48 Changed 19 months ago by dimpase

there is also a typo, please apply the following:

--- a/src/sage/coding/abstract_code.py
+++ b/src/sage/coding/abstract_code.py
@@ -353,7 +353,7 @@ class AbstractCode(Parent):
         r"""
         Return an error stating ``ambient_space`` of ``self`` is not implemented.
 
-        This method is required by the :method:`__call__`.
+        This method is required by the :meth:`__call__`.
 
         EXAMPLES::
 

comment:49 Changed 19 months ago by git

  • Commit changed from 318b4441b0e2f7ee1b1275f733e5f43b64b3669e to 4dbc8789f4ef1e173987df7ba6e13769eacff2ac

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

3996761Merge commit '8b01cc5df9e1508250976b08b4d2212aecb02927' of git://trac.sagemath.org/sage into t/28073/abstract_code
a4582a3Merge branch 'develop' of git://trac.sagemath.org/sage into t/28073/abstract_code
4dbc878documentation fix

comment:50 in reply to: ↑ 47 Changed 19 months ago by gh-emes4

Replying to dimpase:

I am getting (on the branch of the ticket)

This was an error coming from #27634, https://trac.sagemath.org/ticket/27634#comment:40. It was fixed on the ticket, I merged the updated branch.

I fixed the small documentation mistake.

I ran the whole test suite make ptestlong and there were no errors.

comment:51 Changed 19 months ago by gh-emes4

  • Status changed from needs_work to needs_review

comment:52 Changed 18 months ago by dimpase

  • Reviewers set to Dima Pasechnik

looks good

comment:53 Changed 18 months ago by caruso

  • Cc caruso added

comment:54 Changed 18 months ago by gh-Adurand8

  • Cc gh-Adurand8 added

comment:55 Changed 18 months ago by gh-Adurand8

  • Reviewers changed from Dima Pasechnik to Dima Pasechnik, gh-Adurand8
  • Status changed from needs_review to positive_review

comment:56 Changed 18 months ago by dimpase

  • Status changed from positive_review to needs_work

reviewer names should be real names.

comment:57 Changed 18 months ago by gh-Adurand8

  • Reviewers changed from Dima Pasechnik, gh-Adurand8 to Dima Pasechnik, Durand Amaury
  • Status changed from needs_work to positive_review

Sorry, I missed this information. It's corrected !

comment:58 Changed 17 months ago by chapoton

  • Milestone changed from sage-8.9 to sage-9.0

moving milestone to 9.0 (after release of 8.9)

comment:59 Changed 17 months ago by vbraun

  • Branch changed from u/gh-emes4/coding/abstract_code to 4dbc8789f4ef1e173987df7ba6e13769eacff2ac
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.