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

Cached generator matrices and parity check matrices should be immutable #21159

Closed
johanrosenkilde opened this issue Aug 3, 2016 · 16 comments
Closed

Comments

@johanrosenkilde
Copy link
Contributor

In all linear code classes, constructed generator matrices and parity check matrices are cached for efficiency. However, they are often not immutable, leading to incorrect behaviour if the user inadvertently changes them.

sage: C = codes.GeneralizedReedSolomonCode(GF(7).list(), 3)
sage: C.generator_matrix()[0,0] = 0
sage: C.generator_matrix().row(0) in C
False

All such cached matrices should be made immutable by G.set_immutable(True).

Depends on #21328

CC: @sagetrac-dlucas

Component: coding theory

Keywords: linear code, rd3

Author: Clément Pernet

Branch/Commit: ac0e4f9

Reviewer: David Lucas

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

@johanrosenkilde

This comment has been minimized.

@ClementPernet
Copy link
Contributor

Branch: u/cpernet/immutable_generator_matrix

@ClementPernet
Copy link
Contributor

comment:3

I processed all code classes with the following update:

  • add some @cached_method decorator when it was missing to a generator_matrix or a parity_check method
  • systematically set immutable these cached matrices
    Branch attached ready for review.

@ClementPernet
Copy link
Contributor

Commit: d07341b

@ClementPernet
Copy link
Contributor

Author: Clément Pernet

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2017

Changed commit from d07341b to 8409076

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2017

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

8409076fix mistake: make the matrix immutable, not the tuple

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

comment:7

Tests pass and doc builds, I agree with your changes, setting to positive review.

David

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

Reviewer: David Lucas

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

Changed keywords from linear code to linear code, rd3

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

Changed commit from 8409076 to ac0e4f9

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

comment:9

Well, reopenig this ticket to include parity check codes (#21328) in it.
Waiting for review...

David


New commits:

b77de0fAdded new file that introduces the new parity-check code class. Initializes code class, generator matrix encoder class, straightforward encoder class and minimum distance method.
0b0d2f8Correcting mistakes. Two errors are still waiting for correction: ParityCheckCodeStraightforwardEncoder.unencode_nocheck and ParityCheckCodeGeneratorMatrixEncoder.__init__
62abcd7Fixed merge conflict.
1aa4b4eFixed doctests. Shorter output. Generator matrix encoder inherits from the generic one. Fixed encoders. Cleaned the code.
64ee227Merged ticket #21328 in
ac0e4f9Immutable generator matrix for parity codes

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Feb 7, 2017

Dependencies: #21328

@ClementPernet
Copy link
Contributor

comment:11

Positive review to the merge and last commit by dlucas.

@vbraun
Copy link
Member

vbraun commented Feb 14, 2017

Changed branch from u/dlucas/immutable_generator_matrix to ac0e4f9

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

3 participants