Opened 12 years ago

Closed 7 years ago

# Implementation of the SubwordComplex as defined by Knutson and Miller

Reported by: Owned by: Christian Stump tbd major sage-7.0 combinatorics subword complex, simplicial complex Christian Stump Frédéric Chapoton N/A 17518c1 17518c11f06b77c1823a5b0950285085540a41b2

### Description (last modified by Christian Stump)

This patch provides an implementation of the subword complex:

Fix a Coxeter system (W,S). Let Q = be a finite word in S and pi in W.

The subword complex Delta(Q,pi) is then defined to be the simplicial complex with vertices being {0,...,n-1}, (n = len(Q), one vertex for each letter in Q) and with facets given by all (indices of) subwords Q' of Q for which Q\Q' is a reduced expression for pi.

```    sage: W = CoxeterGroup(['A',2],index_set=[1,2])
sage: w = W.from_reduced_word([1,2,1])
sage: C = SubwordComplex([2,1,2,1,2],w); C
Subword complex of type ['A', 2] for Q = [2, 1, 2, 1, 2] and pi = 121
sage: C.facets()
{(1, 2), (3, 4), (0, 4), (2, 3), (0, 1)}
```

### comment:1 Changed 12 years ago by Christian Stump

Authors: → Christian Stump PLEASE CHANGE → combinatorics modified (diff) subword complex simplicial complex added new → needs_work PLEASE CHANGE → enhancement

### comment:2 Changed 11 years ago by Christian Stump

Dependencies: → 11187, 11122 modified (diff)

### comment:3 Changed 11 years ago by Christian Stump

Dependencies: 11187, 11122 → #11187, #11122

### comment:4 Changed 11 years ago by Christian Stump

Milestone: → sage-4.7.2

### comment:5 Changed 11 years ago by Christian Stump

Dependencies: #11187, #11122 → #11122

### comment:6 follow-up:  7 Changed 10 years ago by Frédéric Chapoton

Dependencies: #11122 → #12774 needs_work → needs_review

### comment:7 in reply to:  6 Changed 10 years ago by Christian Stump

Dependencies: #12774 → #11187, #12774

Okay, I must confess that I tend to not finish implementations once the patch does what I want it to do...

The dependencies of this patch must (unfortunately) inherit as well the dependencies of #11122. This is to say that this patch actually relies on many things implemented in #11187.

I would be very, very glad if anyone would finally start reviewing the universal cyclotomic field, #8327 on which #11187 depends (I would first need to rebase it, and make sure that the trac version is updated to the newest version on the combinat queue).

Cheers, Christian

### comment:8 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11 → sage-5.12

### comment:9 Changed 9 years ago by For batch modifications

Milestone: sage-6.1 → sage-6.2

### comment:10 Changed 9 years ago by Frédéric Chapoton

Branch: → u/chapoton/11010 → a525fdcf47ead6e1794c4f54acac00ee70f07e91

I have made a git branch. It does not work: runs into an infinite recursion because of #15456.

New commits:

 ​3b8df81 `trac_11010-subword_complex-cs.patch` ​a525fdc `trac #11010 code clean-up`

### comment:11 Changed 9 years ago by Frédéric Chapoton

Status: needs_review → needs_work → coverage

The code needs to be 100% doctested. Some empty methods should be removed.

### comment:12 Changed 9 years ago by Frédéric Chapoton

Dependencies: #11187, #12774 → #11187, #12774, #15456

Hello Christian, do you really think that this ticket depends on #11187 ?

I am tempted to ressurect this one, without waiting for the famous category-and-axiom ticket.

With not so much work, I managed to get a branch with only 19 failures.

### comment:13 Changed 9 years ago by git

Commit: a525fdcf47ead6e1794c4f54acac00ee70f07e91 → 76afdca9d2842a4f0a4060f60130bb37c41df99f

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

 ​4f3945d `trac_11010-subword_complex-cs.patch` ​e77f24d `trac #11010 code clean-up` ​0599a0a `trac #11010 has_descent is better` ​76afdca `trac #11010 more corrections, and rebased`

### comment:14 Changed 9 years ago by Frédéric Chapoton

