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

New functions for binary linear codes #14973

Closed
sagetrac-veronica mannequin opened this issue Jul 26, 2013 · 33 comments
Closed

New functions for binary linear codes #14973

sagetrac-veronica mannequin opened this issue Jul 26, 2013 · 33 comments

Comments

@sagetrac-veronica
Copy link
Mannequin

sagetrac-veronica mannequin commented Jul 26, 2013

The idea of this work is to add some functions to linear_code.py and new decoding algorithms.
Theory and algorithms are taken from: I. Marquez-Corbella. A combinatorial Commutative Algebra Approach to Complete Decoding PhD thesis, University of Valladolid, 2013.

Apply to devel/sage: attachment: trac_14973-new_decoding_functions.patch

CC: @ppurka @dimpase

Component: coding theory

Keywords: binary codes

Branch/Commit: u/ppurka/ticket/14973 @ 743826b

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

@sagetrac-veronica sagetrac-veronica mannequin added this to the sage-6.1 milestone Jul 26, 2013
@fchapoton
Copy link
Contributor

comment:2

one quick comment: you should also add an example for the function __insert_nextnew

@burcin
Copy link

burcin commented Jul 26, 2013

comment:3

This is just a list of short comments, nothing like a comprehensive review. Anybody else looking at this should feel free to test the code and report problems.

Did you run the tests in this file? I get

