Opened 3 years ago

Closed 3 years ago

#21165 closed enhancement (fixed)

Various cleanup and deprecation in `sage.coding.linear_code`

Reported by: jsrn Owned by:
Priority: major Milestone: sage-7.4
Component: coding theory Keywords: cleanup, sd75
Cc: dlucas Merged in:
Authors: Johan Rosenkilde Reviewers: David Lucas
Report Upstream: N/A Work issues:
Branch: 08bdda0 (Commits) Commit: 08bdda08b38e9552dbe4a77dc9ff57f9962c4938
Dependencies: Stopgaps:

Description (last modified by jsrn)

sage.coding.linear_code has a number of global functions and class methods which are badly named, badly documented, badly placed or badly conceived. This ticket takes some steps towards cleaning up.

This ticket fixes the following issues:

  • Renamed and moved best_known_linear_code, best_known_linear_code_www, bounds_minimum_distance and self_orthogonal_binary_codes to new module sage.coding.databases, also deprecating them from the global namespace. Also improved their documentation. Added the new module to codes.
  • Rename the module sd_codes to self_dual_codes.
  • Remove the function sd_codes.self_dual_codes_binary from the global namespace, renaming it into self_dual_binary_codes accessible from codes.databases. Removed all helper computations that were done at Sage startup from sd_codes, making them all private and evaluated only when calling self_dual_binary_codes.
  • Deprecated sd_zeta_polynomial and its helper functions. After struggling for a long time, I found out that this method supposed to compute exactly the same as zeta_polynomial, except that it uses a method which only works for self-dual codes. The method is a semi-explicit expression for the zeta function which has theoretical interest, but no computational interest (it is not faster to compute). So the method should simply be removed.
  • Deprecated LinearCodeFromVectorSpace and made LinearCode accept a vector space at construction time.
  • Improved some unhelpful doc-examples in sage.coding.extended_code.
  • Some cleanup of the imports on the top-level of linear_code.py.
  • Rename and deprecate from the global namespace the functions code2leon, min_wt_vec_gap.
  • Improve the deprecation message for wtdist_gap.
  • Improved doc of covering_radius.
  • Use weight_distribution as primary term instead of spectrum, but support the second as an alias. Inline the helper function _spectrum_from_gap. Improved the doc.
  • Renamed code_constructions.LinearCodeFromCheckMatrix to code_constructions.from_parity_check_matrix. Simplified its doc.
  • Changed a random doctest in punctured_code whose output changed after an unrelated patch (due to RandomLinearCode)
  • Simplified RandomLinearCode, renamed to codes.random_linear_code and fixed order of arguments.
  • Removed TrivialCode as it was mathematically senseless and hadn't worked for a while (it had no tests).
  • Massively modernized and simplified the doc of code_constructions.py.
  • Privatized and/or deprecated the helper functions of code_constructions.py.

Change History (23)

comment:1 Changed 3 years ago by jsrn

  • Branch set to u/jsrn/21165_linear_code_cleanup

comment:2 Changed 3 years ago by git

  • Commit set to b1329da78e45943d3e80cfc83b19cd93728cb592

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

dd334afRename LinearCodeFromCheckMatrix -> from_parity_check_matrix
20aa7e6Fix typo
462cc43Fix import and doc of from_parity_check_matrix
f18c494Reduce brittle and random doctest to something fixed.
a8f45aaRandomLinearCode(n, k, F) -> random_linear_code(F, n, k)
d88f4e4Removed TrivialCode. It makes no mathematical sense and hasn't worked for a long while, so no deprecation.
5268fb2Clarify and massively simplify module doc of code_constructions.py
da23dc6Some doctest markup fixes
b1329daPrivatize and/or deprecate the helper functions of code_constructions.py

comment:3 Changed 3 years ago by jsrn

  • Description modified (diff)
  • Status changed from new to needs_review

OK, I think I'm done for the moment. The diff touches many lines in many files, but most of those changes are trivial renaming. Substantial changes occurred in linear_code.py, databases.py and code_constructions.py.

comment:4 Changed 3 years ago by jsrn

  • Dependencies set to 20835

Forgot to add that I merged in #20835 since I modified some doctests to use the systematic generator matrix.

comment:5 Changed 3 years ago by jsrn

  • Dependencies changed from 20835 to #20835

comment:6 Changed 3 years ago by dlucas

  • Branch changed from u/jsrn/21165_linear_code_cleanup to u/dlucas/21165_linear_code_cleanup

comment:7 follow-up: Changed 3 years ago by dlucas

  • Commit changed from b1329da78e45943d3e80cfc83b19cd93728cb592 to 079e51ffbf79e6b3780e27341dc201a0b16d5ca7

Hello,

Thanks for cleaning up all this!

I only have a couple of remarks:

  • There's a lone import statement at the bottom of databases.py. Is there any reason why it's here and not at the top of this file with the others?
  • Same file, the first two methods do not have an INPUT block, while they do have some input arguments.

Otherwise, tests pass and documentation builds, everything seems to be properly deprecated, I agree with your changes.

BTW, it was not merging with the latest beta, fixed this and pushed my change.

Best,

David


New commits:

079e51fFixed merge conflict

comment:8 Changed 3 years ago by jsrn

  • Branch changed from u/dlucas/21165_linear_code_cleanup to u/jsrn/21165_linear_code_cleanup

comment:9 Changed 3 years ago by git

  • Commit changed from 079e51ffbf79e6b3780e27341dc201a0b16d5ca7 to 62ddc6cdc2ad8ffd81d06597de3af1870af5bc01

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

65cb84cNo global imports in databases.py
62ddc6cSimplify some code. Fix function names in doctests

comment:10 in reply to: ↑ 7 Changed 3 years ago by jsrn

  • Dependencies #20835 deleted
  • Milestone changed from sage-7.3 to sage-7.4

Replying to dlucas:

  • There's a lone import statement at the bottom of databases.py. Is there any reason why it's here and not at the top of this file with the others?

It's not there to make the functional available to code in the module, but rather to make the function available on the command line as codes.databases.<tab>.

However, your comment made me realise that all the global imports at the top of databases.py are also present in that tab-completion. So now I've localised all of those.

  • Same file, the first two methods do not have an INPUT block, while they do have some input arguments.

I've added it.

Otherwise, tests pass and documentation builds, everything seems to be properly deprecated, I agree with your changes.

Be aware that many of the tests in databases.py require the spkg gap_packages or internet, and are not run by default with ./sage -t. I had forgotten this myself, but now made them run and fixed some import bugs.

BTW, it was not merging with the latest beta, fixed this and pushed my change.

Thanks, and thanks for reviewing.

Best, Johan

comment:11 Changed 3 years ago by dlucas

  • Branch changed from u/jsrn/21165_linear_code_cleanup to u/dlucas/21165_linear_code_cleanup

comment:12 Changed 3 years ago by dlucas

  • Commit changed from 62ddc6cdc2ad8ffd81d06597de3af1870af5bc01 to 189d8e59744914fb14ff052436f53e59f3cb2927
  • Reviewers set to David Lucas

Hello,

Just found a broken doctest in one of the thematic tutorials (forgot to test that yesterday). It was a trivial deprecation, fixed it myself and pushed the related change. I'm fine with this ticket, so if you're fine with my change, you can set this ticket to positive review.

David


New commits:

189d8e5Fixed broken doctest in thematic tutorial

comment:13 Changed 3 years ago by jsrn

  • Status changed from needs_review to positive_review

Great! Thanks a lot.

comment:14 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long src/sage/categories/homset.py  # 3 doctests failed
sage -t --long src/sage/structure/coerce.pyx  # 1 doctest failed

comment:15 Changed 3 years ago by jsrn

  • Branch changed from u/dlucas/21165_linear_code_cleanup to u/jsrn/21165_linear_code_cleanup

comment:16 Changed 3 years ago by jsrn

  • Commit changed from 189d8e59744914fb14ff052436f53e59f3cb2927 to c283cfc9f73a2d8ae4efd7b1bb86d15b5ae7df24
  • Status changed from needs_work to needs_review

