Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of finite reflection groups #11187

Closed
stumpc5 opened this issue Apr 13, 2011 · 435 comments
Closed

Implementation of finite reflection groups #11187

stumpc5 opened this issue Apr 13, 2011 · 435 comments

Comments

@stumpc5
Copy link
Contributor

stumpc5 commented Apr 13, 2011

This patch implements reflection groups to work with finite (crystallographic, real, complex) reflection groups. It is based on the GAP3 package chevie which is now an experimental package, see #20107.

sage: W = ReflectionGroup(['A',2],5,23); W
Reducible complex reflection group of rank 7 and type A2 x ST5 x H3
sage: W.irreducible_components()
[Irreducible real reflection group of rank 2 and type A2,
 Irreducible complex reflection group of rank 2 and type ST5,
 Irreducible real reflection group of rank 3 and type H3]

CC: @sagetrac-vripoll @kevindilks @simon-king-jena @sagetrac-jmichel @hivert

Component: combinatorics

Keywords: reflection group, days49, days64.5, days80

Author: Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Travis Scrimshaw

Branch/Commit: 0b6d895

Reviewer: Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Vivien Ripoll, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/11187

@stumpc5
Copy link
Contributor Author

stumpc5 commented Apr 18, 2011

comment:2

Apply only trac_11187-finite_reflection_groups-cs.patch

@stumpc5
Copy link
Contributor Author

stumpc5 commented Apr 24, 2011

comment:3

Apply only trac_11187-finite_reflection_groups-cs.patch

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jun 8, 2011

Dependencies: 8327

@stumpc5

This comment has been minimized.

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jun 10, 2011

comment:6

Apply only trac_11187-finite_reflection_groups-cs.patch

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jun 10, 2011

Changed dependencies from 8327 to #8327

@stumpc5 stumpc5 added this to the sage-4.7.2 milestone Jun 11, 2011
@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 24, 2013

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jan 24, 2013

comment:9
+    class SubcategoryMethods:
+
+        @cached_method
+        def Irreducible(self):
+            """
+            Returns the full subcategory of the irreducible objects of ``self``.
+
+            EXAMPLES::
+
+                sage: CoxeterGroups().Irreducible()
+                Category of irreducible coxeter groups
+
+            TESTS::
+
+                sage: AffineWeylGroups().Irreducible.__module__
+                'sage.categories.complex_reflection_groups'
+            """
+            return self._with_adjectives(('Irreducible',))
+
+    class Irreducible(AdjectiveCategory):
+        pass
+

@tscrim
Copy link
Collaborator

tscrim commented Apr 12, 2013

Changed dependencies from #8327 to #8327 #14137

@tscrim
Copy link
Collaborator

tscrim commented Apr 12, 2013

comment:10

I've factored out the CartanMatrix class into #14137.

@tscrim
Copy link
Collaborator

tscrim commented Jun 21, 2013

comment:11

I've rebased the patch and noticed that this will likely depend on #10963. I'll post a new version once I completely untangle what is going on with #10963.

@tscrim
Copy link
Collaborator

tscrim commented Jun 21, 2013

Changed keywords from reflection group to reflection group, days49

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jun 21, 2013

comment:12

Replying to @tscrim:

I've rebased the patch and noticed that this will likely depend on #10963. I'll post a new version once I completely untangle what is going on with #10963.

I always made some minor changes on this patch locally - I would actually only start working on this patch after you guys are finished with #10963. If you want to actively work on this patch, ping me again, and we can discuss what is still needed to finilize this patch.

Cheers, Christian

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@fchapoton
Copy link
Contributor

comment:16

Hello Christian

Do you have a git branch for this ticket somewhere ? or should I try to resurrect the patch ?

@stumpc5
Copy link
Contributor Author

stumpc5 commented Jul 1, 2014

comment:17

Do you have a git branch for this ticket somewhere ? or should I try to resurrect the patch ?

No, I don't. But I have a local old version of Sage containing that code and quite a bit more, so I wonder if I should/will try to organize it a bit and put it up here.

Do you need any from that code in particular (in which case I am more tempted to work on that), or is that just a general question?

@hivert
Copy link

hivert commented Apr 21, 2016

comment:264

Replying to @stumpc5:

all tests pass this time, sorry for not being able to get you the error...

Please let me know if it happen again.

@fchapoton
Copy link
Contributor

comment:265

Replying to @hivert:

Replying to @stumpc5:

all tests pass this time, sorry for not being able to get you the error...

Please let me know if it happen again.

see there for one problem with map_reduce:

http://patchbot.sagemath.org/log/11187/Ubuntu/14.04/x86_64/3.16.0-60-generic/irma-atlas/2016-04-19%2009:27:11?short

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 22, 2016

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

8d76520using a cached version self._number_of_reflections to avoid computing it again and again, expecially important in has_descent

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 22, 2016

Changed commit from 9b0ef92 to 8d76520

@stumpc5
Copy link
Contributor Author

stumpc5 commented Apr 22, 2016

comment:267

Should I actually at all still push stuff into this ticket, or should we rather get it to positive review and do everything else in individual tickets? (I have redone the doctest for reflection_group_real.py after this last change.)

@fchapoton
Copy link
Contributor

comment:268

Please stop touching anything here if you want this to be positive reviewed soon and not in 4 more years.

@stumpc5
Copy link
Contributor Author

stumpc5 commented Apr 22, 2016

comment:269

if you want this to be positive reviewed soon and not in 4 more years.

That's a little harsh for my taste. Anyway, I agree that I should stop doing anything here, the list of improvements is at #20394.

@fchapoton
Copy link
Contributor

comment:270

sorry for that, I am not in a good mood..

@stumpc5
Copy link
Contributor Author

stumpc5 commented Apr 22, 2016

comment:271

I would rebase this to beta5, run the patchbot and do all doctests with the gap3 flag one more time.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 22, 2016

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

0b6d895Merge branch 'develop' into public/11187

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 22, 2016

Changed commit from 8d76520 to 0b6d895

@tscrim
Copy link
Collaborator

tscrim commented Apr 22, 2016

Changed author from Christian Stump to Christian Stump, Frédéric Chapoton, Nicolas Thiery, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Apr 22, 2016

Reviewer: Christian Stump, Frédéric Chapoton, Nicolas Thiery, Vivian Ripoll, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Apr 22, 2016

Changed reviewer from Christian Stump, Frédéric Chapoton, Nicolas Thiery, Vivian Ripoll, Travis Scrimshaw to Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Vivien Ripoll, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Apr 22, 2016

Changed author from Christian Stump, Frédéric Chapoton, Nicolas Thiery, Travis Scrimshaw to Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Travis Scrimshaw

@fchapoton
Copy link
Contributor

Changed author from Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Travis Scrimshaw to Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Travis Scrimshaw

@fchapoton
Copy link
Contributor

Changed reviewer from Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Vivien Ripoll, Travis Scrimshaw to Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Vivien Ripoll, Travis Scrimshaw

@fchapoton
Copy link
Contributor

comment:276

ok, guys, I think it is time. I am setting to positive review in the name of all authors/reviewers.

@stumpc5
Copy link
Contributor Author

stumpc5 commented Apr 22, 2016

comment:277

Great -- thanks to all of you for participating!

@vbraun
Copy link
Member

vbraun commented Apr 23, 2016

Changed branch from public/11187 to 0b6d895

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants