Opened 6 years ago
Closed 6 years ago
#15029 closed enhancement (fixed)
Implement similarity classes over principal ideal local rings of length two
Reported by: | amri | Owned by: | amri |
---|---|---|---|
Priority: | major | Milestone: | sage-5.13 |
Component: | combinatorics | Keywords: | similarity classs, matrices, local rings |
Cc: | sage-combinat, tscrim | Merged in: | sage-5.13.beta0 |
Authors: | Amritanshu Prasad | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #14907 | Stopgaps: |
Description (last modified by )
In the paper Similarity of matrices over local rings of length two http://arxiv.org/abs/1212.6157, the similarity classes of n x n matrices with entries in a principal ideal local ring of length two are computed for n = 2, 3, 4. These calculations are being implemented in sage.
Apply:
Attachments (3)
Change History (13)
Changed 6 years ago by
comment:1 Changed 6 years ago by
- Cc sage-combinat tscrim added
- Dependencies set to #14907
- Reviewers set to tscrim
- Status changed from new to needs_review
comment:2 Changed 6 years ago by
comment:3 Changed 6 years ago by
- Reviewers changed from tscrim to Travis Scrimshaw
- Status changed from needs_review to needs_work
Changed 6 years ago by
Added functions for similarity over principal ideal local rings of length two
comment:4 Changed 6 years ago by
As suggested, similarity classes for principal ideal local rings of length two is now implemented as functions in the module similarity_class_type.py
Sorry for all the whitespace changes; please ignore them.
Apply: trac_15029-additions-similarity_class_type-ap.patch
comment:5 Changed 6 years ago by
- Status changed from needs_work to needs_review
Changed 6 years ago by
comment:6 Changed 6 years ago by
- Description modified (diff)
Hey Amri,
Here's a review patch which does a few things:
- Changes the name of the functions
*2
to*_length_two
to be a little more explicit. - Adds
INPUT:
blocks to some of the functions. - Fixes some docstring mistakes I didn't catch the first time around in the whole file. I also converted it to use Sage's macro for finite fields
\GF{q}
.
If you're happy with my changes, go ahead and set this to positive review.
Thanks for your work on this,
Travis
comment:7 Changed 6 years ago by
For patchbot:
Apply: trac_15029-additions-similarity_class_type-ap.patch, trac_15029-review-ts.patch
comment:8 Changed 6 years ago by
- Status changed from needs_review to positive_review
Thanks Travis, for the considerable work. It looks great now.
Amri.
comment:9 Changed 6 years ago by
- Milestone changed from sage-5.12 to sage-5.13
comment:10 Changed 6 years ago by
- Merged in set to sage-5.13.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
Hey Amri,
It seems like you've created 2 classes which are basically computing a few methods and don't contain helper methods. I would instead move these to (module-level) functions in
similarity_class_type.py
which take thedata, q, selftranspose, invertible
as arguments since there is:Two other minor points:
:arxiv.org:`1212.6157`
?[mq]:...
line from the patch header by doing aqrefresh -e
?Thanks,
Travis