sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 481 forks source link

An Abstract Class for Rank Metric Codes #21226

Closed 5b3bd7e6-c3f3-4830-a555-991f5e6beec0 closed 4 years ago

5b3bd7e6-c3f3-4830-a555-991f5e6beec0 commented 8 years ago

We propose to implement AbstractRankMetricCode, an abstract class for rank metric codes which will initialize the basic parameters that every rank metric code possesses. This will inherit from AbstractCode class. Further, we propose to add rank-metric based methods to compute distance, weight and allow for matrix to vector (and reverse) conversions between representations. Finally, we create a generic representative class LinearRankMetricCode as well as encoding (generator matrix, systematic) and decoding (nearest neighbour) methods.

CC: @sagetrac-dlucas @johanrosenkilde @dimpase @xcaruso @Adurand8 @mbombar @vbraun

Component: coding theory

Author: Arpit Merchant, Marketa Slukova, Johan Rosenkilde

Branch: 899d390

Reviewer: Dima Pasechnik, Johan Rosenkilde, Xavier Caruso

Issue created by migration from https://trac.sagemath.org/ticket/21226

johanrosenkilde commented 4 years ago

Changed branch from u/dimpase/coding/linear_rank_metric to u/jsrn/coding/linear_rank_metric

johanrosenkilde commented 4 years ago

Changed commit from ffb8297 to 5f46031

johanrosenkilde commented 4 years ago
comment:40

I've fixed the issue with __eq__ by implementing and documenting a correct version. Then sage.coding.linear_code.AbstractLinearCode had a redundant (and less efficient) implementation of __eq__ that I removed. I also moved __ne__ from that class to AbstractLinearCodeNoMetric.

Using ./sage -coverage, I found that linear_rank_code had another method decode_to_code with no doctests. I added one. I discovered then that two other doctests of that file were running very slowly (>10 s), so I just changed the tests to a smaller ones which runs much faster.


New commits:

150d834Example for coding.linear_code_no_metric.__init__
5f32a03Merge branch 'u/dimpase/coding/linear_rank_metric' of git://trac.sagemath.org/sage into 21226_abstract_rank_metric
4a770f8Fix `__eq__` and `__ne__` for AbstractLinearCodeNoMetric, and remove it from AbstractLinearCode
9ef1e9cAdd doctest to coding.linear_rank_metric.LinearRankMetricCodeNearestNeighborDecoder.decode_to_code
5f46031Make doctests of sage.coding.linear_rank_metric run 50 times faster
johanrosenkilde commented 4 years ago

Changed author from Arpit Merchant, Marketa Slukova to Arpit Merchant, Marketa Slukova, Johan Rosenkilde

xcaruso commented 4 years ago
comment:41

Thanks.

Since I haven't really looked into the code of this ticket, I let Dima give again a positive review if appropriate.

Btw, I'm wondering: is it preferable to implement __eq__, __ne__, etc. or _richcmp_? I believed that the latter solution was better, but I'm not sure.

xcaruso commented 4 years ago
comment:42

Note also that the patchbot reported some warning (pyflakes, pycodestyle and blocks).

dimpase commented 4 years ago
comment:43

Thanks. First time I missed the _eq_ problem, as I thought it's just some kind of Python3 formality. Looks OK now.

vbraun commented 4 years ago
comment:44

PDF docs don't build:

[docpdf] l.4668 \(1,^^H
[docpdf]               eta,\ldots,^^Heta^{sm}\) be the power basis that SageMath uses to
dimpase commented 4 years ago
comment:45

hmm, is \beta the problem, or \ldots, or both?

xcaruso commented 4 years ago
comment:46

I'm not 100% sure but I think it comes from \beta. However, I don't know what's wrong.

Besides, reading this doctest, I find that it's not very clear; for instance, what is q is implicit and not obvious, I would say. Moreover, the two notations GF(q) and F_{q} are used to refer to the same object (namely, the finite field with q elements) and I think that they are both incorrect, the correct one being \GF(q).

dimpase commented 4 years ago
comment:47

by the way, there are typos - beta vs \beta in 6 places.

Perhaps \beta must be added to some silly list... (and/or written in utf-8)

dimpase commented 4 years ago

Changed commit from 5f46031 to 899d390

dimpase commented 4 years ago

Changed dependencies from #28350 to none

dimpase commented 4 years ago

Changed branch from u/jsrn/coding/linear_rank_metric to u/dimpase/coding/linear_rank_metric

dimpase commented 4 years ago
comment:48

ok, this works with pdf


New commits:

899d390make \beta unicode
vbraun commented 4 years ago

Changed branch from u/dimpase/coding/linear_rank_metric to 899d390

dimpase commented 4 years ago
comment:50

the last commit did not make it into the beta, or got overwritten

dimpase commented 4 years ago

Changed commit from 899d390 to none

dimpase commented 4 years ago
comment:51

Volker, something went missing with this merge, e.g. the last commit, ​899d390, did not get merged. ​

vbraun commented 4 years ago
comment:52

This ticket isn't in any relased beta yet. 899d390018 will be in 9.2.beta7 which I'm releasing right now

dimpase commented 4 years ago
comment:53

OK - it was confusing that it was marked as closed already.