Opened 5 years ago
Closed 5 years ago
#19416 closed enhancement (fixed)
Implement some generic category methods
Reported by: | tscrim | Owned by: | tscrim |
---|---|---|---|
Priority: | major | Milestone: | sage-6.10 |
Component: | categories | Keywords: | |
Cc: | jhpalmieri | Merged in: | |
Authors: | Travis Scrimshaw | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | ff914dc (Commits) | Commit: | ff914dc988541148b4f10a3434dd688fc5ff7e75 |
Dependencies: | Stopgaps: |
Description
We implement a generic is_commutative
test for finite-dimensional algebras with a basis and a cardinality
for modules with a basis.
Change History (6)
comment:1 Changed 5 years ago by
- Branch set to public/categories/generic_methods-19416
- Commit set to f9b5da0e38fc793122ac2615df86efa3a590f3ad
- Status changed from new to needs_review
comment:2 follow-up: ↓ 4 Changed 5 years ago by
I would prefer if the docstring for is_commutative
said "Return whether self is a commutative algebra" (or "Return whether this algebra is a commutative algebra").
Is it worth saving any time by removing self.one()
from the basis before doing the iteration? With the following change:
-
src/sage/categories/finite_dimensional_algebras_with_basis.py
diff --git a/src/sage/categories/finite_dimensional_algebras_with_basis.py b/src/sage/categories/finite_dimensional_algebras_with_basis.py index da1a870..8934e6c 100644
a b class FiniteDimensionalAlgebrasWithBasis(CategoryWithAxiom_over_base_ring): 951 951 @cached_method 952 952 def is_commutative(self): 953 953 """ 954 Return if``self`` is a commutative algebra.954 Return whether ``self`` is a commutative algebra. 955 955 956 956 EXAMPLES:: 957 957 … … class FiniteDimensionalAlgebrasWithBasis(CategoryWithAxiom_over_base_ring): 963 963 True 964 964 """ 965 965 B = list(self.basis()) 966 B.remove(self.one()) 966 967 return all(b*bp == bp*b for i,b in enumerate(B) for bp in B[i+1:]) 967 968 968 969 class ElementMethods:
it appears to be faster to check commutativity for SymmetricGroupAlgebra(5, QQ)
. (I removed the @cached_method
line before testing.) It's even a little faster for commutative algebras: the time spent removing the element from the list is made up by the time saved by not having as many iterations.
comment:3 Changed 5 years ago by
- Commit changed from f9b5da0e38fc793122ac2615df86efa3a590f3ad to ff914dc988541148b4f10a3434dd688fc5ff7e75
comment:4 in reply to: ↑ 2 Changed 5 years ago by
Replying to jhpalmieri:
I would prefer if the docstring for
is_commutative
said "Return whether self is a commutative algebra" (or "Return whether this algebra is a commutative algebra").
Done.
Is it worth saving any time by removing
self.one()
from the basis before doing the iteration? With the following change: ... it appears to be faster to check commutativity forSymmetricGroupAlgebra(5, QQ)
. (I removed the@cached_method
line before testing.) It's even a little faster for commutative algebras: the time spent removing the element from the list is made up by the time saved by not having as many iterations.
The only possible issue I see is that not all algebras in Sage have one
as a basis element, so for those algebras, by adding this extra remove
check, it might be longer. However, since the vast majority of algebras in Sage do have one
as a basis element, and probably it appears early on, this should be faster and I think it is worthwhile to have it.
comment:5 Changed 5 years ago by
- Reviewers set to John Palmieri
- Status changed from needs_review to positive_review
Looks good, thanks.
comment:6 Changed 5 years ago by
- Branch changed from public/categories/generic_methods-19416 to ff914dc988541148b4f10a3434dd688fc5ff7e75
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Implement generic cardinality() and is_commutative().