Dependencies: #11187, #12774, #15456 → #12774

Here is a better branch.

I am removing the dependencies to #11187, #15456 as I think they are not really needed.

There are still 16 failing doctests. Work in progress.

### comment:15 Changed 9 years ago by Christian Stump

Dear Frederic,

Thanks for your work on that -- I am super busy with other stuff, hopefully this will get better in about 4 weeks from now.

I am not sure if I/we should keep the construction for positive genus:

• Mathematically, this has not been considered before,
• it is not quite the right generalization of subword complexes as defined by Knutson-Miller. In particular, such complexes do not need to be vertex-decomposable or shellable.
• we have a proper generalization which will have similar combinatorics and will capture the needed behaviour for positive genus
• our paper in which I introduce these things is still on hold and we will need at least another 3-6 months before getting this into a state to make it publicly available

I should be able to quickly look at your branch during the weekend to clean things up a bit. Or do you prefer to work things out yourself?

Cheers, Christian

### comment:16 Changed 9 years ago by git

Commit: 76afdca9d2842a4f0a4060f60130bb37c41df99f → 88643156a58720df968a0cb95e38f94668871a1a

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

 ​8864315 `trac #11010 one more doctest passing`

### comment:17 follow-up:  18 Changed 9 years ago by Frédéric Chapoton

Hello Christian,

I would be happy to see you come back here. The code is not very easy to understand by me alone.

The remaining issues (15 doctests) seem to be related to a strange use of roots, where Weyl group elements seem to be supposed to act on the indices of roots in the list of roots. And they do not, of course.

I will not do anything more on the ticket until next week.

### comment:18 in reply to:  17 Changed 9 years ago by Christian Stump

Hi,

I would be happy to see you come back here. The code is not very easy to understand by me alone.

I spend some time today trying to get Sage working properly again, but I failed so far. I guess I need someone helping me to work properly with git...

I don't know how to resolve that now (and I also don't know if I tried to upgrade to the newest development version or the latest stable version).

The remaining issues (15 doctests) seem to be related to a strange use of roots, where Weyl group elements seem to be supposed to act on the indices of roots in the list of roots. And they do not, of course.

I use the CHEVIE implementation of finite reflection groups which are implemented as permutation groups on indices of roots. This speed things drastically, but it is not reall mathematically clean. Moreover, I just looked at the code, and

• I also pushed many non-features that are there only for my research (and for which I cannot prove what I want to prove)
• Many features only make sense for finite Coxeter groups, so such checks are still to be implemented.

### comment:19 Changed 9 years ago by Christian Stump

Hello Frederic,

I now spent some time playing with the patch.

My main concern is: I can re-implement things to work with any implementation of Coxeter groups (i.e., with the current implementations of Weyl groups and Coxeter groups). BUT: this will slow down things drastically!

So we either leave this ticket open and wait for my CHEVIE version of Coxeter groups, #11187 (I am not too positive that these will make it into Sage this year though), or we get rid of the speedy implementations and have a slow version to play in very low ranks that can be used soon.

Cheers, Christian

### comment:20 Changed 9 years ago by Christian Stump

Branch: u/chapoton/11010 → u/stumpc5/ticket/11010 Mar 17, 2014, 9:12:06 AM → Mar 17, 2014, 9:12:06 AM

### comment:21 Changed 9 years ago by For batch modifications

Milestone: sage-6.2 → sage-6.3

### comment:22 Changed 8 years ago by For batch modifications

Milestone: sage-6.3 → sage-6.4

### comment:23 Changed 7 years ago by git

Commit: 88643156a58720df968a0cb95e38f94668871a1a → ab914f50d616ad4df0fbd5a8c587d1843cd0a58e

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

 ​014fec2 `got the doctests of complex reflection group category pass` ​d3198f9 `fixing a few bugs on coxeter number and fundamental invariants` ​e2736f6 `added another doctest` ​8db1dee `first version of adding functionality to coxeter group categories` ​425ca38 `first version of coxeter groups from chevie with all tests passing!` ​c0a036b `Avoid creation of _reduced_word during iteration` ​405b26c `turned well-generated into an axiom` ​0639665 `fixed a but with Word(reduced_word) we worked on yesterday` ​28f8abd `added a test for well generated` ​ab914f5 `first version of the subword complexes as of 2015`

