Opened 3 years ago
Closed 3 years ago
#28350 closed enhancement (fixed)
Abstract Linear Code No Metric Class
Reported by:  ghemes4  Owned by:  ghemes4 

Priority:  major  Milestone:  sage9.1 
Component:  coding theory  Keywords:  gsoc19 
Cc:  Dima Pasechnik, Johan Rosenkilde, Xavier Caruso, Durand Amaury  Merged in:  
Authors:  Marketa Slukova, Amaury Durand  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  158d73f (Commits, GitHub, GitLab)  Commit:  158d73fbc655eb05ffe09e8dc735833dbb91221b 
Dependencies:  Stopgaps: 
Description
With the changes in the coding component, the classes AbstractLinearCode
and AbstractLinearRankMetricCode
share a lot of the same code. To avoid this and also make creating new linear codes with different metrics easier, we propose to create a new class, AbstractLinearCodeNoMetric
, which will stand between AbstractCode
and AbstractLinearCode
/AbstractLinearRankMetricCode
. This class will contain all the methods relevant for linear codes, which do not depend on a metric. In practice this is mostly methods related to generator matrix.
Change History (29)
comment:1 Changed 3 years ago by
Branch:  → u/emes4/28073 

comment:2 Changed 3 years ago by
Branch:  u/emes4/28073 → u/emes4/28350 

comment:3 Changed 3 years ago by
Branch:  u/emes4/28350 

comment:4 Changed 3 years ago by
Branch:  → u/ghemes4/coding/no_metric 

Commit:  → 45cf76ee5da312cf4fc2b737328daeacdd9156cf 
comment:5 Changed 3 years ago by
I created the class AbstractLinearCodeNoMetric
and moved stuff from AbstractLinearCode
there, including the category theory set up. I don't understand what the facade
does, but when I remove it, the test suite test in LinearCode
is unhappy.
The only thing I couldn't get to work was the requirement that the dimension of the code is at most the length. The problem is that the dimension requires a generator matrix (unless it's given) and that causes some issues. I kept the check line commented out in __init__
of AbstractLinearCodeNoMetric
.
Unlike AbstractCode
, in AbstractLinearCodeNoMetric
, it is required that subclasses have default encoders/decoders. This should make sense since we require a generic constructor class.
comment:6 Changed 3 years ago by
Status:  new → needs_review 

comment:7 Changed 3 years ago by
Owner:  set to ghemes4 

comment:8 Changed 3 years ago by
Commit:  45cf76ee5da312cf4fc2b737328daeacdd9156cf → 01d9a3d0d6af1995325233e913de98f48baf488c 

Branch pushed to git repo; I updated commit sha1. New commits:
01d9a3d  Merge branch 'develop' of git://trac.sagemath.org/sage into t/28350/abstract_linear_code_no_metric_class

comment:9 Changed 3 years ago by
Commit:  01d9a3d0d6af1995325233e913de98f48baf488c → 226ffbf1baf068c0b01df70543554b2be7cb9d5e 

Branch pushed to git repo; I updated commit sha1. New commits:
226ffbf  Added no metric to coding documentation index. Moved zero method from AbstractLinearCode. Changed base_field check.

comment:10 Changed 3 years ago by
Updated Sage.
Added linear_code_no_metric.py
to the index of coding module documentation.
Moved zero
method from AbstractLinearCode
.
Changed base_field
check message.
comment:11 Changed 3 years ago by
Status:  needs_review → needs_work 

with python3:
(i.e. doing ./configure withpython=3
&& make)
sage t src/sage/coding/decoder.py ********************************************************************** File "src/sage/coding/decoder.py", line 197, in sage.coding.decoder.Decoder.__hash__ Failed example: hash(D) #random Exception raised: Traceback (most recent call last): File "/mnt/opt/Sage/sagedev/local/lib/python3.7/sitepackages/sage/doctest/forker.py", line 681, in _run self.compile_and_execute(example, compiler, test.globs) File "/mnt/opt/Sage/sagedev/local/lib/python3.7/sitepackages/sage/doctest/forker.py", line 1123, in compile_and_execute exec(compiled, globs) File "<doctest sage.coding.decoder.Decoder.__hash__[3]>", line 1, in <module> hash(D) #random File "/mnt/opt/Sage/sagedev/local/lib/python3.7/sitepackages/sage/coding/linear_code.py", line 2632, in __hash__ return hash((self.code(), self.maximum_error_weight())) TypeError: unhashable type: 'LinearCode_with_category' **********************************************************************
this causes more errors
sage t src/sage/coding/decoder.py # 1 doctest failed sage t src/sage/coding/linear_code_no_metric.py # 2 doctests failed sage t src/sage/coding/linear_code.py # 1 doctest failed
comment:12 Changed 3 years ago by
Cc:  Xavier Caruso added 

