Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Information set decoding for linear codes #20138

Closed
sagetrac-dlucas mannequin opened this issue Feb 29, 2016 · 94 comments
Closed

Information set decoding for linear codes #20138

sagetrac-dlucas mannequin opened this issue Feb 29, 2016 · 94 comments

Comments

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 29, 2016

This ticket proposes an implementation of Lee-Brickell algorithm refinement for the information set decoding.

It depends on #19653 for the reimplementation of C.random_element.

Depends on #19653

CC: @jlavauzelle

Component: coding theory

Author: David Lucas, Johan Rosenkilde, Yann Laigle-Chapuy

Branch/Commit: eb2efb3

Reviewer: Yann Laigle-Chapuy

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

@sagetrac-dlucas sagetrac-dlucas mannequin added this to the sage-7.1 milestone Feb 29, 2016
@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Feb 29, 2016

Branch: u/dlucas/information_set_decoding

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Feb 29, 2016

Commit: 645e329

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Feb 29, 2016

comment:2

I pushed the first version of the code for this ticket.


New commits:

884d8abNew decoder, InformationSetDecoder
351b30cSet this new decoder in other code families and in the catalog
645e329Fixed broken doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 3, 2016

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

6292df9window_size and number_errors are now class arguments
e72a5abRewrote `_repr_` and `_latex_` methods, fixed a typo
f2c074cdecode_to_code now iterates over the whole range of provided errors
0e1391dAdded *args to Decoder/Encoder-related methods in AbstractLinearCode

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 3, 2016

Changed commit from 645e329 to 0e1391d

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 3, 2016

Author: David Lucas

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Mar 3, 2016

comment:4

I made some changes:

  • window_size and number_errors (previously p and w) are no longer parameters of decode_to_code but class arguments to provide at construction time. number_errors can be provided as a tuple/list, and in that case, the decoder will iterate over it when trying to decode a word.

  • I completely reworked the documentation, explained how the algorithm works and added a lot of doctests.

  • I completed input sanitization with some extra checks.

  • I fixed a stupid bug: only **kwargs was considered on Decoder/Encoder-related methods from AbstractLinearCode, like decoder(). So, C.decoder('InformationSet', 2, 2) was not working...

This is now open for review.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2016

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

8192769Updated to 7.1beta6 and fixed conflicts

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2016

Changed commit from 0e1391d to 8192769

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 26, 2016

Changed commit from 8192769 to 924be79

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 26, 2016

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

465cd56Merge branch 'develop' into information_set_decoding
59de997Added information set decoder to Hamming codes
924be79Minor changes to the information set decoder

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Apr 26, 2016

comment:7

I updated this ticket to latest beta, fixed broken doctests, and did minor changes to the decoder (I refined its types and rewrote a line in the documentation).
It's still open for review.

@sagetrac-dlucas sagetrac-dlucas mannequin modified the milestones: sage-7.1, sage-7.2 Apr 26, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 3, 2016

Changed commit from 924be79 to 93066b1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 3, 2016

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

62468b9Updated to latest beta and fixed conflict
93066b1Fixed silly copy-paste mistake in ISD's decoder_type

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 3, 2016

comment:9

I updated this ticket to 7.3b2, and fixed a bug.
Still open for review.

Best,

David

@sagetrac-dlucas sagetrac-dlucas mannequin modified the milestones: sage-7.2, sage-7.3 Jun 3, 2016
@sagetrac-ylchapuy
Copy link
Mannequin

sagetrac-ylchapuy mannequin commented Sep 1, 2016

comment:10

Here are my two cents:

  • you should use itertools instead of Subsets
  • you shouldn't enumerate the information_set but take them at random (otherwise you might get stuck a long time with bad choices)
  • why use while True loops instead of just for?

all in all, here is how I would do it (notations from the article you cite):

import itertools
def LeeBrickell(r, C, w, p):
    G = C.generator_matrix()
    n = C.length()
    k = C.dimension()
    Fstar = list(C.base_ring())[1:]
    while True:
        # step 1.
        I = sample(range(n), k)
        Gi = G.matrix_from_columns(I)
        try:
            Gi_inv = Gi.inverse()
        except ZeroDivisionError:
            continue
        G = Gi_inv * G
        #step 2.
        y = r - vector([r[i] for i in I]) * G
        g = G.rows()
        #step 3.
        for A in itertools.combinations(range(k), p):
            for m in itertools.product(Fstar, repeat=p):
                e = y - sum(m[i]*g[A[i]] for i in range(p))
                if e.hamming_weight() == w:
                    return r - e