### comment:24 Changed 7 years ago by Vincent Pilaud

Branch: u/stumpc5/ticket/11010 → u/vpilaud/ticket/11010

### comment:25 Changed 7 years ago by git

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

 ​bd4f2ad `improve the subword complex, first version of plot in type B`

### comment:26 Changed 7 years ago by Christian Stump

Vincent, please remove the `c` and the `html` file again from the branch since they are not supposed to be going into sage.

### comment:27 Changed 7 years ago by git

Commit: bd4f2ada52cbe683bd8c768e82f729a24b73e8aa → f165d54668bc6342df43c2c31877195aa2f7765d

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

 ​f165d54 `pseudoline representation of facets of type B subword complexes, more documentation`

### comment:28 Changed 7 years ago by Frédéric Chapoton

Milestone: sage-6.4 → sage-6.8

does not apply, needs to be rebased

### comment:29 Changed 7 years ago by Frédéric Chapoton

Dependencies: #12774 → #12774, #11187

### comment:30 Changed 7 years ago by Frédéric Chapoton

Branch: u/vpilaud/ticket/11010 → u/chapoton/11010 f165d54668bc6342df43c2c31877195aa2f7765d → 0aa6989fc49f62d72c8d6452f4bb46b4533518d7

I have made a new branch, by disentangling u/vpilaud/ticket/11010 from #11187 (but sadly still depending on #11187).

It seems that all my previous work on this ticket was lost. I do not appreciate.

New commits:

 ​bbff606 `first version of the subword complexes as of 2015` ​0aa6989 `trac #11010 full pep8 and pyflakes`

### comment:31 Changed 7 years ago by Christian Stump

It seems that all my previous work on this ticket was lost. I do not appreciate.

I don't understand. How can work be lost on a ticket?

### comment:32 Changed 7 years ago by Frédéric Chapoton

Sorry for having been a little rude. I was tired and angry yesterday evening.

It remains that I have done twice the same clean-up work. I think it was because the branch by Vincent was not based on my previous branch.

I would really like if this ticket would no longer depend on #11187, which has no hope to enter sage as long as it depends on gap3.

You said once that it could be possible, but with a high speed penalty. Maybe this is nevertheless the way to go.

### comment:33 Changed 7 years ago by git

Commit: 0aa6989fc49f62d72c8d6452f4bb46b4533518d7 → 32da9d29df4394dc0553d760bd2482f80dc07fed

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

 ​9dccaa7 `Merge branch 'u/chapoton/11010' into 6.10.b4` ​32da9d2 `trac #11010 newstyle doctest continuation`

### comment:34 Changed 7 years ago by git

Commit: 32da9d29df4394dc0553d760bd2482f80dc07fed → 046879866540893d9d7f678c2962728a322f0ebd

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

 ​0468798 `trac #11010 make sure that doc builds`

### comment:35 Changed 7 years ago by Frédéric Chapoton

Milestone: sage-6.8 → sage-6.10

### comment:36 Changed 7 years ago by Travis Scrimshaw

As far as I can see, just one thing is needed:

• `reflections()` (technically `nr_reflections()`, but we can just do `len(self.reflections())`)

We can construct this easily enough by conjugation of simple reflections:

```sage: W = CoxeterGroup(['A',2])
sage: S = W.simple_reflections()
sage: I = W.index_set()
sage: R = RecursivelyEnumeratedSet(S, lambda x: [S[i]*x*S[i] for i in I if not x.has_descent(i)], structure="graded")
sage: list(R)
[
[-1  1]  [ 1  0]  [ 0 -1]
[ 0  1], [ 1 -1], [-1  0]
]
sage: all(x.absolute_length() for x in R)
True
```

The `apply_simple_reflection` is already a part of Sage for Coxeter groups.

Aside - This probably deserves to be in the category of finite Coxeter groups, but there is some slight conflict with this

