Opened 6 years ago
Closed 6 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, GitHub, GitLab) | Commit: | 08bdda08b38e9552dbe4a77dc9ff57f9962c4938 |
Dependencies: | Stopgaps: |
Description (last modified by )
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
andself_orthogonal_binary_codes
to new modulesage.coding.databases
, also deprecating them from the global namespace. Also improved their documentation. Added the new module tocodes
. - Rename the module
sd_codes
toself_dual_codes
. - Remove the function
sd_codes.self_dual_codes_binary
from the global namespace, renaming it intoself_dual_binary_codes
accessible fromcodes.databases
. Removed all helper computations that were done at Sage startup fromsd_codes
, making them all private and evaluated only when callingself_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 aszeta_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 madeLinearCode
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 ofspectrum
, but support the second as an alias. Inline the helper function_spectrum_from_gap
. Improved the doc. - Renamed
code_constructions.LinearCodeFromCheckMatrix
tocode_constructions.from_parity_check_matrix
. Simplified its doc. - Changed a random doctest in
punctured_code
whose output changed after an unrelated patch (due toRandomLinearCode
) - Simplified
RandomLinearCode
, renamed tocodes.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 6 years ago by
- Branch set to u/jsrn/21165_linear_code_cleanup
comment:2 Changed 6 years ago by
- Commit set to b1329da78e45943d3e80cfc83b19cd93728cb592
comment:3 Changed 6 years ago by
- 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 6 years ago by
- 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 6 years ago by
- Dependencies changed from 20835 to #20835
comment:6 Changed 6 years ago by
- Branch changed from u/jsrn/21165_linear_code_cleanup to u/dlucas/21165_linear_code_cleanup
comment:7 follow-up: ↓ 10 Changed 6 years ago by
- 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:
079e51f | Fixed merge conflict
|
comment:8 Changed 6 years ago by
- Branch changed from u/dlucas/21165_linear_code_cleanup to u/jsrn/21165_linear_code_cleanup
comment:9 Changed 6 years ago by
- Commit changed from 079e51ffbf79e6b3780e27341dc201a0b16d5ca7 to 62ddc6cdc2ad8ffd81d06597de3af1870af5bc01
comment:10 in reply to: ↑ 7 Changed 6 years ago by
- 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 6 years ago by
- Branch changed from u/jsrn/21165_linear_code_cleanup to u/dlucas/21165_linear_code_cleanup
comment:12 Changed 6 years ago by
- 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:
189d8e5 | Fixed broken doctest in thematic tutorial
|
comment:13 Changed 6 years ago by
- Status changed from needs_review to positive_review
Great! Thanks a lot.
comment:14 Changed 6 years ago by
- 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 6 years ago by
- Branch changed from u/dlucas/21165_linear_code_cleanup to u/jsrn/21165_linear_code_cleanup
comment:16 Changed 6 years ago by
- 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 6 years ago by
- Status changed from needs_review to positive_review
Bug fixed, doctests pass, I set this ticket to positive_review
.
comment:18 Changed 6 years ago by
- 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 6 years ago by
- Branch changed from u/jsrn/21165_linear_code_cleanup to u/dlucas/21165_linear_code_cleanup
comment:20 Changed 6 years ago by
- 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:
189d8e5 | Fixed broken doctest in thematic tutorial
|
08bdda0 | Merge branch 'u/jsrn/21165_linear_code_cleanup' of trac.sagemath.org:sage into 21165_linear_code_cleanup
|
comment:21 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:22 Changed 6 years ago by
- Keywords sd75 added
- Status changed from needs_review to positive_review
Tests pass now.
comment:23 Changed 6 years ago by
- Branch changed from u/dlucas/21165_linear_code_cleanup to 08bdda08b38e9552dbe4a77dc9ff57f9962c4938
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Rename LinearCodeFromCheckMatrix -> from_parity_check_matrix
Fix typo
Fix import and doc of from_parity_check_matrix
Reduce brittle and random doctest to something fixed.
RandomLinearCode(n, k, F) -> random_linear_code(F, n, k)
Removed TrivialCode. It makes no mathematical sense and hasn't worked for a long while, so no deprecation.
Clarify and massively simplify module doc of code_constructions.py
Some doctest markup fixes
Privatize and/or deprecate the helper functions of code_constructions.py