Opened 12 years ago
Closed 8 years ago
#9290 closed enhancement (fixed)
Implement Coxeter groups in their geometric representation
Reported by: | nthiery | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-5.13 |
Component: | combinatorics | Keywords: | coxeter |
Cc: | sage-combinat | Merged in: | sage-5.13.beta2 |
Authors: | Travis Scrimshaw | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #15204 | Stopgaps: |
Description (last modified by )
The root system / coxeter group code is designed from the ground up to allow for this extension.
Steps:
- Double check
CartanType(["H",3]).coxeter_diagram()
and friends
- Given a coxeter diagram, construct the dynkin diagram
g
corresponding to the geometric representation; most of the time, this will involve roots of unity, and require e.g. a cyclotomic field (see also #8327)
- Make sure that
L = RootSystem(g).root_space()
accepts such a diagram
- Make sure that
WeylGroup(L)
accepts such a root space
- Fix all the interfaces to properly reflect the generalization (e.g. WeylGroup? above should really be CoxeterGroup?).
Apply:
Attachments (2)
Change History (21)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
- Description modified (diff)
comment:3 Changed 9 years ago by
- Keywords coxeter added
comment:4 Changed 8 years ago by
see also the code in the ticket #14816
comment:5 Changed 8 years ago by
- Status changed from new to needs_review
Here's a patch which should take care of all except the second point since I thought Dynkin diagrams where separate from Coxeter diagrams except when there are 1,2,3, or 4 bonds (and even then, we're ignoring the arrows).
Base patch is from #15137, but now uses the UCF as the default field.
comment:6 Changed 8 years ago by
- Milestone changed from sage-wishlist to sage-5.12
comment:7 Changed 8 years ago by
- Status changed from needs_review to needs_work
there are some failing doctests
comment:8 Changed 8 years ago by
- Status changed from needs_work to needs_review
Yep, something is up with the patchbot, so I gave it a kick.
sage -t ../combinat/root_system/cartan_matrix.py [97 tests, 15.77 s] sage -t matrix_gps/coxeter_group.py [72 tests, 0.36 s] sage -t ../combinat/root_system/coxeter_group.py [28 tests, 18.65 s] ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 18.8 seconds cpu time: 26.2 seconds cumulative wall time: 34.8 seconds
comment:9 Changed 8 years ago by
one doctest failing
File "/home/chapoton/sage-5.12.beta5/devel/sage/sage/groups/matrix_gps/coxeter_group.py", line 278, in sage.groups.matrix_gps.coxeter_group.CoxeterMatrixGroup.__init__ Failed example: TestSuite(W).run(max_runs=30) # long time Expected nothing Got: Failure in _test_matrix_generators: Traceback (most recent call last): File "/home/chapoton/sage-5.12.beta5/local/lib/python2.7/site-packages/sage/misc/sage_unittest.py", line 282, in run test_method(tester = tester) File "/home/chapoton/sage-5.12.beta5/local/lib/python2.7/site-packages/sage/groups/matrix_gps/finitely_generated.py", line 376, in _test_matrix_generators tester.assertEqual(g.matrix(), h.matrix()) File "cachefunc.pyx", line 1774, in sage.misc.cachefunc.CachedMethodCallerNoArgs.__call__ (sage/misc/cachefunc.c:9546) File "/home/chapoton/sage-5.12.beta5/local/lib/python2.7/site-packages/sage/groups/matrix_gps/group_element.py", line 447, in matrix m = g.matrix(self.base_ring()) File "element.pyx", line 1076, in sage.libs.gap.element.GapElement.matrix (sage/libs/gap/element.c:8618) File "element.pyx", line 1473, in sage.libs.gap.element.GapElement_Cyclotomic.sage (sage/libs/gap/element.c:10511) File "parent.pyx", line 761, in sage.structure.parent.Parent.__getattr__ (sage/structure/parent.c:6823) File "misc.pyx", line 251, in sage.structure.misc.getattr_from_other_class (sage/structure/misc.c:1606) AttributeError: 'UniversalCyclotomicField_with_category' object has no attribute '_n' ------------------------------------------------------------ The following tests failed: _test_matrix_generators
comment:10 Changed 8 years ago by
- Dependencies set to #15204
The problem was the conversion from gap's cyclotomics to sage by using the UCF. This is fixed in #15204.
sage: W = CoxeterGroup([[1,3,2],[3,1,6],[2,6,1]]) sage: W._test_matrix_generators()
comment:11 follow-up: ↓ 12 Changed 8 years ago by
Hello, I have a few comments and questions
"assert implementation" : I think this use of assert to check input is not encouraged
"lazy_import('sage.groups.raag', 'RightAngledArtinGroup?')" : has this something to do in this ticket ?
What are the changes "sage/groups/matrix_gps/finitely_generated.py" for ?
comment:12 in reply to: ↑ 11 Changed 8 years ago by
Hey Frederic,
Replying to chapoton:
"assert implementation" : I think this use of assert to check input is not encouraged
This was a hold-over from the previous implementation, but this is definitely a good time to get rid of it. Fixed.
"lazy_import('sage.groups.raag', 'RightAngledArtinGroup?')" : has this something to do in this ticket ?
Because I didn't split it cleanly with #15137. Fixed.
What are the changes "sage/groups/matrix_gps/finitely_generated.py" for ?
I need to pass the CoxeterGroups
category up during the initialization, so I had to make those changes.
Thanks for catching that,
Travis
comment:13 Changed 8 years ago by
Well, it seems that the patch triggers the import of numpy at startup. I have not tried to investigate where this happens.
comment:14 Changed 8 years ago by
- Status changed from needs_review to needs_work
Changed 8 years ago by
comment:15 Changed 8 years ago by
- Status changed from needs_work to needs_review
I made CoxeterMatrixGroup
lazy import in to fix the numpy startup import.
Changed 8 years ago by
comment:16 Changed 8 years ago by
Hello Travis,
I have made a cosmetic review patch, that you can fold into yours if you want.
This almost looks good to go, but I was a bit disappointed when I tried:
sage: K = NumberField(x**2+5,'t') sage: CoxeterGroup(['H',3],base_ring=K)
and it failed. If there is a way to make that work, it would be great !
comment:17 Changed 8 years ago by
ok, I can see no easy way to do that. Let us forget this for the moment.
Good to go ?
comment:18 Changed 8 years ago by
- Description modified (diff)
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
Hey Frederic,
I can't see an easy way to do so either. There might be a solution, but it'll probably be either complicated or cumbersome.
Thanks for doing the review,
Travis
comment:19 Changed 8 years ago by
- Merged in set to sage-5.13.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
Partially depends on #8237