Opened 9 years ago
Closed 9 years ago
#13694 closed enhancement (fixed)
Implement __getitem__ for LinearCode
Reported by: | ppurka | Owned by: | wdj |
---|---|---|---|
Priority: | major | Milestone: | sage-5.7 |
Component: | coding theory | Keywords: | |
Cc: | dimpase | Merged in: | sage-5.7.beta3 |
Authors: | Punarbasu Purkayastha | Reviewers: | Dmitrii Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12014, #13417 | Stopgaps: |
Description (last modified by )
The current implementation of __getitem__
for a LinearCode
element tries to generate the whole list of codewords and then returns the element needed. This makes it impossible to work with this since for linear codes of large dimensions, the generation of all the codewords is not only unnecessary, but it also eats up several gigabytes of memory.
The attached patch implements __getitem__
for a linear code.
Apply
- #13417
- #12014
- trac_13694-implement_getitem.patch to
devel/sage
Attachments (1)
Change History (12)
comment:1 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
- Status changed from needs_review to needs_work
comment:3 Changed 9 years ago by
I did the change, and then I remembered that it will never come up in the documentation! Anyway, if someone decides to call __getitem__?
then they will see it.
comment:4 Changed 9 years ago by
- Status changed from needs_work to needs_review
comment:5 Changed 9 years ago by
- Status changed from needs_review to needs_work
with your fix, one gets fake elements, i.e. a stopping condition is missing:
sage: C Linear code of length 7, dimension 4 over Finite Field of size 2 sage: C[7] (1, 1, 1, 0, 0, 0, 0) sage: C[77] (1, 0, 1, 1, 0, 1, 0) sage: C[777] (1, 0, 0, 1, 1, 0, 0) sage: C[7777] (1, 0, 0, 0, 0, 1, 1)
comment:6 follow-up: ↓ 7 Changed 9 years ago by
- Status changed from needs_work to needs_review
Nice catch! I hadn't thought of this scenario.
comment:7 in reply to: ↑ 6 Changed 9 years ago by
Replying to ppurka:
Nice catch! I hadn't thought of this scenario.
Shouldn't it throw IndexError
? At least this is the old behaviour.
comment:8 Changed 9 years ago by
Yes, it should be IndexError
(didn't know about this exception). I have updated the patch.
comment:9 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:10 Changed 9 years ago by
- Reviewers set to Dmitrii Pasechnik
comment:11 Changed 9 years ago by
- Merged in set to sage-5.7.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
I think the the long comment in the code body should be basically put into docstrings, so that it also seen in the documentation. This would also make it clearer in which order the words are coming. I'd give it positive review then.
It's a neat trick, to use p-adics!