@sagetrac-ylchapuy
Copy link
Mannequin

sagetrac-ylchapuy mannequin commented Sep 1, 2016

Reviewer: Yann Laigle-Chapuy

@sagetrac-ylchapuy
Copy link
Mannequin

sagetrac-ylchapuy mannequin commented Sep 1, 2016

comment:11

You should also check if the resulting word is in the code (it might not be the case if you try to decode beyond the minimal distance).

@sagetrac-ylchapuy
Copy link
Mannequin

sagetrac-ylchapuy mannequin commented Sep 2, 2016

comment:12

In case you want to optimize this a bit, you can replace the itertools enumerations with sage.combinat.gray_codes stuff (and if the field is GF(2) get rid of the itertools.product).

This is probably better kept for another ticket.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 8, 2017

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

50018f6Fix doctest failure in doc. Add docstring and test to __hash__

@johanrosenkilde
Copy link
Contributor

comment:63

Indeed. These omissions are fixed now.

@sagetrac-ylchapuy
Copy link
Mannequin

sagetrac-ylchapuy mannequin commented Jul 16, 2017

comment:64

Ok, I finally took the time to build the doc and check it out. Build without a fuss and LGTM.

Your long doctest with codes.QuadraticResidueCode(59, GF(3)) takes around 150ms on my machine, I don't know if I would call this long but it's not a problem.

The bot is also green, so let's move forward: positive review :)

@sagetrac-ylchapuy sagetrac-ylchapuy mannequin modified the milestones: sage-7.3, sage-8.0 Jul 16, 2017
@johanrosenkilde
Copy link
Contributor

comment:65

Great! Thanks for the cooperation - I think this has become a really nice addition to Sage. I'm looking forward to seeing your implementation in #23244.

@vbraun
Copy link
Member

vbraun commented Jul 22, 2017

comment:66
sage -t --long src/sage/coding/information_set_decoder.py
**********************************************************************
File "src/sage/coding/information_set_decoder.py", line 553, in sage.coding.information_set_decoder.LeeBrickellISDAlgorithm.calibrate
Failed example:
    A.time_estimate()/5 < avg and avg < A.time_estimate() * 5 # long time
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   1 of  15 in sage.coding.information_set_decoder.LeeBrickellISDAlgorithm.calibrate
    [170 tests, 1 failure, 5.71 s]

@kiwifb
Copy link
Member

kiwifb commented Jul 23, 2017

comment:67

Replying to @vbraun:

sage -t --long src/sage/coding/information_set_decoder.py
**********************************************************************
File "src/sage/coding/information_set_decoder.py", line 553, in sage.coding.information_set_decoder.LeeBrickellISDAlgorithm.calibrate
Failed example:
    A.time_estimate()/5 < avg and avg < A.time_estimate() * 5 # long time
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   1 of  15 in sage.coding.information_set_decoder.LeeBrickellISDAlgorithm.calibrate
    [170 tests, 1 failure, 5.71 s]

I couldn't get that on my setup while repeating the test a 1000 time.

@johanrosenkilde
Copy link
Contributor

comment:68

Hmm, we were also in doubt about that test, as you can see from the discussion above. I had believed that it was fairly stable now, but evidently timing behaviour on the build bots are not very dependable.

So perhaps we should simply remove the test - what do you think? Any alternative suggestions for how we could test the calibration code?

@vbraun
Copy link
Member

vbraun commented Jul 23, 2017

comment:69
  • Don't test that the timings have particular values
  • Do test that timings can be collected
  • Do test that your code makes the right decisions when you specify timings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2017

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

eb2efb3Replace brittle calibration test with a test for proper selection

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2017

Changed commit from 50018f6 to eb2efb3

@johanrosenkilde
Copy link
Contributor

comment:71

Replying to @vbraun:

  • Don't test that the timings have particular values
  • Do test that timings can be collected
  • Do test that your code makes the right decisions when you specify timings

OK, I've pushed a patch removing the offending test and instead making a small refactor such that we can test the last item on your list.

It seems a pity, however, that there is no good way to test that the calibrated timings have anything to do with reality.

@sagetrac-ylchapuy
Copy link
Mannequin

sagetrac-ylchapuy mannequin commented Jul 27, 2017

comment:73

OK, let's do it that way (and the failure in the docbuild is unrelated)

LGTM

@vbraun
Copy link
Member

vbraun commented Jul 29, 2017

Changed branch from u/jsrn/information_set_decoding to eb2efb3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants