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

Various cleanup and deprecation in sage.coding.linear_code #21165

Closed
johanrosenkilde opened this issue Aug 4, 2016 · 33 comments
Closed

Various cleanup and deprecation in sage.coding.linear_code #21165

johanrosenkilde opened this issue Aug 4, 2016 · 33 comments

Comments

@johanrosenkilde
Copy link
Contributor

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.

CC: @sagetrac-dlucas

Component: coding theory

Keywords: cleanup, sd75

Author: Johan Rosenkilde

Branch/Commit: 08bdda0

Reviewer: David Lucas

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

@johanrosenkilde
Copy link
Contributor Author

Branch: u/jsrn/21165_linear_code_cleanup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 4, 2016

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 4, 2016

Commit: b1329da

@johanrosenkilde
Copy link
Contributor Author

comment:3

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.

@johanrosenkilde

This comment has been minimized.

@johanrosenkilde
Copy link
Contributor Author

comment:4

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

@johanrosenkilde
Copy link
Contributor Author

Dependencies: 20835

@johanrosenkilde
Copy link
Contributor Author

Changed dependencies from 20835 to #20835

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 17, 2016

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 17, 2016

Changed commit from b1329da to 079e51f

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 17, 2016

comment:7

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

@johanrosenkilde
Copy link
Contributor Author

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2016

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 18, 2016

Changed commit from 079e51f to 62ddc6c

@johanrosenkilde
Copy link
Contributor Author

Changed dependencies from #20835 to none

@johanrosenkilde
Copy link
Contributor Author

comment:10

Replying to @sagetrac-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

@johanrosenkilde johanrosenkilde modified the milestones: sage-7.3, sage-7.4 Aug 18, 2016
@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 18, 2016

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 18, 2016

Changed commit from 62ddc6c to 189d8e5

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 18, 2016

comment:12

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

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 18, 2016

Reviewer: David Lucas

@johanrosenkilde
Copy link
Contributor Author

comment:13

Great! Thanks a lot.

@vbraun
Copy link
Member

vbraun commented Aug 20, 2016

comment:14
sage -t --long src/sage/categories/homset.py  # 3 doctests failed
sage -t --long src/sage/structure/coerce.pyx  # 1 doctest failed

@johanrosenkilde
Copy link
Contributor Author

@johanrosenkilde
Copy link
Contributor Author

comment:16

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

@johanrosenkilde
Copy link
Contributor Author

Changed commit from 189d8e5 to c283cfc

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 21, 2016

comment:17

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

@vbraun
Copy link
Member

vbraun commented Aug 23, 2016

comment:18
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]

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 24, 2016

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 24, 2016

comment:20

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

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Aug 24, 2016

Changed commit from c283cfc to 08bdda0

@johanrosenkilde
Copy link
Contributor Author

comment:22

Tests pass now.

@johanrosenkilde
Copy link
Contributor Author

Changed keywords from cleanup to cleanup, sd75

@vbraun
Copy link
Member

vbraun commented Aug 27, 2016

Changed branch from u/dlucas/21165_linear_code_cleanup to 08bdda0

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