```sage: W = WeylGroup(['A',2], prefix='s')
sage: W.reflections()
Finite family {s1*s2*s1: (1, 0, -1), s1: (1, -1, 0), s2: (0, 1, -1)}
```

(Trying to do this on the Weyl group on the root system seems to run forever and `ctrl-C` out results in a nasty error message.)

### comment:37 Changed 7 years ago by git

Commit: 046879866540893d9d7f678c2962728a322f0ebd → 2d84a4b1f6bdb6e75c0aa1b1a571620c737e0cb6

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

 ​2d84a4b `trac #11010 little more doc`

### comment:38 Changed 7 years ago by git

Commit: 2d84a4b1f6bdb6e75c0aa1b1a571620c737e0cb6 → 3f104918266a45d31cf272fdc03786e302e98c80

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

 ​518eb3c `Merge branch 'u/chapoton/11010' into 6.10.rc0` ​3f10491 `trac #11010 more doc`

### comment:39 Changed 7 years ago by git

Commit: 3f104918266a45d31cf272fdc03786e302e98c80 → cf864bd924979485335feb4521a9c6b166eb36aa

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

 ​cf864bd `trac #11010 trying to add more doc (but alas blindly)`

### comment:40 Changed 7 years ago by git

Commit: cf864bd924979485335feb4521a9c6b166eb36aa → 1447f4bbe8aed1adcabccf762dce56b7b828a3cf

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

 ​1447f4b `trac #11010 a lot of work, still some parts are broken`

### comment:41 Changed 7 years ago by git

Commit: 1447f4bbe8aed1adcabccf762dce56b7b828a3cf → 004f8a8355bbd341aa6140d7d6e66d5d0ed81bc9

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

 ​004f8a8 `trac #11010 caching the positive roots`

### comment:42 Changed 7 years ago by git

Commit: 004f8a8355bbd341aa6140d7d6e66d5d0ed81bc9 → 8b5db7a55c65b449c00c18be8e6c0cc2ddaff199

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

 ​8b5db7a `trac 11010 more caching`

### comment:43 Changed 7 years ago by git

Commit: 8b5db7a55c65b449c00c18be8e6c0cc2ddaff199 → 4e50eec043dd0035b96f48489fababf928f9f21a

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

 ​4e50eec `trac #11010 more work on doc and code of subword complexes`

### comment:44 Changed 7 years ago by git

Commit: 4e50eec043dd0035b96f48489fababf928f9f21a → 567a7670bc8fdfe29367079ed8fcb3be4576fbb2

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

 ​567a767 `trac #11010 almost working now (but plot is broken)`

### comment:45 Changed 7 years ago by Frédéric Chapoton

Ok, guys. We are almost there. Most of the things are working and hopefully not too slow. I have worked hard for two days, so please react !

Just 2 small remaining problems (unless the bot reveals some others):

• the "plot" code is broken, maybe we can remove that for the moment ? unless somebody wants to repair it ?
• the "root_polytope" method is not documented, any objection to its removal ? unless you can provide a working example ?

cheers,

Frederic

Last edited 7 years ago by Frédéric Chapoton (previous) (diff)

### comment:46 Changed 7 years ago by Frédéric Chapoton

Dependencies: #12774, #11187

### comment:47 Changed 7 years ago by Frédéric Chapoton

Work issues: coverage → plot and root_polytope

### comment:48 Changed 7 years ago by Christian Stump

Hi Frederic -- thanks a lot for all your work here! I am still too busy to contribute, but I can promise to have had a closer look by the end of the week, together with my opinion on how to proceed and a few speed tests... Cheers, Christian

### comment:49 Changed 7 years ago by git

Commit: 567a7670bc8fdfe29367079ed8fcb3be4576fbb2 → d0a5f71b2d4b26e7cb5a932181e1390d45072777

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

 ​d0a5f71 `trac #11010 some details`

### comment:50 Changed 7 years ago by Christian Stump

Okay, things seem to work, great!

• Root polytope: all this patch was basically parked on trac for my own research. And this method is stuff for which nothing is proven so far, so it should not be part of the ticket anymore.
• Plot: what do you have in mind outside of types A and B?
• Here are a few timings:

