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:  sage6.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 finitedimensional 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_methods19416
 Commit set to f9b5da0e38fc793122ac2615df86efa3a590f3ad
 Status changed from new to needs_review
comment:2 followup: ↓ 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_methods19416 to ff914dc988541148b4f10a3434dd688fc5ff7e75
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Implement generic cardinality() and is_commutative().