Running doctests with ID 2013-07-26-10-31-10-4d8b78a5.
Doctesting 1 file.
sage -t devel/sage/sage/coding/linear_code.py
**********************************************************************
File "devel/sage/sage/coding/linear_code.py", line 2996, in sage.coding.linear_code.LinearCode.newton_radius
Failed example:
    H = matrix(GF(2),[[1,0,0,0,1,0,0,0,0,0],[1,0,1,1,0,1,0,0,0,0],[1,1,0,1,0,0,1,0,0,0],
Exception raised:
    Traceback (most recent call last):
      File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 485, in _run
        compileflags, 1)
      File "<doctest sage.coding.linear_code.LinearCode.newton_radius[0]>", line 1
        H = matrix(GF(Integer(2)),[[Integer(1),Integer(0),Integer(0),Integer(0),Integer(1),Integer(0),Integer(0),Integer(0),Integer(0),Integer(0)],[Integer(1),Integer(0),Integer(1),Integer(1),Integer(0),Integer(1),Integer(0),Integer(0),Integer(0),Integer(0)],[Integer(1),Integer(1),Integer(0),Integer(1),Integer(0),Integer(0),Integer(1),Integer(0),Integer(0),Integer(0)],
                                                                                                                                                                                                                                                                                                                                                                                  ^
    SyntaxError: unexpected EOF while parsing
**********************************************************************
File "devel/sage/sage/coding/linear_code.py", line 2998, in sage.coding.linear_code.LinearCode.newton_radius
Failed example:
    C = LinearCodeFromCheckMatrix(H)
Exception raised:
    Traceback (most recent call last):
      File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 486, in _run
        self.execute(example, compiled, test.globs)
      File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 845, in execute
        exec compiled in globs
      File "<doctest sage.coding.linear_code.LinearCode.newton_radius[1]>", line 1, in <module>
        C = LinearCodeFromCheckMatrix(H)
    NameError: name 'H' is not defined
**********************************************************************
File "devel/sage/sage/coding/linear_code.py", line 3000, in sage.coding.linear_code.LinearCode.newton_radius
Failed example:
    C.newton_radius(t)
Exception raised:
    Traceback (most recent call last):
      File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 486, in _run
        self.execute(example, compiled, test.globs)
      File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 845, in execute
        exec compiled in globs
      File "<doctest sage.coding.linear_code.LinearCode.newton_radius[3]>", line 1, in <module>
        C.newton_radius(t)
    NameError: name 'C' is not defined
**********************************************************************
1 item had failures:
   3 of   5 in sage.coding.linear_code.LinearCode.newton_radius
    [374 tests, 3 failures, 5.76 s]
----------------------------------------------------------------------
sage -t devel/sage/sage/coding/linear_code.py  # 3 doctests failed
----------------------------------------------------------------------
Total time for all tests: 5.9 seconds
    cpu time: 4.4 seconds
    cumulative wall time: 5.8 seconds

After fixing that problem, I still have:

Running doctests with ID 2013-07-26-10-31-50-0e819b57.
Doctesting 1 file.
sage -t devel/sage/sage/coding/linear_code.py
**********************************************************************
File "devel/sage/sage/coding/linear_code.py", line 2999, in sage.coding.linear_code.LinearCode.newton_radius
Failed example:
    C.newton_radius(t)
Exception raised:
    Traceback (most recent call last):
      File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 486, in _run
        self.execute(example, compiled, test.globs)
      File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 845, in execute
        exec compiled in globs
      File "<doctest sage.coding.linear_code.LinearCode.newton_radius[3]>", line 1, in <module>
        C.newton_radius(t)
    TypeError: newton_radius() takes exactly 1 argument (2 given)
**********************************************************************
1 item had failures:
   1 of   5 in sage.coding.linear_code.LinearCode.newton_radius
    [374 tests, 1 failure, 4.69 s]
----------------------------------------------------------------------
sage -t devel/sage/sage/coding/linear_code.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 4.9 seconds
    cpu time: 4.3 seconds
    cumulative wall time: 4.7 seconds

Please document and test __insert_nextnew() as well. All new functions introduced in a patch should be tested.

You should rename covering_rad() to covering_radius().

The documentation needs a spell check. Did you try building the documentation? I'm not sure if all of it is formatted properly.

The output of weight_distribution_coset() and groebner_representation() should be immutable, i.e., a tuple instead of a list. It might be better to reconsider the format of groebner_representation() altogether. A list of lists is not very mathematical. There are quite a few possibilities in Sage to represent various mathematical structures.

Please run your code through a pep8 checker: https://pypi.python.org/pypi/pep8

In Python, you don't need to write CC[len(CC) - 1] to get the last element of a list. You can just say CC[-1].

It seems that all your examples/tests use the same code. Can you add different examples which stress the code properly, covering corner cases, etc.?

@miguelmarco
Copy link
Contributor

comment:4

Burcin, which structure would you consider appropriate for representing the groebner representation? Maybe a dictionary?

@miguelmarco
Copy link
Contributor

comment:5

Are you sure you uploaded the last version of your patch? The version __insert_nextnew does not have any doctest. And the function covering_rad should be merged with the existing covering_radius, providing an option to choose between the preexisting algorithm, and the one you implemented.

@sagetrac-veronica
Copy link
Mannequin Author

sagetrac-veronica mannequin commented Jul 27, 2013

comment:6

Here goes the real patch:

-Right tests (Doctests already run)

-_insert_nextnew() documented and tested

-The functions covering_rad() was merged with covering_radius(), this function was already implemented but required optional GAP package guava.
Now you can indicate which method use. Algorithm = "guava" does the same that
the old covering_radius() using guava. And Algorithm = None is my implemented
function.

-The output of weight_distribution_coset() is now a tuple.

About making output of groebner_representation() immutable: This function has been implemented
because in next step I'm going to use it for a decoding algorithm. And actually it's going to be more convenient
to change the output to a dictionary, though.

I'll document more examples. I'm testing coset_leaders() to determinate how big can be the length and dimension of the code before
the algorithm gets slow.

@burcin
Copy link

burcin commented Jul 28, 2013

comment:7

With attachment: coding_theory.2.patch:

Running doctests with ID 2013-07-28-09-33-28-48923423.
Doctesting 1 file.
Traceback (most recent call last):
  File "/home/burcin/sage/sage-5.11.beta3/local/bin/sage-runtests", line 87, in <module>
    err = DC.run()
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/control.py", line 891, in run
    self.run_doctests()
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/control.py", line 661, in run_doctests
    self.dispatcher = DocTestDispatcher(self)
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1339, in __init__
    init_sage()
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 93, in init_sage
    import sage.all_cmdline
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/all_cmdline.py", line 18, in <module>
    from sage.all import *
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/all.py", line 124, in <module>
    from sage.coding.all     import *
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/coding/__init__.py", line 1, in <module>
    import all
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/coding/all.py", line 1, in <module>
    from ag_code import ag_code
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/coding/ag_code.py", line 20, in <module>
    import linear_code
  File "/home/burcin/sage/sage-5.11.beta3/local/lib/python2.7/site-packages/sage/coding/linear_code.py", line 1266
    F = self.base_ring()
    ^
IndentationError: expected an indented block

After fixing this, all tests pass.

@ppurka
Copy link
Member

ppurka commented Jul 28, 2013

comment:8

Some comments:

  1. The indentation error is not only in the line F = self.base_ring(). All of the existing code in the covering_radius method needs to be indented.
  2. The @rename_keyword should not be introduced at all in the covering_radius code. This is a new keyword to the covering radius method and so you can simply introduce it without any deprecation warnings.
  3. Is all of your code only for linear codes over GF(2)? Then you need to check the inputs to make sure that if other parents are input then it either degrades gracefully and automatically to an older algorithm (for instance in covering_radius), or it gives an error.
  4. Do you have plans to use values of order other than degrevlex?

In fact, I don't understand why ETuple is being used. The only place I am finding it really being used is in comparing of two lists (the compare_tuples_degrevlex function). I raise this point because quite a bit of time would be spent in simply converting vectors to lists and then the lists to ETuples. Later on, you also need the ETuples to be converted back to vectors in the grobner_representation function. If there is a more direct way of doing the comparison you are doing (without all these conversions to ETuple and back), then that should speed up the computations a bit.

@sagetrac-veronica
Copy link
Mannequin Author

sagetrac-veronica mannequin commented Jul 29, 2013

comment:9

Replying to @ppurka:

Some comments:

Thanks for your comments!!

  1. The indentation error is not only in the line F = self.base_ring(). All of the existing code in the covering_radius method needs to be indented.

Yes, it got moved after I runned the tests, I guess. I just fixed it.

  1. The @rename_keyword should not be introduced at all in the covering_radius code. This is a new keyword to the covering radius method and so you can simply introduce it without any deprecation warnings.

Ok I got it. Also fixed

  1. Is all of your code only for linear codes over GF(2)? Then you need to check the inputs to make sure that if other parents are input then it either degrades gracefully and automatically to an older algorithm (for instance in covering_radius), or it gives an error.

So far it is only for linear codes over GF(2). I'll do the check in every function.

  1. Do you have plans to use values of order other than degrevlex?

degrevlex in most of the cases reduce the number of operations to compute the grobner_basis.
Even that I'd like to leave it as option to the user, but maybe I should do this only for the function groebner_basis()(still at work)
In other cases like grobner_representation() and coset_leaders() I should use only degrevlex.

In fact, I don't understand why ETuple is being used. The only place I am finding it really being used is in comparing of two lists (the compare_tuples_degrevlex function). I raise this point because quite a bit of time would be spent in simply converting vectors to lists and then the lists to ETuples. Later on, you also need the ETuples to be converted back to vectors in the grobner_representation function. If there is a more direct way of doing the comparison you are doing (without all these conversions to ETuple and back), then that should speed up the computations a bit.

You're right
Actually the set of coset leaders it doesn't depend on the order, so at least for this function I'm doing the implementation without using ETuple. I also think that should speed up the computations.

grobner_representation() is going to be used in one of the decoding algorithm, so maybe I can also only use degrevlex, I'll discuss it with my mentors.

@sagetrac-imarquez
Copy link
Mannequin

sagetrac-imarquez mannequin commented Jul 29, 2013

comment:10

I will just add some comments about the last question: as Veronica already mentioned, for the decoding algorithm that Veronica is implementing (whose code will be public in the following days), the term ordering is important.

Note that for those cosets for which more than one coset leader exists the degree compatible ordering chosen will play an important role.

However, in general I will suggest to leave as default the "degrevlex" ordering for several reasons related to the preprocessing of the mentioned decoding algorithm.

@sagetrac-veronica
Copy link
Mannequin Author

sagetrac-veronica mannequin commented Aug 3, 2013

Attachment: coding_theory.patch.gz

New patch with the GDDA decoding algorithm

@sagetrac-veronica
Copy link
Mannequin Author

sagetrac-veronica mannequin commented Aug 3, 2013

comment:11

Here is the patch with the last details fixed. And I added the GDDA algorithm. This algorithm with Hamming Codes and Random linear codes seems to be faster.
For example I have tested(among others):

sage: C = HammingCode(5,GF(2))                                                                                                                                
sage: v = random_vector(GF(2),C.length())  
sage: v                                                                                                                                                       
(1,1,0,1,0,1,0,0,0,0,1,0,1,0,1,0,0,1,1,1,0,1,1,0,0,0,1,1,0,0,1)                                                                                                                      
#using syndrome algorithm
sage: %time C.decode(v)                                                                                                                                       
CPU times: user 847.37 s, sys: 4.45 s, total: 851.82 s                                                                                                        
Wall time: 868.36 s                                                                                                                                           
(1,1,0,0,0,1,0,0,0,0,1,0,1,0,1,0,0,1,1,1,0,1,1,0,0,0,1,1,0,0,1)          

#using nearest neighbor algorithm
sage: %time C.decode(v,"nearest neighbor")                                                                                                                    
CPU times: user 879.19 s, sys: 1.61 s, total: 880.80 s                                                                                                        
Wall time: 880.80 s                                                                                                                                           
(1,1,0,0,0,1,0,0,0,0,1,0,1,0,1,0,0,1,1,1,0,1,1,0,0,0,1,1,0,0,1)        

#using guava algorithm       
sage: %time C.decode(v,"guava")                                                                                                                               
CPU times: user 0.07 s, sys: 0.02 s, total: 0.08 s                                                                                                            
Wall time: 1.00 s                                                                                                                                             
(1,1,0,0,0,1,0,0,0,0,1,0,1,0,1,0,0,1,1,1,0,1,1,0,0,0,1,1,0,0,1)                                                  

#using GDDA algorithm (first time compute grobner representation)
sage: %time C.decode_gdda(v)                                                                                                                                  
CPU times: user 0.73 s, sys: 0.06 s, total: 0.78 s                                                                                                            
Wall time: 0.84 s                                                                                                                                             
(1,1,0,0,0,1,0,0,0,0,1,0,1,0,1,0,0,1,1,1,0,1,1,0,0,0,1,1,0,0,1)    

#using GDDA algorithm (with grobner representation computed)                                                             
sage: %time C.decode_gdda(v)                                                                                                                                  
CPU times: user 0.00 s, sys: 0.00 s, total: 0.00 s                                                                                                            
Wall time: 0.00 s                                                                                                                                             
(1,1,0,0,0,1,0,0,0,0,1,0,1,0,1,0,0,1,1,1,0,1,1,0,0,0,1,1,0,0,1)            


I've been suggeste to use timeit instead of time for benchmarking, so that's next. But with this results of time we can appreciate the differences among the differents decoding algorithms

@sagetrac-veronica
Copy link
Mannequin Author

sagetrac-veronica mannequin commented Sep 23, 2013

comment:12

The next patch is presented as result of a GSoC project 2013.

The work contains implementation of new decoding algorithms for linear codes proposed in "Combinatorial Commutative Algebra Approach to Complete Decoding", PhD Thesis, University of Valladolid, 2013 by Irene Marquez-Corbella.
Algorithms are called "groebner representation" and "groebner basis" and has been added to sage.coding.decoder.py
This algorithms has been hooked up with the decode function in sage.coding.linear_code.py, adding also a block of tests in which is explained the cases where this new algorithms performs better.

The patch contains functions added to sage.coding.linear_code.py such as coset_leaders, error_correcting_capacity, newton_radius, weight_distribution_coset, and new algorithm to covering_radius.

@ppurka
Copy link
Member

ppurka commented Sep 23, 2013

comment:13

Patchbot apply trac_14973-new_decoding_functions.patch

@ppurka

This comment has been minimized.

@ppurka
Copy link
Member

ppurka commented Sep 23, 2013

comment:14

Hmm.. I didn't notice these two issues before:

  1. The syndrome() function is (strangely) duplicated in decoder.py

  2. At some point during the GSOC, we made the output of groebner_basis_singular a list, to make the output similar to the output of the other groebner_basis_fglm function (which was probably named differently). Now, I feel that the generator itself should be returned, i.e., the return statement should be return I.groebner_basis('libsingular:stdfglm') because the output of this function is used sequentially, and used only once. Consequently, all the doctests should be changed to list(groebner_basis_singular..).

@sagetrac-veronica
Copy link
Mannequin Author

sagetrac-veronica mannequin commented Sep 25, 2013

Attachment: trac_14973-new_decoding_functions.patch.gz

@sagetrac-veronica
Copy link
Mannequin Author

sagetrac-veronica mannequin commented Sep 25, 2013

comment:15

I just changed the patch.

  • The syndrome function duplicated has been deleted.
  • groebner_basis_fglm function output is now a generator iterable object.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@fchapoton
Copy link
Contributor

comment:18

This needs to be turned into a git branch (and rebased)

@defeo
Copy link
Member

defeo commented May 22, 2014

Branch: u/defeo/ticket/14973

@ppurka
Copy link
Member

ppurka commented May 24, 2014

comment:22

I am working on resolving the (seems trivial) conflict with 6.3beta1.

@ppurka
Copy link
Member

ppurka commented May 24, 2014

Work Issues: resolve merge conflict with 6.3b1

@ppurka
Copy link
Member

ppurka commented May 24, 2014

comment:23

I added some commits to fix the merge conflicts and also clean up a bit.

Your changes looked ok to me except for just one change that was not present. You can look at the changes that you made and compare with the changes in the hg patch attached to this ticket, like so.

$ git diff develop d02a468  > coding.patch # alternatively, you can use your HEAD: d21956c
$ <some visual diff tool> coding.patch  trac_14973-new_decoding_functions.patch

I used gvimdiff as the visual diff tool. There are many available like meld, kdiff3, etc.

@ppurka
Copy link
Member

ppurka commented May 24, 2014

Changed branch from u/defeo/ticket/14973 to u/ppurka/ticket/14973

@ppurka
Copy link
Member

ppurka commented May 24, 2014

Changed work issues from resolve merge conflict with 6.3b1 to none

@ppurka
Copy link
Member

ppurka commented May 24, 2014

New commits:

d02a468merge develop on to #14973; resolved merge conflicts
9f31603fix docstrings in decoder.py
32d9c4ffix some docstrings in linear_code.py
bf4094dremove deprecation message about method to algorithm from sage/coding
eba4566one more merge conflict that was missed
b1850bagroebner -> Groebner

@ppurka
Copy link
Member

ppurka commented May 24, 2014

Changed commit from d21956c to b1850ba

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 24, 2014

Changed commit from b1850ba to 743826b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 24, 2014

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

743826bfix some documentation build problems

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@fchapoton
Copy link
Contributor

comment:28

needs rebase

@johanrosenkilde
Copy link
Contributor

comment:29

See #21339 which is a re-adaptation of this ticket.

@embray
Copy link
Contributor

embray commented Aug 30, 2016

comment:31

Determined to be invalid/duplicate/wontfix (closing as "wontfix" as a catch-all resolution).

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

7 participants