Fixed. Sorry for not testing: I believed the changes to be completely local to sage.coding. The errors occurred because some of the changes means that certain computations are no longer done at Sage start-up: these initialised GF(2) which meant that e.g. the garbage collector always knew of GF(2).

comment:17 Changed 3 years ago by dlucas

  • Status changed from needs_review to positive_review

Bug fixed, doctests pass, I set this ticket to positive_review.

comment:18 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long src/doc/en/thematic_tutorials/coding_theory.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/coding_theory.rst", line 547, in doc.en.thematic_tutorials.coding_theory
Failed example:
    C = codes.RandomLinearCode(10, 5, GF(7))
Expected nothing
Got:
    doctest:warning
      File "/home/buildbot/slave/sage_git/build/src/bin/sage-runtests", line 89, in <module>
        err = DC.run()
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/control.py", line 1130, in run
        self.run_doctests()
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/control.py", line 854, in run_doctests
        self.dispatcher.dispatch()
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1698, in dispatch
        self.parallel_dispatch()
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1588, in parallel_dispatch
        w.start()  # This might take some time
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1864, in start
        super(DocTestWorker, self).start()
      File "/home/buildbot/slave/sage_git/build/local/lib/python/multiprocessing/process.py", line 130, in start
        self._popen = Popen(self)
      File "/home/buildbot/slave/sage_git/build/local/lib/python/multiprocessing/forking.py", line 126, in __init__
        code = process_obj._bootstrap()
      File "/home/buildbot/slave/sage_git/build/local/lib/python/multiprocessing/process.py", line 258, in _bootstrap
        self.run()
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1837, in run
        task(self.options, self.outtmpfile, msgpipe, self.result_queue)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 2130, in __call__
        runner.run(test)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 636, in run
        return self._run(test, compileflags, out)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest doc.en.thematic_tutorials.coding_theory[76]>", line 1, in <module>
        C = codes.RandomLinearCode(Integer(10), Integer(5), GF(Integer(7)))
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/coding/code_constructions.py", line 1052, in RandomLinearCode
        deprecation(21165, "codes.RandomLinearCode(n, k, F) is deprecated. Please use codes.random_linear_code(F, n, k) instead")
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/misc/superseded.py", line 100, in deprecation
        warning(trac_number, message, DeprecationWarning, stacklevel)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/misc/superseded.py", line 141, in warning
        warn(message, warning_class, stacklevel)
    :
    DeprecationWarning: codes.RandomLinearCode(n, k, F) is deprecated. Please use codes.random_linear_code(F, n, k) instead
    See http://trac.sagemath.org/21165 for details.
**********************************************************************
1 item had failures:
   1 of  80 in doc.en.thematic_tutorials.coding_theory
    [79 tests, 1 failure, 0.59 s]

comment:19 Changed 3 years ago by dlucas

  • Branch changed from u/jsrn/21165_linear_code_cleanup to u/dlucas/21165_linear_code_cleanup

comment:20 Changed 3 years ago by dlucas

  • Commit changed from c283cfc9f73a2d8ae4efd7b1bb86d15b5ae7df24 to 08bdda08b38e9552dbe4a77dc9ff57f9962c4938

Hello,

Fixed. Or, to be more specific, I already fixed it. Commit 189d8e5. I pushed my branch once again, hopefully it will work this time.

BTW, I don't know what happened. git trac pull 21165 returned already up-to-date with u/jsrn/21165_linear_code_cleanup. Which explains why I did not catch that failing doctest: it's fixed on my side...

David


New commits:

189d8e5Fixed broken doctest in thematic tutorial
08bdda0Merge branch 'u/jsrn/21165_linear_code_cleanup' of trac.sagemath.org:sage into 21165_linear_code_cleanup
Last edited 3 years ago by dlucas (previous) (diff)

comment:21 Changed 3 years ago by dlucas

  • Status changed from needs_work to needs_review

comment:22 Changed 3 years ago by jsrn

  • Keywords sd75 added
  • Status changed from needs_review to positive_review

Tests pass now.

comment:23 Changed 3 years ago by vbraun

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