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: sage-7.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 chapoton

  • Branch set to u/chapoton/22430
  • Cc tscrim stumpc5 darij added
  • Commit set to 2f83ea46b40488e9355b33b05e897133a4e9abdd
  • Status changed from new to needs_review

New commits:

2f83ea4better base rings in Coxeter matrix groups

comment:2 Changed 3 years ago by chapoton

  • Status changed from needs_review to needs_work

some failing doctests

comment:3 Changed 3 years ago by git

  • Commit changed from 2f83ea46b40488e9355b33b05e897133a4e9abdd to 869a58ce38bb6448378e276b040acbce777135db

Branch pushed to git repo; I updated commit sha1. New commits:

869a58ctrac 22430 more changes for using quadratic fields in finite Coxeter groups

comment:4 Changed 3 years ago by git

  • Commit changed from 869a58ce38bb6448378e276b040acbce777135db to c8fa602c44c9dccbfccf8dcf7737ab07e9b18fa4

Branch pushed to git repo; I updated commit sha1. New commits:

c8fa602trac 22430 fixing some doctests

comment:5 Changed 3 years ago by chapoton

Bot is green, please review !

comment:6 Changed 3 years ago by chapoton

  • Status changed from needs_work to needs_review

comment:7 Changed 3 years ago by tscrim

  • Branch changed from u/chapoton/22430 to public/combinat/coxeter_group_base_ring-22430
  • 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 simply-laced types because it was 3x faster for listing all of the elements of D4. For the bilinear form, I just forced the use of the fraction field. If my changes are good, then positive review.


New commits:

9bff88bUse ZZ instead of QQ and for all simply-laced types.

comment:8 Changed 3 years ago by chapoton

  • 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 chapoton

  • Keywords coxeter added

comment:10 Changed 3 years ago by vbraun

  • Branch changed from public/combinat/coxeter_group_base_ring-22430 to 9bff88b7a9d12772c3f7c3bccdf6462e91a751bb
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.