comment:13 Changed 3 years ago by
Cc:  Durand Amaury added 

comment:14 Changed 3 years ago by
Any chance this could be brought forward  it holds up few more tickets...
comment:17 Changed 3 years ago by
Milestone:  sage8.9 → sage9.1 

Ticket retargeted after milestone closed
comment:18 Changed 3 years ago by
Branch:  u/ghemes4/coding/no_metric → u/ghAdurand8/coding/no_metric 

comment:19 followup: 21 Changed 3 years ago by
Commit:  226ffbf1baf068c0b01df70543554b2be7cb9d5e → dbaaaa1a685e1dbaca57997ce3f0f56882382745 

Hello,
I'm not really satisfied with the names of the classes. Instead, I would propose the following hierarchy:
AbstractCode AbstractLinearCode AbstractLinearMetricCode AbstractLinearHammingMetricCode # if relevant AbstractLinearRankMetricCode
Also, through it is not directly related to this ticket, I don't understand the purpose of the class LinearCode
...
New commits:
d49390e  merged version 9.1.beta1

80ff011  Add function hash

dbaaaa1  Merge branch 'develop' into t/28350/coding/no_metric

comment:20 Changed 3 years ago by
Commit:  dbaaaa1a685e1dbaca57997ce3f0f56882382745 → db3b18af01e1527f6d393405945d6d722d9c41b1 

Branch pushed to git repo; I updated commit sha1. New commits:
db3b18a  Error commit on linear_code_no_metric in precedent commit

comment:21 Changed 3 years ago by
comment:22 Changed 3 years ago by
Authors:  Marketa Slukova → Marketa Slukova, Durand Amaury 

tests pass now. Is it ready for review?
comment:23 Changed 3 years ago by
Authors:  Marketa Slukova, Durand Amaury → Marketa Slukova, Amaury Durand 

comment:25 Changed 3 years ago by
Dima, is there a specific reason you refrain from reviewing yourself? I'm completely swamped right now, so there's no chance I'll do it any time soon, but I remember being fairly happy about the design during GSoC.
@caruso: Concerning your proposed naming, I guess your names are better from a completely general point of view. The current naming is for "historic reasons" centering on classical "linear codes over Hamming metric" being what everyone would expect. So in some sense the names follow the principal of least surprise by specifically pointing out in the name when it differs from the "expected". But I think I could easily be talked into going with your hierarchy.
The purpose of LinearCode
btw is to act as a thin wrapper around AbstractLinearCode
for constructing "unstructured" codes directly from a generator matrix. That's an essential class for users. The distinction from AbstractLinearCode
was created as part of the encoderframework (long story), but is a typical way to construct a class hierarchy in more strongly typed languages: if I could do so in Python, I would specify on a type level that AbstractLinearCode? is not supposed to be instantiated directly.
comment:26 Changed 3 years ago by
Reviewers:  → Dima Pasechnik 

Status:  needs_work → positive_review 
I was not reviewing as the ticket is set to needs work
. So I've set it to needs review and to positive review now.
comment:28 Changed 3 years ago by
Branch:  u/ghAdurand8/coding/no_metric → u/dimpase/coding/no_metric 

Commit:  db3b18af01e1527f6d393405945d6d722d9c41b1 → 158d73fbc655eb05ffe09e8dc735833dbb91221b 
Dependencies:  #28073 
Status:  needs_work → positive_review 
rebased
New commits:
244f4af  Class linear_code_no_metric. Moved stuff from linear_code.

c716526  Added no metric to coding documentation index. Moved zero method from AbstractLinearCode. Changed base_field check.

0b5baa0  Add function hash

158d73f  Error commit on linear_code_no_metric in precedent commit

comment:29 Changed 3 years ago by
Branch:  u/dimpase/coding/no_metric → 158d73fbc655eb05ffe09e8dc735833dbb91221b 

Resolution:  → fixed 
Status:  positive_review → closed 
Last 10 new commits:
Finished up documentation.
Merge branch 'develop' of git://trac.sagemath.org/sage into t/28073/abstract_code
Merge branch 'develop' of git://trac.sagemath.org/sage into t/28073/abstract_code
Documentation and example fixes.
Merge #27634
Module inheritance. Ambient_space and __call__ changes.
Merge commit '8b01cc5df9e1508250976b08b4d2212aecb02927' of git://trac.sagemath.org/sage into t/28073/abstract_code
Merge branch 'develop' of git://trac.sagemath.org/sage into t/28073/abstract_code
documentation fix
Class linear_code_no_metric. Moved stuff from linear_code.