Opened 5 years ago

Closed 4 years ago

#21159 closed defect (fixed)

Cached generator matrices and parity check matrices should be immutable

Reported by: jsrn Owned by:
Priority: major Milestone: sage-7.6
Component: coding theory Keywords: linear code, beginner, rd3
Cc: dlucas Merged in:
Authors: Clément Pernet Reviewers: David Lucas
Report Upstream: N/A Work issues:
Branch: ac0e4f9 (Commits, GitHub, GitLab) Commit: ac0e4f91f1f964492d6c8732d3d1de80593d59da
Dependencies: #21328 Stopgaps:

Status badges

Description (last modified by jsrn)

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).

Change History (12)

comment:1 Changed 5 years ago by jsrn

  • Description modified (diff)

comment:2 Changed 4 years ago by cpernet

  • Branch set to u/cpernet/immutable_generator_matrix

comment:3 Changed 4 years ago by cpernet

  • Authors set to Clément Pernet
  • Commit set to d07341b9e50608753929ef79dd538cc73823f358
  • Component changed from PLEASE CHANGE to coding theory
  • Milestone changed from sage-7.3 to sage-7.6
  • Status changed from new to needs_review

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.

comment:4 Changed 4 years ago by cpernet

  • Status changed from needs_review to needs_work

comment:5 Changed 4 years ago by git

  • Commit changed from d07341b9e50608753929ef79dd538cc73823f358 to 84090767d241f9e6cd9c5f0df779b05ce746ac64

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

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

comment:6 Changed 4 years ago by cpernet

  • Status changed from needs_work to needs_review

comment:7 Changed 4 years ago by dlucas

  • Keywords rd3 added
  • Reviewers set to David Lucas
  • Status changed from needs_review to positive_review

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

David

comment:8 Changed 4 years ago by dlucas

  • Branch changed from u/cpernet/immutable_generator_matrix to u/dlucas/immutable_generator_matrix

comment:9 Changed 4 years ago by dlucas

  • Commit changed from 84090767d241f9e6cd9c5f0df779b05ce746ac64 to ac0e4f91f1f964492d6c8732d3d1de80593d59da
  • Status changed from positive_review to needs_review

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

comment:10 Changed 4 years ago by dlucas

  • Dependencies set to #21328

comment:11 Changed 4 years ago by cpernet

  • Status changed from needs_review to positive_review

Positive review to the merge and last commit by dlucas.

comment:12 Changed 4 years ago by vbraun

  • Branch changed from u/dlucas/immutable_generator_matrix to ac0e4f91f1f964492d6c8732d3d1de80593d59da
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.