This patch:

```sage: W = WeylGroup(['A',6])
sage: c = list(W.index_set()); Q = c + W.w0.coxeter_sorting_word(c)
sage: %time S = SubwordComplex(Q,W.w0)
CPU times: user 23.6 s, sys: 36 ms, total: 23.6 s
Wall time: 23.6 s
sage: len(S)
429
```

The same patch based on the Coxeter group implementation:

```sage: W = CoxeterGroup(['A',6])
sage: c = list(W.index_set()); Q = c + W.w0.coxeter_sorting_word(c)
sage: %time S = SubwordComplex(Q,W.w0)
CPU times: user 0.41 s, sys: 0.00 s, total: 0.41 s
Wall time: 0.41 s
sage: len(S)
429
```

### comment:51 Changed 7 years ago by Christian Stump

Branch: u/chapoton/11010 → u/stumpc5/11010

### comment:52 Changed 7 years ago by Christian Stump

Commit: d0a5f71b2d4b26e7cb5a932181e1390d45072777 → 6b65d7003f98adc0aab9984cfec28de9b6311eab needs_work → needs_review plot and root_polytope

I removed some experimental code that is not supposed to be merged (this includes the root polytope), and fixed the plot. This file's tests pass locally, so let's see what the patchbot says...

New commits:

 ​5c93cd4 `Merge branch 'u/chapoton/11010' of git://trac.sagemath.org/sage into 11010-new` ​6b65d70 `removed a few experimental methods, fixed the plot`

### comment:53 Changed 7 years ago by Frédéric Chapoton

I think one should put a large Warning about

`To be used only with Coxeter groups in the implementation='reflection'`

In fact, it is not only slow but mostly broken with Weyl groups.

Side remark : it seems that one cannot use the quadratic fields Q(sqrt2) for type B and Q(sqrt5) for type H. To be fixed in another ticket, maybe.

### comment:54 Changed 7 years ago by Christian Stump

In fact, it is not only slow but mostly broken with Weyl groups.

Oh yes, the doctests were written for `implementation='reflection'` so it does not show that it is broken for Weyl groups.

I will have a quick look whether I can fix that!

### comment:55 Changed 7 years ago by Travis Scrimshaw

I'm not too surprised `WeylGroup` is slower; it goes through the GAP interface (in particular, for multiplication) whereas the reflection implementation (of Coxeter groups) does not. However because of this, it can't get things like conjugacy classes. There is probably quite a bit we can improve with (GAP) matrix groups, but that is an issue for another ticket.

However, it surprises me a little bit that it is broken for Weyl groups because the reflection implementation was fairly minimal (up to what comes from the category).

### comment:56 follow-up:  59 Changed 7 years ago by Christian Stump

First, the definition of subword complexes is for Coxeter groups (finite or infinite), but many of its properties (see my paper with Vincent referenced in the header) come from understanding roots and weights. (This means that the construction works, but many methods don't.)

Second, I think the current failure is only because the method `WeylGroup.roots` is missing, as is the method `WeylGroupElement.action_on_root_indices`.

• Do you know how I can get the list of (simple, positive, almost positive, all) roots of a root system when I only have the Weyl group `W` at hand? It seems that I have to build a root system from the Cartan type, though it seems more natural to have a method `WeylGroup.root_system` or at least `WeylGroup.cartan_matrix`, wouldn't it?
Last edited 7 years ago by Christian Stump (previous) (diff)

### comment:57 follow-up:  58 Changed 7 years ago by Frédéric Chapoton

Branch: u/stumpc5/11010 → u/chapoton/11010 6b65d7003f98adc0aab9984cfec28de9b6311eab → f4bc87fa3a884cfb45e9c90bd37c4d1f55c8d090

New branch, where I have added a warning.

Do you really want this to work for Weyl groups ?

You can build roots for Weyl groups just as I did for the Coxeter groups, using a reduced word for w0. But it will not be enough..

New commits:

 ​59e9e2d `Merge branch 'u/stumpc5/11010' into 6.10` ​f4bc87f `trac #11010 some doc details, and pep8`

### comment:58 in reply to:  57 Changed 7 years ago by Christian Stump

Do you really want this to work for Weyl groups ?

I just want to quickly see if I can get the method `root_configuration` working since many others only depend on this one. And many people do use Weyl groups...

### comment:59 in reply to:  56 Changed 7 years ago by Christian Stump

• Do you know how I can get the list of (simple, positive, almost positive, all) roots of a root system when I only have the Weyl group `W` at hand? It seems that I have to build a root system from the Cartan type, though it seems more natural to have a method `WeylGroup.root_system` or at least `WeylGroup.cartan_matrix`, wouldn't it?

Ah, `WeylGroup.domain` does it...

### comment:60 Changed 7 years ago by git

Commit: f4bc87fa3a884cfb45e9c90bd37c4d1f55c8d090 → e252db3ee11ba3642fdcfdad8c472e7e0602f2b6

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

 ​e252db3 `trac #11010 bad unicode character removed`

### comment:61 Changed 7 years ago by Christian Stump

Do you really want this to work for Weyl groups ?

Okay, I give up on this -- I tried the past hours to get it done for Weyl groups, but I basically have to fix every single method, and even have to implement my own fundamental weights in order to make this work for Weyl groups. If this is crucial at some point, we can still get back to this...

### comment:62 Changed 7 years ago by Christian Stump

The patchbot is happy, so I think this is ready to go -- or do you see any remaining problems?

### comment:63 Changed 7 years ago by git

Commit: e252db3ee11ba3642fdcfdad8c472e7e0602f2b6 → 8b79780943df18379d9e16ce016abf83662ca261

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

 ​7d9f5e8 `Merge branch 'u/chapoton/11010' into 7.10.b2` ​8b79780 `trac #11010 one typo`

### comment:64 follow-up:  65 Changed 7 years ago by Frédéric Chapoton

I am not quite satisfied with the custom naive type A/B recognition in the plot method. Travis, is there a better way ? or even a way to have the correct type printed in the repr ?

### comment:65 in reply to:  64 Changed 7 years ago by Travis Scrimshaw

I am not quite satisfied with the custom naive type A/B recognition in the plot method. Travis, is there a better way ? or even a way to have the correct type printed in the repr ?

You can use the (relatively recent) Coxeter matrices and types:

```sage: W = CoxeterGroup(['C',4])
sage: T = W.coxeter_matrix().coxeter_type(); T
Coxeter type of ['C', 4]
```

The current implementation for Coxeter types essentially wraps the Cartan types, so there is a "different" type for B and C. (This was done because there was a lot of information that could be pulled from the Cartan type, as there is a lot of hard-coded data, and I didn't want to have to try and decide on a naming scheme. I fully take the blame for the hacks involved.)

However, the information you're after is inside the wrapped type:

```sage: T.cartan_type()
['B', 4]
sage: T.cartan_type().type()
'B'
sage: T.cartan_type().rank()
4
```

### comment:66 Changed 7 years ago by git

Commit: 8b79780943df18379d9e16ce016abf83662ca261 → 1b10c95a312057dee7d920ff4d06ea713ef5338b

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

 ​1b10c95 `trac #11010 re-introducing Cartan types`

### comment:67 Changed 7 years ago by Frédéric Chapoton

ok, thanks Travis. I have done the needed changes, I think.

### comment:68 Changed 7 years ago by Frédéric Chapoton

Milestone: sage-6.10 → sage-7.0 → Frédéric Chapoton

ok, I think that this can go. Christian, if you agree, set to positive.

### comment:69 Changed 7 years ago by Christian Stump

Status: needs_review → positive_review

### comment:70 Changed 7 years ago by Travis Scrimshaw

Does the plotting also work for type C, being that it is the dual of type B (and we are dealing with Cartan types)?

### comment:71 Changed 7 years ago by Christian Stump

Status: positive_review → needs_work

I am rather confused now:

• First Sage start
```sage: W = CoxeterGroup(['B',4])
sage: W.coxeter_matrix().coxeter_type()
Coxeter type of ['B', 4]
sage: W = CoxeterGroup(['C',4])
sage: W.coxeter_matrix().coxeter_type()
Coxeter type of ['B', 4]
```
• Second Sage start
```sage: W = CoxeterGroup(['C',4])
sage: W.coxeter_matrix().coxeter_type()
Coxeter type of ['C', 4]
sage: W = CoxeterGroup(['B',4])
sage: W.coxeter_matrix().coxeter_type()
Coxeter type of ['C', 4]
```

In particular, the root systems `W.roots()` are not correct in the two second calls of `CoxeterGroup`.

Also, the plotting now assumes that the first index is the type B/C special one. This is, it is expected that

```sage: s0 = W.simple_reflection(W.index-set()[0])
sage: s1 = W.simple_reflection(W.index-set()[1])
sage: (s0*s1).order()
4
```

I doubt that this is correct in the current implementation of `Coxeter groups` where the last index is expected to be special. If so, I will add some magic to figure out the proper labelling...

Last edited 7 years ago by Christian Stump (previous) (diff)

### comment:72 Changed 7 years ago by Travis Scrimshaw

The first issue is the result of my quick hacking to get something working. The Coxeter matrices are equal for the Cartan types B and C, and the Coxeter matrix is the information passed to the `UniqueRepresentation` cache. There are a number of ways to go moving forward, but I think that deserves its own separate ticket.

For the second issue, yes, you are correct. The `n-1` and `n` nodes have the order 4 relation between them, as this agrees with the corresponding B/C Cartan types (and the Coxeter matrix). You are welcome to relabel the Coxeter type, which should work, or go to the Coxeter graph and find the edge label4ed by 4 and use those nodes. Personally, I would use the latter option since it is independent of the labeling.

### comment:73 Changed 7 years ago by Christian Stump

I also have the problem that the plot only allows irreducible types. When playing with reducible types, I got to the types B vs. G bug in #19830...

### comment:74 Changed 7 years ago by Christian Stump

Last edited 7 years ago by Christian Stump (previous) (diff)

### comment:75 Changed 7 years ago by Christian Stump

Branch: u/chapoton/11010 → u/stumpc5/11010

### comment:76 Changed 7 years ago by Frédéric Chapoton

Commit: 1b10c95a312057dee7d920ff4d06ea713ef5338b → 839b2e2437ae30bb4bd42095f8f112b9dbe50eee

Is this "needs_review" ?

New commits:

 ​4864a64 `Merge branch '11010-new' into u/stumpc5/11010-new` ​7722489 `Merge branch 'u/chapoton/11010' of git://trac.sagemath.org/sage into u/stumpc5/11010-new` ​839b2e2 `fixing the index set for the plot`

### comment:77 Changed 7 years ago by Christian Stump

Is this "needs_review" ?

Not quite, I wanted to add a few more doctests for the type B plotting with various indexing sets. But then saw that the indexing in `CoxeterGroup` is corrupted, see #19830. This is another issue, but I still want to add more examples/testing.

### comment:78 Changed 7 years ago by git

Commit: 839b2e2437ae30bb4bd42095f8f112b9dbe50eee → 0f90c266d0b653830d4555b839b041d633e3935d

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

 ​0f90c26 `fixed another b/c bug + some cosmetics to the code`

### comment:79 Changed 7 years ago by git

Commit: 0f90c266d0b653830d4555b839b041d633e3935d → e8e160c5cbca21a348b6505414b7952e073fd56d

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

 ​e8e160c `fixed a docstring and adding one more test`

### comment:80 Changed 7 years ago by Christian Stump

Status: needs_work → needs_review

### comment:81 Changed 7 years ago by Frédéric Chapoton

Branch: u/stumpc5/11010 → u/chapoton/11010 e8e160c5cbca21a348b6505414b7952e073fd56d → 17518c11f06b77c1823a5b0950285085540a41b2 needs_review → positive_review

ok, good enough for me.

I have made two minor doc changes, and now set this again to positive review.

Any further changes will have to be in another ticket.

New commits:

 ​17518c1 `trac #11010 two details`

### comment:82 Changed 7 years ago by Volker Braun

Branch: u/chapoton/11010 → 17518c11f06b77c1823a5b0950285085540a41b2 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.