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

Systematic encoder for linear codes #20835

Closed
sagetrac-dlucas mannequin opened this issue Jun 16, 2016 · 24 comments
Closed

Systematic encoder for linear codes #20835

sagetrac-dlucas mannequin opened this issue Jun 16, 2016 · 24 comments

Comments

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Jun 16, 2016

This ticket introduces a generic systematic encoder for linear codes.

Depends on #20840

CC: @johanrosenkilde @ClementPernet

Component: coding theory

Author: David Lucas

Branch/Commit: 3a2f5ac

Reviewer: Johan Rosenkilde

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

@sagetrac-dlucas sagetrac-dlucas mannequin added this to the sage-7.3 milestone Jun 16, 2016
@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 16, 2016

Branch: u/dlucas/systematic_encoder

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 16, 2016

comment:2

I pushed the code, this ticket is now open for review.


New commits:

54b50b7Implemented a systematic encoder
ebe977bMethod generator_matrix_systematic now uses the encoder

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 16, 2016

Author: David Lucas

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 16, 2016

Commit: ebe977b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 17, 2016

Changed commit from ebe977b to 7248f40

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 17, 2016

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

51205d0Changed `_repr_` and `_latex_` output message. Replaced for the by for in such messages in other objects' representation methods
2d02053Renamed rank_profile_colum_indices, simplified its implementation
7248f40Rewrote class documentation of the encoder

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 17, 2016

comment:4

I made some changes to this ticket:

  • I simplified the output message of _repr_ and _latex_. I also noticed other encoders in linear_code.py were outputting Encoder blah for the Code blah, while Sage convention is to write Encoder blah for Code blah. I fixed this as well.

  • I renamed rank_profile_column_indices into systematic_positions, and utterly simplified its implementation.

  • I also added details on the encoding, and what we call "systematic form" to the class docstring.

This is still open for review.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 17, 2016

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

55f8771Added automatic registration for generic encoder/decoders
550c584Added a note to document this feature in related tutorial
ad1caddfixed broken doctest in the introductory tutorial
2887ee1Merged trac #20840, fixed conflicts and broken doctets

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 17, 2016

Changed commit from 7248f40 to 2887ee1

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 17, 2016

Dependencies: #20840

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 17, 2016

comment:6

As this ticket introduces a new generic encoder, I merged in #20840 and made the appropriate changes.
It now depends on #20840 and is still open for review (and I'm done pushing changes now ;))

David

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 29, 2016

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

5036711Merge branch 'develop' into systematic_encoder
83cb47aSystematic encoder now includes parity check encoder's behaviour. Deprecated parity check encoder
8dc72f5Parity check encoder is no longer systematically imported. Reinstated it in hamming_code.py
46485e6Removed useless imports
6d01e05Fixed broken doctests
bb41e21Merged latest version of #20835 and fixed conflicts

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 29, 2016

Changed commit from 2887ee1 to bb41e21

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Jun 29, 2016

comment:8

As LinearCodeParityCheckEncoder and LinearCodeSystematicEncoder were actually doing the same thing, I modified the latter so it now uses a parity check matrix when computing a generator matrix if it is the default encoder. I also deprecated the former.
I also merged in latest version of #20840 and fixed conflicts.

This is open for review.

David

@johanrosenkilde
Copy link
Contributor

comment:9

In AbstractLinearCode.generator_matrix_systematic, shouldn't the method call AbstractLinearCode.encoder in order to use a possible cached systematic encoder?

Also, perhaps this ticket is as good a place as any to rename that method into systematic_generator_matrix and deprecate the old name.

Best,
Johan

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2016

Changed commit from bb41e21 to 2089792

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2016

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

170e79cMerge branch 'u/dlucas/systematic_encoder' of git://trac.sagemath.org/sage into systematic_encoder
9ab771aRenamed generator_matrix_systematic into systematic_generator_matrix, changed its implementation
2089792Fixed broken doctest in subfield_subcode.py

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Aug 3, 2016

comment:11

Hello,

I made the requested changes.

David

@johanrosenkilde
Copy link
Contributor

Changed branch from u/dlucas/systematic_encoder to u/jsrn/systematic_encoder

@johanrosenkilde
Copy link
Contributor

Reviewer: Johan Rosenkilde

@johanrosenkilde
Copy link
Contributor

comment:13

Good job with the patch. I made a few improvements to the doc. I also added a
check to avoid infinite recursion in case the user made a class with the systematic encoder as default encoder but forgot to override parity check matrix.

If you can accept my changes, please set the ticket to positive review.
Best,
Johan


New commits:

3e5e0b2Improved some documentation
f90f4eeAdd a catch to avoid infinite loop in case of user error + examples
ab1cc5aFix to encoder catalog doc
3a2f5acFixes to LinearCodeSystematicEncoder doc

@johanrosenkilde
Copy link
Contributor

Changed commit from 2089792 to 3a2f5ac

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Aug 3, 2016

comment:14

Tests pass, documentation compiles and I accept your changes, I set this ticket to positive review.

David

@vbraun
Copy link
Member

vbraun commented Aug 7, 2016

Changed branch from u/jsrn/systematic_encoder to 3a2f5ac

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

2 participants