Opened 3 years ago
Closed 3 years ago
#22430 closed enhancement (fixed)
better base ring for finite Coxeter matrix groups
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage7.6 
Component:  combinatorics  Keywords:  coxeter 
Cc:  tscrim, stumpc5, darij  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  9bff88b (Commits)  Commit:  9bff88b7a9d12772c3f7c3bccdf6462e91a751bb 
Dependencies:  Stopgaps: 
Description
Let us use by default quadratic number fields or QQ instead of the UniversalCyclotomicField?, when possible, to define the matrix version of finite Coxeter groups.
This should be faster, hopefully.
Change History (10)
comment:1 Changed 3 years ago by
 Branch set to u/chapoton/22430
 Cc tscrim stumpc5 darij added
 Commit set to 2f83ea46b40488e9355b33b05e897133a4e9abdd
 Status changed from new to needs_review
comment:2 Changed 3 years ago by
 Status changed from needs_review to needs_work
some failing doctests
comment:3 Changed 3 years ago by
 Commit changed from 2f83ea46b40488e9355b33b05e897133a4e9abdd to 869a58ce38bb6448378e276b040acbce777135db
Branch pushed to git repo; I updated commit sha1. New commits:
869a58c  trac 22430 more changes for using quadratic fields in finite Coxeter groups

comment:4 Changed 3 years ago by
 Commit changed from 869a58ce38bb6448378e276b040acbce777135db to c8fa602c44c9dccbfccf8dcf7737ab07e9b18fa4
Branch pushed to git repo; I updated commit sha1. New commits:
c8fa602  trac 22430 fixing some doctests

comment:5 Changed 3 years ago by
Bot is green, please review !
comment:6 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:7 Changed 3 years ago by
 Branch changed from u/chapoton/22430 to public/combinat/coxeter_group_base_ring22430
 Commit changed from c8fa602c44c9dccbfccf8dcf7737ab07e9b18fa4 to 9bff88b7a9d12772c3f7c3bccdf6462e91a751bb
 Reviewers set to Travis Scrimshaw
Thank you for doing this. This has been something on my todo list for quite some time. I changed the default base ring to ZZ
for all simplylaced types because it was 3x faster for listing all of the elements of D_{4}. For the bilinear form, I just forced the use of the fraction field. If my changes are good, then positive review.
New commits:
9bff88b  Use ZZ instead of QQ and for all simplylaced types.

comment:8 Changed 3 years ago by
 Status changed from needs_review to positive_review
ok, looks good to me. I ran the tests in groups, categories and combinat. Should be good enough.
comment:9 Changed 3 years ago by
 Keywords coxeter added
comment:10 Changed 3 years ago by
 Branch changed from public/combinat/coxeter_group_base_ring22430 to 9bff88b7a9d12772c3f7c3bccdf6462e91a751bb
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
better base rings in Coxeter matrix groups