Opened 11 years ago
Closed 6 years ago
#11010 closed enhancement (fixed)
Implementation of the SubwordComplex as defined by Knutson and Miller
Reported by: | stumpc5 | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-7.0 |
Component: | combinatorics | Keywords: | subword complex, simplicial complex |
Cc: | Merged in: | ||
Authors: | Christian Stump | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | 17518c1 (Commits, GitHub, GitLab) | Commit: | 17518c11f06b77c1823a5b0950285085540a41b2 |
Dependencies: | Stopgaps: |
Description (last modified by )
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)}
Attachments (1)
Change History (83)
comment:1 Changed 11 years ago by
- Component changed from PLEASE CHANGE to combinatorics
- Description modified (diff)
- Keywords subword complex simplicial complex added
- Status changed from new to needs_work
- Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 11 years ago by
- Dependencies set to 11187, 11122
- Description modified (diff)
comment:3 Changed 11 years ago by
- Dependencies changed from 11187, 11122 to #11187, #11122
comment:4 Changed 11 years ago by
- Milestone set to sage-4.7.2
comment:5 Changed 11 years ago by
- Dependencies changed from #11187, #11122 to #11122
comment:6 follow-up: ↓ 7 Changed 10 years ago by
- Dependencies changed from #11122 to #12774
- Status changed from needs_work to needs_review
comment:7 in reply to: ↑ 6 Changed 10 years ago by
- Dependencies changed from #12774 to #11187, #12774
Changed 9 years ago by
comment:8 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:9 Changed 8 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:10 Changed 8 years ago by
- Branch set to u/chapoton/11010
- Commit set to a525fdcf47ead6e1794c4f54acac00ee70f07e91
comment:11 Changed 8 years ago by
- Status changed from needs_review to needs_work
- Work issues set to coverage
The code needs to be 100% doctested. Some empty methods should be removed.
comment:12 Changed 8 years ago by
- Dependencies changed from #11187, #12774 to #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 8 years ago by
- Commit changed from a525fdcf47ead6e1794c4f54acac00ee70f07e91 to 76afdca9d2842a4f0a4060f60130bb37c41df99f
comment:14 Changed 8 years ago by
- Dependencies changed from #11187, #12774, #15456 to #12774
comment:15 Changed 8 years ago by
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 8 years ago by
- Commit changed from 76afdca9d2842a4f0a4060f60130bb37c41df99f to 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 8 years ago by
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 8 years ago by
Replying to chapoton:
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 upgraded Sage using http://wiki.sagemath.org/TentativeConventions#Get_the_latest_official_development_version_of_Sage, but I got an error
error: command 'gcc' failed with exit status
, so this is my first problem
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 8 years ago by
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 8 years ago by
- Branch changed from u/chapoton/11010 to u/stumpc5/ticket/11010
- Modified changed from 03/17/14 09:12:06 to 03/17/14 09:12:06
comment:21 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:22 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:23 Changed 7 years ago by
- Commit changed from 88643156a58720df968a0cb95e38f94668871a1a to 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
- Branch changed from u/stumpc5/ticket/11010 to u/vpilaud/ticket/11010
comment:25 Changed 7 years ago by
- Commit changed from ab914f50d616ad4df0fbd5a8c587d1843cd0a58e to bd4f2ada52cbe683bd8c768e82f729a24b73e8aa
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
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
- Commit changed from bd4f2ada52cbe683bd8c768e82f729a24b73e8aa to 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
- Milestone changed from sage-6.4 to sage-6.8
does not apply, needs to be rebased
comment:29 Changed 7 years ago by
- Dependencies changed from #12774 to #12774, #11187
comment:30 Changed 7 years ago by
- Branch changed from u/vpilaud/ticket/11010 to u/chapoton/11010
- Commit changed from f165d54668bc6342df43c2c31877195aa2f7765d to 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
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
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
- Commit changed from 0aa6989fc49f62d72c8d6452f4bb46b4533518d7 to 32da9d29df4394dc0553d760bd2482f80dc07fed
comment:34 Changed 7 years ago by
- Commit changed from 32da9d29df4394dc0553d760bd2482f80dc07fed to 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
- Milestone changed from sage-6.8 to sage-6.10
comment:36 Changed 7 years ago by
As far as I can see, just one thing is needed:
reflections()
(technicallynr_reflections()
, but we can just dolen(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
- Commit changed from 046879866540893d9d7f678c2962728a322f0ebd to 2d84a4b1f6bdb6e75c0aa1b1a571620c737e0cb6
Branch pushed to git repo; I updated commit sha1. New commits:
2d84a4b | trac #11010 little more doc
|
comment:38 Changed 6 years ago by
- Commit changed from 2d84a4b1f6bdb6e75c0aa1b1a571620c737e0cb6 to 3f104918266a45d31cf272fdc03786e302e98c80
comment:39 Changed 6 years ago by
- Commit changed from 3f104918266a45d31cf272fdc03786e302e98c80 to 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 6 years ago by
- Commit changed from cf864bd924979485335feb4521a9c6b166eb36aa to 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 6 years ago by
- Commit changed from 1447f4bbe8aed1adcabccf762dce56b7b828a3cf to 004f8a8355bbd341aa6140d7d6e66d5d0ed81bc9
Branch pushed to git repo; I updated commit sha1. New commits:
004f8a8 | trac #11010 caching the positive roots
|
comment:42 Changed 6 years ago by
- Commit changed from 004f8a8355bbd341aa6140d7d6e66d5d0ed81bc9 to 8b5db7a55c65b449c00c18be8e6c0cc2ddaff199
Branch pushed to git repo; I updated commit sha1. New commits:
8b5db7a | trac 11010 more caching
|
comment:43 Changed 6 years ago by
- Commit changed from 8b5db7a55c65b449c00c18be8e6c0cc2ddaff199 to 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 6 years ago by
- Commit changed from 4e50eec043dd0035b96f48489fababf928f9f21a to 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 6 years ago by
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
comment:46 Changed 6 years ago by
- Dependencies #12774, #11187 deleted
comment:47 Changed 6 years ago by
- Work issues changed from coverage to plot and root_polytope
comment:48 Changed 6 years ago by
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 6 years ago by
- Commit changed from 567a7670bc8fdfe29367079ed8fcb3be4576fbb2 to d0a5f71b2d4b26e7cb5a932181e1390d45072777
Branch pushed to git repo; I updated commit sha1. New commits:
d0a5f71 | trac #11010 some details
|
comment:50 Changed 6 years ago by
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) 429The 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 6 years ago by
- Branch changed from u/chapoton/11010 to u/stumpc5/11010
comment:52 Changed 6 years ago by
- Commit changed from d0a5f71b2d4b26e7cb5a932181e1390d45072777 to 6b65d7003f98adc0aab9984cfec28de9b6311eab
- Status changed from needs_work to needs_review
- Work issues plot and root_polytope deleted
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 6 years ago by
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 6 years ago by
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 6 years ago by
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 6 years ago by
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 methodWeylGroup.root_system
or at leastWeylGroup.cartan_matrix
, wouldn't it?
comment:57 follow-up: ↓ 58 Changed 6 years ago by
- Branch changed from u/stumpc5/11010 to u/chapoton/11010
- Commit changed from 6b65d7003f98adc0aab9984cfec28de9b6311eab to 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 6 years ago by
Replying to chapoton:
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 6 years ago by
Replying to stumpc5:
- 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 methodWeylGroup.root_system
or at leastWeylGroup.cartan_matrix
, wouldn't it?
Ah, WeylGroup.domain
does it...
comment:60 Changed 6 years ago by
- Commit changed from f4bc87fa3a884cfb45e9c90bd37c4d1f55c8d090 to e252db3ee11ba3642fdcfdad8c472e7e0602f2b6
Branch pushed to git repo; I updated commit sha1. New commits:
e252db3 | trac #11010 bad unicode character removed
|
comment:61 Changed 6 years ago by
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 6 years ago by
The patchbot is happy, so I think this is ready to go -- or do you see any remaining problems?
comment:63 Changed 6 years ago by
- Commit changed from e252db3ee11ba3642fdcfdad8c472e7e0602f2b6 to 8b79780943df18379d9e16ce016abf83662ca261
comment:64 follow-up: ↓ 65 Changed 6 years ago by
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 6 years ago by
Replying to 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 ?
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 6 years ago by
- Commit changed from 8b79780943df18379d9e16ce016abf83662ca261 to 1b10c95a312057dee7d920ff4d06ea713ef5338b
Branch pushed to git repo; I updated commit sha1. New commits:
1b10c95 | trac #11010 re-introducing Cartan types
|
comment:67 Changed 6 years ago by
ok, thanks Travis. I have done the needed changes, I think.
comment:68 Changed 6 years ago by
- Milestone changed from sage-6.10 to sage-7.0
- Reviewers set to Frédéric Chapoton
ok, I think that this can go. Christian, if you agree, set to positive.
comment:69 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:70 Changed 6 years ago by
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 6 years ago by
- Status changed from positive_review to 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...
comment:72 Changed 6 years ago by
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 6 years ago by
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 6 years ago by
comment:75 Changed 6 years ago by
- Branch changed from u/chapoton/11010 to u/stumpc5/11010
comment:76 Changed 6 years ago by
- Commit changed from 1b10c95a312057dee7d920ff4d06ea713ef5338b to 839b2e2437ae30bb4bd42095f8f112b9dbe50eee
comment:77 Changed 6 years ago by
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 6 years ago by
- Commit changed from 839b2e2437ae30bb4bd42095f8f112b9dbe50eee to 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 6 years ago by
- Commit changed from 0f90c266d0b653830d4555b839b041d633e3935d to e8e160c5cbca21a348b6505414b7952e073fd56d
Branch pushed to git repo; I updated commit sha1. New commits:
e8e160c | fixed a docstring and adding one more test
|
comment:80 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:81 Changed 6 years ago by
- Branch changed from u/stumpc5/11010 to u/chapoton/11010
- Commit changed from e8e160c5cbca21a348b6505414b7952e073fd56d to 17518c11f06b77c1823a5b0950285085540a41b2
- Status changed from needs_review to 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 6 years ago by
- Branch changed from u/chapoton/11010 to 17518c11f06b77c1823a5b0950285085540a41b2
- Resolution set to fixed
- Status changed from positive_review to closed
Replying to chapoton:
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