-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
Implemented constructions for Kasami codes #30085
Comments
comment:2
OK, could you add checks on things like minimal distances of these codes? |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
|
Dependencies: #21226 |
comment:7
I don't know these codes, but I a priori think it would be better to follow the general convention for codes that they be classes. The idea is that if one has special knowledge on such codes (e.g. know either their minimum distance, their weight enumerator, etc.) then a sub-class of AbstractLinearCode would be able to override those methods with specialised, fast ones. Converting your functions to classes requires a bit more boilerplate, but you can follow the thematic tutorial on this. |
comment:8
If you insist on keeping these as functions, consider putting the functions in the |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Reviewer: Dima Pasechnik |
comment:11
Johan, are you happy with the design now? (tests pass, by the way) |
comment:12
I think it is not a good idea to put all the interesting doctests in underscore functions. Actually, I do not see why you need those helper functions. Couldn't you put directly all your code in the method |
comment:13
Moreover, on patchbot's report:
I think it is more reasonable to remove
and
|
comment:15
Structural comments:
Code comments: There are many lines exceeding 80 chars. Is this not still a convention of
The main disadvantage is that this currently emits a warning, because |
comment:16
Replying to @johanrosenkilde:
Of course, it would be very nice to close #24279 and subsequent tickets but I would let this for another time since I'm not sure it will be an easy job. |
comment:17
long lines in the code (apart from output of doctests) should not be too long - officially, 80 chars, make it 90 if really needed, but no more. |
comment:18
@xcaruso: Sure, it's a valid point to not wish to defer progress here by first fixing up stuff elsewhere. I definitely wouldn't wait for #28485, since #24279 is a much easier fix using stuff already shipped in Sage. If not waiting for either tickets is preferred, I see two good options: 1) Use my 2-line implementation above and accept the warning for now. In #24279, we remove this warning. 2) Expand my 2-line implementation to a 5-line one which basically implements the meat of |
comment:19
Here is an implementation avoiding
|
comment:20
Thanks for the feedback, I'm now working on fixing all point raised. I referenced the Kasami's papers which are quoted by Brouwer and Wikipedia, but I don't see how these relates to these codes. Brouwer does not provide a proof for the minimum distances stated and I believe they are wrong.
I have no idea if the weight distribution of these codes is known. As far as the generator matrix is concerned, I'm happy that there is a quicker way to compute it. |
comment:22
The doctoring doesn't compile. I get an error
I really don't see what's wrong as it matches the sphinx guidelines (https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html). Any help is highly appreciated. |
comment:23
I'd check on I'm building your branch now, will see later myself. |
comment:25
Another point: you should document the function |
this fixed docbuild errors |
comment:27
Attachment: typeset.diff.gz I've attached a diff which makes the docs build. (probably far from minimal, but OK) |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:29
Sorry, I was meaning the Btw, I'm not quite sure but it's maybe better to implement |
comment:30
OK, I just realized that this method was added in #21226... |
comment:31
#21226 has been fixed, so this looks good to go, too. |
Changed branch from u/gh-Ivo-Maffei/kasami_codes to |
We added methods to generate the extended Kasami codes and the Kasami codes.
For a definition of those codes see the book "Distance-Regular Graphs" by Brouwer et. al.
Depends on #21226
CC: @dimpase @johanrosenkilde @xcaruso
Component: coding theory
Keywords: linear_codes
Author: Ivo Maffei
Branch/Commit:
b94cc3b
Reviewer: Dima Pasechnik
Issue created by migration from https://trac.sagemath.org/ticket/30085
The text was updated successfully, but these errors were encountered: