Opened 6 years ago

Closed 12 months ago

#11187 closed enhancement (fixed)

Implementation of finite reflection groups

Reported by: stumpc5 Owned by: tbd
Priority: major Milestone: sage-7.2
Component: combinatorics Keywords: reflection group, days49, days64.5, days80
Cc: vripoll, kdilks, SimonKing, jmichel, hivert Merged in:
Authors: Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Travis Scrimshaw Reviewers: Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Vivien Ripoll, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 0b6d895 (Commits) Commit: 0b6d895c865ebdb319de6d58e862e7ae1a1cb784
Dependencies: Stopgaps:

Description (last modified by stumpc5)

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]

Attachments (1)

trac_11187-finite_reflection_groups-cs.patch (199.3 KB) - added by stumpc5 4 years ago.

Download all attachments as: .zip

Change History (279)

comment:1 Changed 6 years ago by stumpc5

  • Component changed from PLEASE CHANGE to combinatorics
  • Status changed from new to needs_work

comment:2 Changed 6 years ago by stumpc5

Apply only trac_11187-finite_reflection_groups-cs.patch

comment:3 Changed 6 years ago by stumpc5

  • Status changed from needs_work to needs_review

Apply only trac_11187-finite_reflection_groups-cs.patch

comment:4 Changed 6 years ago by stumpc5

  • Dependencies set to 8327
  • Description modified (diff)

comment:5 Changed 6 years ago by stumpc5

  • Status changed from needs_review to needs_work

comment:6 Changed 6 years ago by stumpc5

Apply only trac_11187-finite_reflection_groups-cs.patch

comment:7 Changed 6 years ago by stumpc5

  • Dependencies changed from 8327 to #8327

comment:8 Changed 6 years ago by stumpc5

  • Milestone set to sage-4.7.2

Changed 4 years ago by stumpc5

comment:9 Changed 4 years ago by stumpc5

+    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
+

comment:10 Changed 4 years ago by tscrim

  • Dependencies changed from #8327 to #8327 #14137

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

comment:11 follow-up: Changed 4 years ago by tscrim

  • Keywords days49 added

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.

comment:12 in reply to: ↑ 11 Changed 4 years ago by stumpc5

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

comment:13 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:14 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:15 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:16 follow-up: Changed 3 years ago by chapoton

Hello Christian

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

comment:17 in reply to: ↑ 16 Changed 3 years ago by stumpc5

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?

comment:18 Changed 3 years ago by chapoton

Hello, no, no real need for this code, just asking a general question. Now that #10963 is over.

comment:19 Changed 3 years ago by tscrim

  • Branch set to public/groups/finite_reflection_groups-11187
  • Commit set to 1f3fe1e0caa28a1cb6e2d17a61d94c89460e2bd4

Here's a git version of this patch with changes necessary to get Sage to start (I did not run any tests, so I am not saying things are working). There might be other changes to the files in question which need to be moved over too. Just as a general note from doing the rebasing, there are a bunch of methods missing doctests.


New commits:

3489e1c#11187 Implementation of finite reflection groups using the GAP3 package chevie
1f3fe1eSome other fixes from changes to Sage in order to get Sage to start.

comment:20 Changed 3 years ago by git

  • Commit changed from 1f3fe1e0caa28a1cb6e2d17a61d94c89460e2bd4 to b60e61b6695fb1cde48d867390d50c32569fc2b0

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

bc4e068Merge branch 'public/groups/finite_reflection_groups-11187' of ssh://trac.sagemath.org:22/sage into 11187
b60e61btrac #11187 a few minor changes

comment:21 Changed 3 years ago by git

  • Commit changed from b60e61b6695fb1cde48d867390d50c32569fc2b0 to 1de360ff9e9c4db57e7de8b4ea45bbc4807e243c

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

1de360ftrac #11187 crystallo takes 2 l

comment:22 Changed 3 years ago by git

  • Commit changed from 1de360ff9e9c4db57e7de8b4ea45bbc4807e243c to deb13e3c50d259c1708aaa815300d94045a14b07

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

deb13e3trac #11187 trying to make doctest pass

comment:23 Changed 3 years ago by git

  • Commit changed from deb13e3c50d259c1708aaa815300d94045a14b07 to b6c1a6273f96f1b5d6103b3090eee3633b2f54b2

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

b6c1a62trac #11187 a few more corrections

comment:24 Changed 3 years ago by git

  • Commit changed from b6c1a6273f96f1b5d6103b3090eee3633b2f54b2 to becb1cab8eacec6e60661677fb2c1fcfe2429afe

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

becb1catrac #11187 a few more changes, removing duplicate functions

comment:25 Changed 3 years ago by git

  • Commit changed from becb1cab8eacec6e60661677fb2c1fcfe2429afe to e97998c532abc75eec6b18fecff6769601a1bade

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

da599f0Merge branch 'public/groups/finite_reflection_groups-11187' of ssh://trac.sagemath.org:22/sage into 11187
e97998ctrac #11187 more work, mostly minor

comment:26 Changed 3 years ago by git

  • Commit changed from e97998c532abc75eec6b18fecff6769601a1bade to 03111c8373909a6f4ac8e7c8b63625c74b4077a5

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

03111c8trac #11187 a little pyflakes cleanup

comment:27 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:28 Changed 3 years ago by git

  • Commit changed from 03111c8373909a6f4ac8e7c8b63625c74b4077a5 to 4a37eac384b296acde87af5bddbe39a4ba8addf0

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

6a0a3f3Merge branch 'public/groups/finite_reflection_groups-11187' of ssh://trac.sagemath.org:22/sage into 11187
4a37eactrac #11187 very tiny changes

comment:29 Changed 2 years ago by git

  • Commit changed from 4a37eac384b296acde87af5bddbe39a4ba8addf0 to 527cb502a4f3729472344a7fbb894502ddd03050

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

527cb50Merge branch 'public/groups/finite_reflection_groups-11187' into 6.6.b1

comment:30 Changed 2 years ago by vripoll

  • Cc vripoll added

comment:31 Changed 2 years ago by git

  • Commit changed from 527cb502a4f3729472344a7fbb894502ddd03050 to f2fbe0ad1c694936e6d15195055c261aadc17e1d

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

f2fbe0amerged in 6.8.beta2

comment:32 Changed 2 years ago by stumpc5

  • Branch changed from public/groups/finite_reflection_groups-11187 to u/stumpc5/11187

comment:33 Changed 2 years ago by git

  • Commit changed from f2fbe0ad1c694936e6d15195055c261aadc17e1d to ba7b5f092f41da9bbdb18455ecc5af2623dc42cc

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

ba7b5f0made Sage not crash on startup

comment:34 Changed 2 years ago by kdilks

  • Cc kdilks added

comment:35 Changed 2 years ago by stumpc5

  • Branch changed from u/stumpc5/11187 to u/stumpc5/11187-new

comment:36 Changed 2 years ago by git

  • Commit changed from ba7b5f092f41da9bbdb18455ecc5af2623dc42cc to 6ca40fa7459d11bf9c3564777d962a7493eb476c

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

a2f0f31first implementation of the iterator through magma elements
6ca40faMerge branch 't/18584/iterator_through_magma_elements' into u/stumpc5/11187-new

comment:37 Changed 2 years ago by git

  • Commit changed from 6ca40fa7459d11bf9c3564777d962a7493eb476c to 33f93c6061d90dd3849525f63aa7ffaa21049492

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

33f93c6removed the magma iterator

comment:38 Changed 2 years ago by git

  • Commit changed from 33f93c6061d90dd3849525f63aa7ffaa21049492 to 08a782138c922172d901834b691a80ec850f9bb2

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

08a7821fixed all doctests in src/sage/combinat/root_system/complex_reflection_group.py

comment:39 Changed 2 years ago by git

  • Commit changed from 08a782138c922172d901834b691a80ec850f9bb2 to 014fec229c73bb16da7825d24633c47076f22c4c

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

014fec2got the doctests of complex reflection group category pass

comment:40 Changed 2 years ago by git

  • Commit changed from 014fec229c73bb16da7825d24633c47076f22c4c to d3198f92cb72fd0ca67c4118ddcb00289143bd62

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

d3198f9fixing a few bugs on coxeter number and fundamental invariants

comment:41 Changed 2 years ago by git

  • Commit changed from d3198f92cb72fd0ca67c4118ddcb00289143bd62 to 8db1deeed4893580fd7c88d42d2fb9366a895bcf

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

e2736f6added another doctest
8db1deefirst version of adding functionality to coxeter group categories

comment:42 Changed 2 years ago by git

  • Commit changed from 8db1deeed4893580fd7c88d42d2fb9366a895bcf to 425ca38c084a51fa56c301153165269031fb5e0d

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

425ca38first version of coxeter groups from chevie with all tests passing!

comment:43 Changed 2 years ago by SimonKing

  • Cc SimonKing added

comment:44 Changed 2 years ago by SimonKing

The current code gives

sage: C = ComplexReflectionGroups()
sage: C.Irreducible()
Category of complex reflection groups
sage: C.Irreducible() is C
True

Is that what you intended? Perhaps the "irreducible" axiom should be defined on ComplexReflectionGroups().Finite(), not on ComplexReflectionGroups().

comment:45 Changed 2 years ago by SimonKing

To summarise the discussion Christian and I just had: I believe WellGenerated should be an axiom. Christian said that the general notion of well-generated is a bit tricky for infinite groups. Hence, he actually only wants to take into account well-generatedness for finite complex reflection groups.

Hence, the aim would be to define WellGeneratedComplexReflectionGroups as application of an axiom to ComplexReflectionGroups.Finite. However, I believe that the name should contain the word "finite" if we really want to focus on finite well-generated groups. Alternatively, it should be implemented as a nested-nested class WellGenerated within the nested class ComplexReflectionGroups.Finite.

I also think that WellGeneratedComplexReflectionGroups should provide a parent method _test_well_generated that tests whether self is well-generated. Such method would automatically be executed when running the test suite.

comment:46 Changed 2 years ago by SimonKing

We have discussed already that in order to fix the Irreducible axiom, it is needed (in addition to the nested class) to have a SubcategoryMethod called Irreducible.

Note that if we define WellGeneratedComplexReflectionGroups as a sub-category of ComplexReflectionGroups.Finite(), we should probably remove its subcategory-methods Finite and Irreducible, since both axioms have already been defined in the super-category.

comment:47 Changed 2 years ago by SimonKing

The iterators currently cache the list of all elements of the group. Is that (always) a good idea?

comment:48 Changed 2 years ago by SimonKing

  • Branch changed from u/stumpc5/11187-new to u/SimonKing/11187-new

comment:49 Changed 2 years ago by SimonKing

  • Commit changed from 425ca38c084a51fa56c301153165269031fb5e0d to c0a036b2e2f01b1be6a7d004373fafe0c399e14d

After discussion with Christian, the iterator of a complex reflection group is a lot faster. What we did was to keep ._reduced_word as a list, and only transform it into an actual word when needed (i.e., in the .reduced_word() method).

Before:

sage: G = ComplexReflectionGroup([2,1,4])
sage: %timeit L = list(G); G._elements=None
The slowest run took 6.02 times longer than the fastest. This could mean that an intermediate result is being cached 
1 loops, best of 3: 66.4 ms per loop

After:

sage: G = ComplexReflectionGroup([2,1,4])
sage: %timeit L = list(G); G._elements=None
The slowest run took 66.44 times longer than the fastest. This could mean that an intermediate result is being cached 
1 loops, best of 3: 5.56 ms per loop

New commits:

c0a036bAvoid creation of _reduced_word during iteration

comment:50 Changed 2 years ago by stumpc5

  • Branch changed from u/SimonKing/11187-new to u/stumpc5/11187-new

comment:51 Changed 2 years ago by chapoton

  • Commit changed from c0a036b2e2f01b1be6a7d004373fafe0c399e14d to 0639665e4f6ac2bee4a4cc9dfcaed5963d910155
  • Milestone changed from sage-6.4 to sage-6.8

New commits:

405b26cturned well-generated into an axiom
0639665fixed a but with Word(reduced_word) we worked on yesterday

comment:52 Changed 23 months ago by git

  • Commit changed from 0639665e4f6ac2bee4a4cc9dfcaed5963d910155 to 28f8abd2eb45700978592f1781a1f87e3c4d5e69

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

28f8abdadded a test for well generated

comment:53 Changed 23 months ago by SimonKing

Hi Christian,

This is not how the tests work:

                def _test_well_generated(self, **options):
                    tester = self._tester(**options)
                    return self.is_well_generated()

Instead, do

                def _test_well_generated(self, **options):
                    tester = self._tester(**options)
                    tester.assert_(self.is_well_generated())

or something similar.

comment:54 Changed 23 months ago by chapoton

see patchbot reports :

  • some doctests should be marked with # optional - chevie or whatever
  • doctest continuation is now ....: and not ...
  • raise statements should use py3 syntax raise Error('message')

comment:55 Changed 23 months ago by git

  • Commit changed from 28f8abd2eb45700978592f1781a1f87e3c4d5e69 to 697cd3f3077ef749bee642e0281b24fc66c3cbeb

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

3c79040added rational catalan numbers
697cd3freplaced asserts by raises and a few other fixes

comment:56 follow-up: Changed 23 months ago by stumpc5

@Simon and Nicolas: The category for complex reflection groups (with axioms finite/irreducible/well-generated) is now sort of ready. I would now like to make ComplexReflectionGroups().Finite().WellGenerated() a super category of FiniteCoxeterGroups.

  • Is that what I should do?
  • If I simply add a method supercategories to FiniteCoxeterGroups, its string repr changes in a not desired way. How should I treat that?

Thanks, Christian

comment:57 Changed 23 months ago by git

  • Commit changed from 697cd3f3077ef749bee642e0281b24fc66c3cbeb to 92ab198593eb2d772c48c4cfb80408d6da1daaa9

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

92ab198made the category of coxeter groups a subcategory of complex reflection groups

comment:58 in reply to: ↑ 56 Changed 23 months ago by tscrim

Replying to stumpc5:

@Simon and Nicolas: The category for complex reflection groups (with axioms finite/irreducible/well-generated) is now sort of ready. I would now like to make ComplexReflectionGroups().Finite().WellGenerated() a super category of FiniteCoxeterGroups.

  • Is that what I should do?
  • If I simply add a method supercategories to FiniteCoxeterGroups, its string repr changes in a not desired way. How should I treat that?

It sounds more like you want to add ComplexReflectionGroups().WellGenerated() as a supercategory to CoxeterGroups, in particular to replace Groups().FinitelyGenerated() by ComplexReflectionGroups().WellGenerated(). I also think ComplexReflectionGroups() should be a subcategory of Groups().FinitelyGenerated() (otherwise the additional axiom should go on the supercategory for CoxeterGroups()). The finite part will take care of itself by the CategoryWithAxiom magic.

comment:59 Changed 23 months ago by git

  • Commit changed from 92ab198593eb2d772c48c4cfb80408d6da1daaa9 to abc690c06e9f6b60bde44f689259d3787d5681ea

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

ec94ae3added a few more doctests for CoxeterGroupsChevie
1878996work in progress: new implementations are presumably correct, but existing bug discovered
5716df5Fixed according to Travis's suggestions
4404133Fixed according to Travis's suggestions, and now actually running I think
8fe1bb8Tab fixed
a6dcb8bmerged with now-resolved conflicts
c37a5efthings appear to work in the sense that all tests have passed.
1862325removed trailing whitespace.
1f29ae6reordered documentation, as requested by chapoton
abc690cmany doctests, and merged #18597

comment:60 follow-up: Changed 23 months ago by chapoton

Hello,

  • once again: doctest continuation is now ....: and not ...
  • could you please put a positive review on the duplicate #9288 ?

comment:61 Changed 23 months ago by git

  • Commit changed from abc690c06e9f6b60bde44f689259d3787d5681ea to 742c0df5916cdd39d843553858f295df9a1ed18e

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

742c0dfreplaced '... ' by '....:'

comment:62 in reply to: ↑ 60 Changed 23 months ago by stumpc5

Hi

  • once again: doctest continuation is now ....: and not ...

done

  • could you please put a positive review on the duplicate #9288 ?

done

Thanks for looking at this (and all the other tickets!

Christian

comment:63 Changed 23 months ago by git

  • Commit changed from 742c0df5916cdd39d843553858f295df9a1ed18e to df37b7e9dbec9913ad652826e165e7f534ded0d3

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

ab914f5first version of the subword complexes as of 2015
cb3ca9cimprove the subword complex, in particular its documentation
f105602Merge branch 'u/stumpc5/11010' into u/stumpc5/11187-new
8618845playing, I hopefully can reorganize this again
ab4999fmore intelligent fix a la stump
df37b7eMerge branch 't/18610/bug__circular_descent_check_in_weylgroups' into u/stumpc5/11187-new

comment:64 Changed 23 months ago by stumpc5

  • Dependencies changed from #8327 #14137 to #8327 #14137 #18590

comment:65 Changed 23 months ago by stumpc5

  • Dependencies changed from #8327 #14137 #18590 to #8327 #14137 #18590 #18597

comment:66 Changed 23 months ago by git

  • Commit changed from df37b7e9dbec9913ad652826e165e7f534ded0d3 to e180c3133fb8c90f941b3f6f4be8a10e0bb15065

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

e180c31Revert "improve the subword complex, in particular its documentation"

comment:67 Changed 23 months ago by git

  • Commit changed from e180c3133fb8c90f941b3f6f4be8a10e0bb15065 to 450dadd370f2a8b1e6d05630d669742df3a1beef

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

450daddRevert "first version of the subword complexes as of 2015"

comment:68 Changed 23 months ago by stumpc5

I took out the subword complex stuff which is handled in #11010, and which is depending on this one.

comment:69 Changed 23 months ago by git

  • Commit changed from 450dadd370f2a8b1e6d05630d669742df3a1beef to e8e0d9ae9e5770b2088d6fb9ee6db412d21353a2

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

e8e0d9aadded two doctests

comment:70 Changed 23 months ago by stumpc5

  • Dependencies changed from #8327 #14137 #18590 #18597 to #8327 #14137 #18590 #18597 #18620

comment:71 Changed 23 months ago by stumpc5

  • Dependencies changed from #8327 #14137 #18590 #18597 #18620 to #18620

comment:72 Changed 23 months ago by git

  • Commit changed from e8e0d9ae9e5770b2088d6fb9ee6db412d21353a2 to 80b9a54c6f35f6204fd226df8e95df45b05435d3

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

e6de158added more doctests
80b9a54added more doctests, many more to come

comment:73 Changed 23 months ago by git

  • Commit changed from 80b9a54c6f35f6204fd226df8e95df45b05435d3 to d98ee4ecfcdc337ba74ec9a008dd58a17239cfde

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

a89a90fremoved trailing whitespace.
ce30fafreordered documentation, as requested by chapoton
21abe06replaced '... ' by '....:'
4121914improve the subword complex, in particular its documentation
b2b7f68more intelligent fix a la stump
d662af6Revert "improve the subword complex, in particular its documentation"
1ef2de7Revert "first version of the subword complexes as of 2015"
03b60e2added two doctests
2a34befadded more doctests
d98ee4eadded more doctests, many more to come

comment:74 Changed 23 months ago by git

  • Commit changed from d98ee4ecfcdc337ba74ec9a008dd58a17239cfde to 294d5f8f513b74bae5de22221b069a21df1ed6f4

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

294d5f8fixed a docstring

comment:75 Changed 23 months ago by git

  • Commit changed from 294d5f8f513b74bae5de22221b069a21df1ed6f4 to eddaef5ada286366d9dc8d1f5af329f915c9da76

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

2661cedlazy import of ComplexReflectionGroups
346532eold style doc replaced
894a671fixed some doctests for ComplexReflectionGroups
eddaef5more doctest for ComplexReflectionGroups, almost done there

comment:76 Changed 23 months ago by git

  • Commit changed from eddaef5ada286366d9dc8d1f5af329f915c9da76 to fb852f57be8fe8897ee2c2bad7b4847c865476e8

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

fb852f5added back the references to __init__

comment:77 Changed 23 months ago by git

  • Commit changed from fb852f57be8fe8897ee2c2bad7b4847c865476e8 to 3783ea23dc6a102eea745f59cd2a6b48fb87d5ca

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

3783ea2added some stuff that got lost during rebasing

comment:78 Changed 23 months ago by git

  • Commit changed from 3783ea23dc6a102eea745f59cd2a6b48fb87d5ca to 43f1dcedca38ef310502c66f4bc9147040d2972a

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

c07e76emoved from ComplexReflectionGroup/CoxeterGroupChevie to ReflectionGroup
f845089change in irreducible components
bbe138fMerge branch 't/18620/9a3e5cdacac40b505f7aad0a62b9e1bec76d8f3a' into u/stumpc5/11187-new
43f1dcereflection groups in the reference

comment:79 Changed 23 months ago by git

  • Commit changed from 43f1dcedca38ef310502c66f4bc9147040d2972a to 2f737ce0ee193dc7de8b89586dc3939f2686c161

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

2f737cegot the documentation run again as expected

comment:80 Changed 23 months ago by git

  • Commit changed from 2f737ce0ee193dc7de8b89586dc3939f2686c161 to 9df16f6d63f7107669e9229211f9520aeb39a7ad

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

9df16f6cleaned the documentation of the complex reflection group category

comment:81 Changed 23 months ago by git

  • Commit changed from 9df16f6d63f7107669e9229211f9520aeb39a7ad to bfd9aa684127059d97777ad55432772322a78ebc

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

bfd9aa6added some references to ComplexReflectionGroups

comment:82 Changed 23 months ago by git

  • Commit changed from bfd9aa684127059d97777ad55432772322a78ebc to e3b047bff2bd12f99a3fbea87f5417cd2c012745

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

e3b047badded some references to ComplexReflectionGroups

comment:83 Changed 23 months ago by stumpc5

  • Description modified (diff)
  • Keywords days 64.5 added

comment:84 Changed 23 months ago by git

  • Commit changed from e3b047bff2bd12f99a3fbea87f5417cd2c012745 to d6736681a50cfae4bcd58ad601af43860b5733e3

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

7c8cc37typo
dd3d515work on cartan and coxeter matrices
d673668removed a few patches that are not needed here

comment:85 Changed 23 months ago by git

  • Commit changed from d6736681a50cfae4bcd58ad601af43860b5733e3 to ef3cbe7303af54c2390e04404fb6a4c2623b9c3e

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

ef3cbe7fixed a docstring

comment:86 follow-up: Changed 23 months ago by chapoton

Hello Christian, thanks for your hard work on this ticket. I have only had a quick look, so trivial remarks:

  • not yet 100% coverage, I think (I have seen a few methods without examples)
  • TODO:: should be .. TODO::
  • Could you please replace Returns by Return everywhere.
  • Sometime the first line of the doc is made of two sentences. Please separate them as:
    Return something.
    
    Something is very very important.
    
  • Please try to keep the lines 80 characters long.
  • Bad alignement of EXAMPLES:: in distinguished_reflection, distinguished_reflections
  • missing optional in sage: W = CoxeterGroup(["A",4], implementation="chevie")

And you should aim to have a green light from the patchbot. So far, this has never been the case.

comment:87 in reply to: ↑ 86 Changed 23 months ago by stumpc5

Replying to chapoton:

Thanks for having a look at this ticket!

I have only had a quick look, so trivial remarks

I am aware of the stuff to do in the documentation, but before getting there (or, at least, in parallel, I'll fix your complaints now), I try to get the overall setting right (this includes the category framework, the interaction with other reflection group classes, and the dependence of chevie).

And you should aim to have a green light from the patchbot. So far, this has never been the case.

I'd like to get it green, but it has not run in 5 days, and I can only fix stuff the patchbot complaints about if I see its results regularly after uploading a new commit sequence (a patchbot that doesn't run soon after uploading doesn't really help during a coding sprint)...

Thanks again, Christian

comment:88 Changed 23 months ago by chapoton

There are many patchbots, but only one that has added all your participants to its trust list (at my demand). This one is currently not active. You can try to contact some active patchbots owners to ask them to trust the participants (see http://wiki.sagemath.org/buildbot/owners and http://patchbot.sagemath.org/machines). Or run your own patchbot.

comment:89 Changed 23 months ago by stumpc5

There are many patchbots, but only one that has added all your participants to its trust list (at my demand).

Oh, I didn't know that all participants must be on the trust list, I thought it was only the submitter of the branch - I gonna try to get a patchbot running...

comment:90 Changed 23 months ago by chapoton

Yes, this comes from some difficulty to identify the true authors of the code..

For patchbot, please see the instructions there : http://wiki.sagemath.org/buildbot/details

comment:91 Changed 23 months ago by git

  • Commit changed from ef3cbe7303af54c2390e04404fb6a4c2623b9c3e to 40d22b23447993f976e9680aec23d32c4f4c34f4

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

64fe437copyright and stripped trailing whitespaces
0a616e2minor movings of code
eee3af5moved reduced_word for real groups
40d22b2fixed todos, bad alignments, returns

comment:92 Changed 23 months ago by kdilks

I just started a patchbot recently, and I believe I now have it configured to trust all relevant participants and prioritize this ticket.

First run through wasn't very helpful, since I don't have Chevie installed. Though presumably everything should be marked with some kind of optional tag so that tests are skipped if Chevie isn't installed.

I'm in the process of trying to install Chevie, and if I succeed, I'll have it do another run.

comment:93 Changed 23 months ago by kdilks

I have Gap3/Chevie installed now.

It's probably easier (and a lot faster) to run

./sage -t ./src/sage/combinat/root_system/reflection_group_complex.py ,

and if things are crashing and you need to see where, then do

./sage -t --verbose ./src/sage/combinat/root_system/reflection_group_complex.py .

Patchbots are good for finding merge conflicts with new betas and seeing if you unintentionally broke other parts of Sage, but at this point I think it's sufficient to only test the one file you're changing with this ticket.

After playing around for a bit, something seems to be broken with the iterator. The example W = ReflectionGroup((1,1,3)) (should be isomorphic to S3) included as a test works fine, but if I try W = ReflectionGroup((3,1,1)) (should be isomorphic to cyclic group of order 3) then it keeps printing out the three elements in a never-ending loop.

comment:94 Changed 23 months ago by jdemeyer

Replace

# optional (require chevie)

by

# optional - chevie

comment:95 Changed 23 months ago by jdemeyer

When adding new files, you should follow http://doc.sagemath.org/html/en/developer/coding_basics.html#headings-of-sage-library-code-files, in particular use the correct license statement.

comment:96 follow-up: Changed 23 months ago by stumpc5

Thanks for having a look here! I am currently working (since yesterday) on speeding the iteration through the group, I hope I have gotten somewhere later today...

I guess I must tag each and every test as

# optional - chevie

which means that these tests are not run on patchbots per default, or do they run if the patchbot has chevie installed?

comment:97 in reply to: ↑ 96 ; follow-up: Changed 23 months ago by jdemeyer

Replying to stumpc5:

I guess I must tag each and every test

Not literally every test, there are probably tests which do not require chevie.

do they run if the patchbot has chevie installed?

They would be run if the Sage optional package chevie is installed. However, such a package does not exist.

comment:98 in reply to: ↑ 97 ; follow-up: Changed 23 months ago by stumpc5

Replying to jdemeyer:

Not literally every test, there are probably tests which do not require chevie.

Every test (except for the creation of the category of complex reflection groups) handle with reflection groups (no surprise), and even initializing such a group requires chevie.

But within finite time, I will work on making some core functionalities work without having chevie installed.

They would be run if the Sage optional package chevie is installed. However, such a package does not exist.

Is that a problem? Should there be a possibility of installing chevie as an optional package? I update the description of this ticket to explain how to get chevie installed in a system. See #8906 for old stuff about that.

Last edited 23 months ago by stumpc5 (previous) (diff)

comment:99 in reply to: ↑ 98 Changed 23 months ago by jdemeyer

Replying to stumpc5:

Is that a problem? Should there be a possibility of installing chevie as an optional package?

It's not a problem for me or for this ticket. I guess it is a problem if people want to use the code that you are writing here, since getting chevie to work within Sage is non-trivial.

comment:100 Changed 23 months ago by stumpc5

  • Description modified (diff)

comment:101 Changed 23 months ago by git

  • Commit changed from 40d22b23447993f976e9680aec23d32c4f4c34f4 to 1c64c9b599944baabbecea7267854493546687c7

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

5a205a1moved right coset representatives
25139ddremoved the preloading of the ucf for reflection groups, and moved the new iterator into real reflection groups.
7df41c2new iterators through group
83f9893updated licence statements
1c64c9badded a todo for the iterator

comment:102 Changed 23 months ago by git

  • Commit changed from 1c64c9b599944baabbecea7267854493546687c7 to 43f505667c936dc07ed42a653383e844f2555cab

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

d0bfd81fixed many doctests coming from new iteration order
43f5056reached 100% coverage and all tests passing

comment:103 Changed 23 months ago by stumpc5

Okay, got 100% coverage and all tests passing! But I wouldn't know how to get the startup module green as I turned the ComplexReflectionGroups category into a super category of CoxeterGroups which is loaded on startup.

comment:104 follow-up: Changed 23 months ago by kdilks

I have a failing doctest for is_regular. There's not a problem with the is_regular method, but for W = ReflectionGroup(23), there are two elements where my computer returns a different reduced word than expected ( https://www.diffchecker.com/ewvevrda ). I consistently get the same different answer. Are you using the older/more stable gap-jm4, or the latest gap-jm5 ?

Unrelated, but I also can't get the long_element() method to work.

Last edited 23 months ago by kdilks (previous) (diff)

comment:105 in reply to: ↑ 104 Changed 23 months ago by stumpc5

Replying to kdilks:

for W = ReflectionGroup(23), there are two elements where my computer returns a different reduced word than expected

This computation is currently done purely in my code, and it's not related to chevie. The iterator comes from _iterator_tracking_words so let's see what happens there.

Related here, and indeed also to your question on long_element, observe that currently

sage: W = ReflectionGroup(23); W
Irreducible complex reflection group of rank 3 and type H3
sage: W.category()
Join of Category of finite permutation groups and Category of irreducible finite coxeter groups
sage: W = ReflectionGroup(['H',3]); W
Irreducible real reflection group of rank 3 and type H3
sage: W.category()
Join of Category of finite permutation groups and Category of irreducible finite coxeter groups

Since the iter algorithm for coxeter groups is slightly smarter (it takes into account that the weak order is graded), the two iter functions are different.

But:

  • both iter functions are supposed to return the same iteration through the group
  • both iter functions should always return the elements in the very same order (in contrary to what we see on my vs on your machine)

I found a bug (having a set rather than a list in one spot), so let's see if this is fixed with my next push in a few minutes after all tests pass here.

Unrelated, but I also can't get the long_element() method to work.

The reason is, that you constructed the real group as a complex group, so the long element for ReflectionGroup(['H',3]) works while it currently doesn't work for ReflectionGroup(23). I do have to fix that and be more careful constructing the right class instance.

comment:106 Changed 23 months ago by git

  • Commit changed from 43f505667c936dc07ed42a653383e844f2555cab to 29a3fa3b0f4f303db3f0ec6c0835a902563a4812

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

e056f65iteration is now stable among machines
29a3fa3reordered some doctests according to the new iteration order

comment:107 Changed 23 months ago by git

  • Commit changed from 29a3fa3b0f4f303db3f0ec6c0835a902563a4812 to b123a8170cbb025cfdeb6cb783b0b49854848f32

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

4a13365trivialities
556ab0echanged the reduced word computation for real groups
b123a81hope the iteration order is now stable, also added a few optional chevie, but not many

comment:108 Changed 22 months ago by chapoton

There are a few failing doctests, some easy, some less clear.

comment:109 Changed 22 months ago by chapoton

now needs rebase, and care for the many failing doctests

comment:110 Changed 21 months ago by git

  • Commit changed from b123a8170cbb025cfdeb6cb783b0b49854848f32 to ab724b4cb5560b44007591ef873eb30c840dd08f

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

5f8f940improved the reduced word for reflection groups
711cf2c8906: create file ``type`` declaring spkg optional
f2777b88906: create package-version.txt file containing the version
c5addcf8906: generate checksums.ini
af16d798906: create SPKG.txt file with package description, etc.
ddbe1d58906: create spkg-install script, patches and increment patch level
0defcc18906: add gap3 commandline option to sage startup script
ab724b4merged latest develop

comment:111 Changed 21 months ago by git

  • Commit changed from ab724b4cb5560b44007591ef873eb30c840dd08f to 963397d991abbb26605eb76cf7b3a895ba90d0ac

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

1c64c9badded a todo for the iterator
d0bfd81fixed many doctests coming from new iteration order
43f5056reached 100% coverage and all tests passing
e056f65iteration is now stable among machines
29a3fa3reordered some doctests according to the new iteration order
4a13365trivialities
556ab0echanged the reduced word computation for real groups
b123a81hope the iteration order is now stable, also added a few optional chevie, but not many
5f8f940improved the reduced word for reflection groups
963397dmerged develop

comment:112 Changed 20 months ago by git

  • Commit changed from 963397d991abbb26605eb76cf7b3a895ba90d0ac to 7fd11b569246a9bc351b31e84c8c65e529b9f4aa

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

7fd11b5Merge branch 'develop' into u/stumpc5/11187-new

comment:113 Changed 20 months ago by stumpc5

  • Dependencies changed from #18620 to #18620, #8906

comment:114 Changed 19 months ago by git

  • Commit changed from 7fd11b569246a9bc351b31e84c8c65e529b9f4aa to 64431778120d52faa686e33c23ac0ac6c9d472a3

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

6443177Merge branch 'develop' into u/stumpc5/11187-new

comment:115 Changed 16 months ago by git

  • Commit changed from 64431778120d52faa686e33c23ac0ac6c9d472a3 to 3f66a41fc017ceaa8806eaba8db27207e511ac2b

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

218d6faMerge branch 'u/stumpc5/11187-new' into 11187-test
00b9834started replacing eval by sage_eval
3f66a41Merge branch '11187-test' into 11187-new

comment:116 Changed 16 months ago by git

  • Commit changed from 3f66a41fc017ceaa8806eaba8db27207e511ac2b to dd250eb43799a4d1f7670614611a9d002956955f

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

61b0606Merge branch 'public/interfaces/fix_gap3-19795' of git://trac.sagemath.org/sage into u/stumpc5/11187-new
dd250ebMerge branch 'u/stumpc5/11187-new' into temp

comment:117 Changed 16 months ago by git

  • Commit changed from dd250eb43799a4d1f7670614611a9d002956955f to a00d9c7c47a8689219b9ae8ae723c2b4afa890d5

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

a00d9c7sage_eval to eval in one instance, needs to be checked at some point

comment:118 Changed 15 months ago by git

  • Commit changed from a00d9c7c47a8689219b9ae8ae723c2b4afa890d5 to 8c4204a0d70b13061de5d6602f43692bc627ec24

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

8c4204abug fix for the fundamental invariants

comment:119 Changed 15 months ago by git

  • Commit changed from 8c4204a0d70b13061de5d6602f43692bc627ec24 to 7bb0e6cca3dd78380cfaa176f5e5384b626cb4b0

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

3b24329fixed small bug
683a2f7added doctest for the bug
7bb0e6cMerge branch 'u/stumpc5/19912' into u/stumpc5/11187-new

comment:120 Changed 15 months ago by chapoton

  • Milestone changed from sage-6.8 to sage-7.1

comment:121 Changed 15 months ago by git

  • Commit changed from 7bb0e6cca3dd78380cfaa176f5e5384b626cb4b0 to 3957a563983601f143659b1a843fc656ca4a0ae3

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

20d254cfixed the same bug in two more places
fa40e5cMerge branch 'u/stumpc5/19912' into u/stumpc5/11187-new
1df9f53working on a few glitches for CRGs
66ec881merged 7.1.beta0
3957a56Merge branch 'u/stumpc5/19912' into u/stumpc5/11187-new

comment:122 Changed 15 months ago by stumpc5

  • Dependencies changed from #18620, #8906 to #18620, #8906, #19912

comment:123 Changed 15 months ago by git

  • Commit changed from 3957a563983601f143659b1a843fc656ca4a0ae3 to 530a5c802620230b461820cb737f59dedb335f16

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

530a5c8added two implementations of the invariant forms

comment:124 Changed 15 months ago by git

  • Commit changed from 530a5c802620230b461820cb737f59dedb335f16 to 8b39c853ac1d85e90e4ed84633c777a2e63ff701

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

6d67adawork on reflection hyperplanes
8b39c85added linear functional description of hyperplanes

comment:125 Changed 14 months ago by git

  • Commit changed from 8b39c853ac1d85e90e4ed84633c777a2e63ff701 to aa87d02482826ef4211c06e9fe2ebc13048fc482

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

aa87d02added matrix representation for non-well-generated types

comment:126 Changed 14 months ago by git

  • Commit changed from aa87d02482826ef4211c06e9fe2ebc13048fc482 to 186d38745e7dbde9f619d7152a48bc33065c77ab

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

186d387Merge branch 'u/stumpc5/11187-new' into test

comment:127 Changed 14 months ago by git

  • Commit changed from 186d38745e7dbde9f619d7152a48bc33065c77ab to 094c608b5eda8d16f1b84af1f439614e4f869a2e

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

094c608added some more todos for character theory

comment:128 Changed 14 months ago by stumpc5

  • Dependencies changed from #18620, #8906, #19912 to #20107

comment:130 in reply to: ↑ 129 Changed 13 months ago by stumpc5

  • Cc jmichel added

Replying to dimpase:

are you aware of Geck's PyCox? http://www.mathematik.uni-stuttgart.de/~geckmf/chv1r6180.py

Sure, but it is limited to finite _real_ reflection groups, while I certainly want to work with real and with complex reflection groups. E.g., the results in

were only possible by using the code in this ticket to work in Sage with complex reflection groups from chevie through the gap3 interface.

comment:131 Changed 13 months ago by git

  • Commit changed from 094c608b5eda8d16f1b84af1f439614e4f869a2e to 3da1ebb8a47bdd4ff2c36761b15e663c6d2485fe

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

cac3206improved spkg-src
181a352undoing one thing in spkg-src
ec01e30moved to experimental package
6e599cfrenamed optional package to experimental package in interfaces/gap3.py
1583f99replaced #20107 by :trac:20107
38dbd60replaced optional chevie by optional gap3 in combinat/root_system/coxeter_group.py
d4f3a1dupdated the spkg-src to delete gap3 packages that need compilation
7696a2cadded patch for init.g and fixed typo in patch for gap.sh
3d123dfmerged 20107 to the newest develop
3da1ebbMerge branch 'u/stumpc5/11187' into tmp

comment:132 Changed 13 months ago by stumpc5

Concerning the iteration through elements without keeping them in memory, we have to benchmark agains

    gap> i:=0;;
    gap> W:=CoxeterGroup("E",7);;
    gap> ForEachElement(W,function(x)i:=i+1; 
    > if i mod 1000000=0 then Print("*\c");fi;
    > end);Print("\n");
    **

which takes about a second per million elements, see https://webusers.imj-prg.fr/~jean.michel/gap3/htm/chap075.htm.

comment:133 Changed 13 months ago by vripoll

  • Branch changed from u/stumpc5/11187-new to u/vripoll/11187-new

comment:134 follow-up: Changed 13 months ago by tscrim

  • Cc hivert added
  • Commit changed from 3da1ebb8a47bdd4ff2c36761b15e663c6d2485fe to e2f919fa6529b2444dbec4cfffde671b8927d1b7
  • Milestone changed from sage-7.1 to sage-7.2

Continuing the discussion from #13580.

Replying to hivert:

Replying to tscrim:

Christian, Vivien, and I are working on trying to speed up the iteration of finite Coxeter groups by using (subgroups of) permutation groups and the iterator method of generic Coxeter groups, which uses a SearchForest (see #11187 and the CoxeterGroups category). Our goal is to try and match or beat GAP3 iteration time (which we are about ~30x slower).

I'll be both very happy and interested to have usecase for this code.

This will have the more serious use case I know of (the code in the generic Coxeter groups iterator, e.g., via CoxeterGroup, has not had any serious optimization attempts AFAIK).

We are in the process pushing our successor computation code to Cython and some of our other methods, but we would appreciate any insights you have.

Do you have a typical example relying on SearchForest of code on top of #11187 that I can profile ? Note that I included in the documentation of map_reduce.py a guide on how to profile the parallel code. I'd like to measure the proportion of time that is spend on Coxeter code one one hand and on the parallel infrastructure on the other.

Our current test cases are B6 (~0.5s) and E7 (~1m).

Now, concerning coxeter group, I'm quite sure that rewriting part of the code in C/C++ and using advanced parallel stuff (AVX instruction set + SIMD) you can get speedup a large as several hundreds and even thousand ! As a witness on https://github.com/hivert/IVMPG I wrote a code whose goal is to enumerate integer vector modulo permutation group. Compared to the code we have in Sage which has been Cythonized and optimized by Simon King I manage to get speedup a large as x2000 ! I need help on the build system to be able to distribute it. I'm pretty sure the code there is very similar to what you need in Coxeter groups.

One of the thoughts I'm bouncing around is doing a full C/C++ version using arrays, and then using Cython (mainly PermutationGroupElement) as a transfer layer to Sage/GAP. In which case it probably should be split off as a little specialized spkg.

That is quite an impressive speedup. I know a little bit about build systems (and its on my todo to learn more for fixing up coxeter3/LiE too), but, e.g., Jeroen or François are good people to ask. Although if you need GCC 5.x, then we will need to do some changes to Sage. Yet, I think moving to GCC 5.x is something that we want and are going to do soon...


New commits:

e2f919fNew iterator for real reflection groups.
Last edited 13 months ago by tscrim (previous) (diff)

comment:135 in reply to: ↑ 134 Changed 13 months ago by hivert

Replying to tscrim:

One of the thoughts I'm bouncing around is doing a full C/C++ version using arrays, and then using Cython (mainly PermutationGroupElement) as a transfer layer to Sage/GAP. In which case it probably should be split off as a little specialized spkg.

This is very similar to what I did. The idea is that if you want large performance, you need to completely control the way you use the memory. So what I did is to build in parallel a large linked list of arrays and then thanks to Cython I returns Sage an iterator which is a view on this list which wrap the element of the linked list into proper Sage Element.

comment:136 Changed 13 months ago by stumpc5

  • Branch changed from u/vripoll/11187-new to u/stumpc5/11187-new

comment:137 Changed 13 months ago by git

  • Commit changed from e2f919fa6529b2444dbec4cfffde671b8927d1b7 to 48835b7db7daaf22eb903f146140d06b44dda872

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

48835b7Merge branch 'develop' into u/stumpc5/11187

comment:138 Changed 13 months ago by vripoll

  • Branch changed from u/stumpc5/11187-new to u/vripoll/11187-new

comment:139 Changed 13 months ago by stumpc5

  • Commit changed from 48835b7db7daaf22eb903f146140d06b44dda872 to 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a

For the record: as discussed last night, the first goal would be to separate the category stuff into its own ticket (@tscim is working on this), and then to get the present version well documented, tested, and finished. Only after this work is done, we will start improving stuff and add functionality.

To this end, we have to:

  • get the category stuff right
  • test properly that reducible groups work as desired
  • test properly that the labelling system of simples/hyperplanes/reflections works properly
  • test that the assumptions on how we obtain the data from chevie is correct (one example here is that the code tries to get n simple reflections, N* distinguished reflections, and then the remaining (N-N*) other reflections when listing all reflections. This has to be rechecked, but there are plenty of more similar examples.

New commits:

e75f1a2Updated TODO list
1b798a7Merge branch 'u/stumpc5/11187-new' of git://trac.sagemath.org/sage into t/11187/11187-new

comment:140 Changed 13 months ago by chapoton

  • Branch changed from u/vripoll/11187-new to u/chapoton/11187
  • Commit changed from 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a to 0fc36ce6f8ae66d792fd8197313786d961f37fa4

new branch, I have cleaned up the imports


New commits:

0fc36cetrac #11187 pyflakes cleanup

comment:141 Changed 13 months ago by git

  • Commit changed from 0fc36ce6f8ae66d792fd8197313786d961f37fa4 to 220e0f03ed025daeb6372bee28477882afee6861

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

220e0f0trac #11187 changing next to python3 syntax

comment:142 Changed 13 months ago by git

  • Commit changed from 220e0f03ed025daeb6372bee28477882afee6861 to 3270a98685af6ab39f2c3e13f695a5dd226227d6

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

3270a98trac #11187 remove a method reflection to root in complex groups

comment:143 Changed 13 months ago by git

  • Commit changed from 3270a98685af6ab39f2c3e13f695a5dd226227d6 to b947ea518e2152eb903d88f459c183a85d3f3587

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

b947ea5trac #11187 adding doc for reflecting hyperplane

comment:144 Changed 13 months ago by git

  • Commit changed from b947ea518e2152eb903d88f459c183a85d3f3587 to a697817ab730832f10a06ee6a1ff878be7060dd3

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

a697817trac #11187 remove the invariant linear form in complex, for the moment

comment:145 Changed 13 months ago by tscrim

  • Dependencies changed from #20107 to #20107 #20397

comment:146 Changed 13 months ago by git

  • Commit changed from a697817ab730832f10a06ee6a1ff878be7060dd3 to 9b73808d144d03319e2bde278e07f55f3e34741e

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

9b73808trac #11187 correct a doctest in coxeter_groups

comment:147 Changed 13 months ago by git

  • Commit changed from 9b73808d144d03319e2bde278e07f55f3e34741e to 114b20b8cc80c613f656262d495441edc02a9261

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

114b20btrac #11187 adding a doctest for the invariant form

comment:148 Changed 13 months ago by git

  • Commit changed from 114b20b8cc80c613f656262d495441edc02a9261 to 4e80c52a448ead8b5072d1ba0a342de1ec116098

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

71b77a1Merge branch 'develop' into u/tscrim/11187
b171922Merge branch 'u/stumpc5/11187-new' of git://trac.sagemath.org/sage into u/tscrim/11187
67e882aTrying to get a bit more speed out and doing some code cleanup.
c76dd9dMerge branch 'u/tscrim/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
ea14eb8fixed some doctests, fixed a bug in hyperplanes, work on the iterator
4e80c52Merge branch 'u/chapoton/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187

comment:149 Changed 13 months ago by git

  • Commit changed from 4e80c52a448ead8b5072d1ba0a342de1ec116098 to d7343219c7e12dd870605967b9be27222a27a2df

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

d734321trac #11187 a typo in doc in complex reflection groups

comment:150 Changed 13 months ago by git

  • Commit changed from d7343219c7e12dd870605967b9be27222a27a2df to 7d88a6a49bd3f6332e7a3f71b59739d3aeb5d843

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

7d88a6atrac #11187 again a minor doc typo in complex refl group

comment:151 Changed 13 months ago by git

  • Commit changed from 7d88a6a49bd3f6332e7a3f71b59739d3aeb5d843 to f42aba99d6597cfb7c849900cba284b43816f64e

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

f42aba9trac #11187 a typo in doc of real-refl-groups

comment:152 Changed 13 months ago by tscrim

  • Branch changed from u/chapoton/11187 to u/tscrim/11187
  • Commit changed from f42aba99d6597cfb7c849900cba284b43816f64e to e3b52f7780447e5cfb208bfc12bd088f36d3cf47

New commits:

19af4e7Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
b7f5ffeMerge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
a384a06Pulling changes from #11187 and making tests use colored permutations.
e3b52f7Merge branch 'public/categories/complex_reflection_groups-20397' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187

comment:153 Changed 13 months ago by vripoll

  • Branch changed from u/tscrim/11187 to u/vripoll/11187-new
  • Commit changed from e3b52f7780447e5cfb208bfc12bd088f36d3cf47 to 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a

comment:154 Changed 13 months ago by git

  • Commit changed from 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a to 2b01e8df1562bb6415635f784f8423cea3af8fd8

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

c76dd9dMerge branch 'u/tscrim/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
ea14eb8fixed some doctests, fixed a bug in hyperplanes, work on the iterator
4e80c52Merge branch 'u/chapoton/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
d734321trac #11187 a typo in doc in complex reflection groups
7d88a6atrac #11187 again a minor doc typo in complex refl group
f42aba9trac #11187 a typo in doc of real-refl-groups
b7f5ffeMerge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
a384a06Pulling changes from #11187 and making tests use colored permutations.
e3b52f7Merge branch 'public/categories/complex_reflection_groups-20397' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
2b01e8dMerge branch 'u/tscrim/11187' of git://trac.sagemath.org/sage into t/11187/11187-new

comment:155 Changed 13 months ago by stumpc5

  • Branch changed from u/vripoll/11187-new to u/stumpc5/11187-new

comment:156 Changed 13 months ago by tscrim

  • Branch changed from u/stumpc5/11187-new to u/tscrim/11187
  • Commit changed from 2b01e8df1562bb6415635f784f8423cea3af8fd8 to 6daedef45c6888f596b803977897aeec9512e4ac
  • Dependencies changed from #20107 #20397 to #20107

Everything should work now up to the printing of well-generated as part of the finite Coxeter group category.


Last 10 new commits:

f77e89fGetting the degrees correct.
14d4c39Merge branch 'public/20395' into public/categories/complex_reflection_groups-20397
79bbb3fMerge branch 'public/categories/complex_reflection_groups-20397' into u/tscrim/reflection_groups-11187
a7c3db8Fixing up the categories and doing some other cleanup.
b9e0b0dworking on the iterator
823ab4fMerge branch 'u/chapoton/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
80fe997finished to encorporate the new iterator
3538599Merge branch 'u/vripoll/11187-new' of git://trac.sagemath.org/sage into u/stumpc5/11187
90244d3Merge branch 'u/stumpc5/11187-new' of git://trac.sagemath.org/sage into u/tscrim/reflection_groups-11187
6daedefFixing everything else up except for well-generated being added to finite Coxeter groups.

comment:157 Changed 13 months ago by chapoton

  • Branch changed from u/tscrim/11187 to u/chapoton/11187
  • Commit changed from 6daedef45c6888f596b803977897aeec9512e4ac to 5f568f95708bd417947b28de3bf2806d423c947b

New commits:

5f568f9trac #11187 fixing shepard groups

comment:158 Changed 13 months ago by stumpc5

  • Branch changed from u/chapoton/11187 to u/stumpc5/11187

comment:159 Changed 13 months ago by tscrim

  • Branch changed from u/stumpc5/11187 to u/tscrim/11187
  • Commit changed from 5f568f95708bd417947b28de3bf2806d423c947b to d4775c7541edc49cc4aaace14edeec15255db700

I was able to stop GeneralizedCoxeterGroup, ComplexReflectionGroup, and FiniteCoxeterGroup from being imported during startup.


New commits:

a7976cbremoved ComplexReflectionGroups from global namespace
d4775c7Removing FiniteCoxeterGroups, ComplexReflectionGroups, and GeneralizedWeylGroups from startup import.

comment:160 Changed 13 months ago by git

  • Commit changed from d4775c7541edc49cc4aaace14edeec15255db700 to beadf8a46bccffc7934b1fd15d607561be937642

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

beadf8aLazy imports must have absolute paths.

comment:161 Changed 13 months ago by git

  • Commit changed from beadf8a46bccffc7934b1fd15d607561be937642 to 185be8f64c6f3d265615b3773c1ef27378a72d2a

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

f7a6dd1Fixing global namespace issues.
a53d8f1added somes lines in doc + correction typo shephard
4e71ef7Some typos and corrections in the descriptions
535d37eremoved spaces in example
185be8fMerge branch 'public/categories/complex_reflection_groups-20397' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187

comment:162 Changed 13 months ago by git

  • Commit changed from 185be8f64c6f3d265615b3773c1ef27378a72d2a to b81dc4216eeba5b4d4efa4929f8d2a10c05f7da2

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

6dc2fdbtrac #11187 doc correction, and more doc
b81dc42Fixing simple_reflection_orders.

comment:163 Changed 13 months ago by chapoton

  • Branch changed from u/tscrim/11187 to u/chapoton/11187
  • Commit changed from b81dc4216eeba5b4d4efa4929f8d2a10c05f7da2 to 2d70fabf62f9e613ffc9bf488e0dd3e9698ce02c

some tiny fixes in doc, mainly


New commits:

2b0c33atrac #11187 replace exec by sage_eval, add more doc on reflection groups
7271eadtrac #11187 fixing the doc (again)
298cf98trac #1118è adding one doc
dffa31etrac #11187 some more fine doc tuning
2ec03b0trac #11187 working on doc
7324754trac #11187 still working on doc
2d70fabtrac #11187 more doc tuning

comment:164 Changed 13 months ago by stumpc5

in the cython iterator, we should use sth like

tuple( W.simple_reflection(W._index_set_inverse[i]) for i in xrange(len(W._index_set)) )

comment:165 Changed 13 months ago by tscrim

  • Branch changed from u/chapoton/11187 to u/tscrim/11187
  • Commit changed from 2d70fabf62f9e613ffc9bf488e0dd3e9698ce02c to 5b1f2b0353e134ee7413a9b3dee3ffa3347ed7ae

New commits:

0bcce62Making the iterator even faster.
a04b9c8Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
9c96d33added ST type casting for real groups + documentation of crg's
5b1f2b0Merge branch 'u/stumpc5/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187

comment:166 Changed 13 months ago by git

  • Commit changed from 5b1f2b0353e134ee7413a9b3dee3ffa3347ed7ae to d4256691756e16c64b6f2f855367f5afb911a00c

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

aa07666trac #11187 fixing catalog
b2ba34cMerge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
fa1b496Changing the indexing sets and how they are handled.
a1b61e6moderate pep8 changes
6da6f94Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
85d0ce1fixed a typo in the doc
ab7afbamarked a missing doctest + fixed a bug to show type C2
72082b1fixed a bug in noncrossing partition lattice and reflection/absolute order
844f450Merge branch 'u/stumpc5/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
d425669Fixing failures and some other things.

comment:167 Changed 13 months ago by nthiery

  • Branch changed from u/tscrim/11187 to u/nthiery/11187

comment:168 Changed 13 months ago by stumpc5

  • Branch changed from u/nthiery/11187 to u/stumpc5/11187
  • Commit changed from d4256691756e16c64b6f2f855367f5afb911a00c to e074050131cd483022c5783f749cbce3ac522ae5

New commits:

87cd989fixed one doctest in category complex reflection group
05a8ab411187: fixed trivial doctest failures
32bf1ae11187: cleanup of the organization of the various axioms (WellGenerated, ...) for complex/generalized reflection groups + documentation improvements
73276caMerge branch 'u/tscrim/11187' of trac.sagemath.org:sage into t/11187/11187
e074050merged

comment:169 Changed 13 months ago by git

  • Commit changed from e074050131cd483022c5783f749cbce3ac522ae5 to b572c74c3a4db2909bea5c538c8be26089068fc6

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

b572c74fixed a missing doctest

comment:170 Changed 13 months ago by stumpc5

Only two trivial doctests failing + the missing doctest for _test_well_generated, though Nicolas thought he had added it.

comment:171 Changed 13 months ago by nthiery

  • Branch changed from u/stumpc5/11187 to u/nthiery/11187

comment:172 Changed 13 months ago by git

  • Commit changed from b572c74c3a4db2909bea5c538c8be26089068fc6 to 4e4e65ed2613a4486b34219f66a34ba40183cf58

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

24191b011187: factored out common code from GeneralizedCoxeterGroups and ComplexReflectionGroups
fced757trac #11187 fixing import of QQbar and prod
1daeaeatrac #11187 fixing degrees for finite Coxeter and Weyl
2791d1eMerge branch 'u/chapoton/11187' of trac.sagemath.org:sage into t/11187/11187
4e4e65e11187: finished the generic irreducible_components; fixed trivial doctest failures

comment:173 Changed 13 months ago by git

  • Commit changed from 4e4e65ed2613a4486b34219f66a34ba40183cf58 to 6d88d923ee9bbf2a86c223f4aec60ddad4e2eb6f

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

6d88d9211187: oops, forgot to add one file

comment:174 Changed 13 months ago by nthiery

  • Branch changed from u/nthiery/11187 to public/11187

comment:175 Changed 13 months ago by git

  • Commit changed from 6d88d923ee9bbf2a86c223f4aec60ddad4e2eb6f to 9f72be34c793f93cb0cc48f03ad7b2849ffabcad

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

9f72be3Moving the iterator for complex reflection groups to the Cython file.

comment:176 Changed 13 months ago by git

  • Commit changed from 9f72be34c793f93cb0cc48f03ad7b2849ffabcad to ef835f68696526ba2a9dff934b65e45a1f095219

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

833b550trac #11187 better code for degrees
ef835f6Merge branch 'public/11187' into chapoton branch

comment:177 Changed 13 months ago by git

  • Commit changed from ef835f68696526ba2a9dff934b65e45a1f095219 to a45d823a6544c48bdb903b143338e3d5fbbccd36

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

a45d823fixed the allowed input types for reflection groups

comment:178 Changed 13 months ago by git

  • Commit changed from a45d823a6544c48bdb903b143338e3d5fbbccd36 to a5e263dba2601bb006e2b7510d35c0f656603a8c

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

a9d858a11187: documentation, crosslinks, full subcategories, uniformization of simple reflections/reflections/distinguished reflections
211baa4Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
a5e263dMerge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187

comment:179 Changed 13 months ago by git

  • Commit changed from a5e263dba2601bb006e2b7510d35c0f656603a8c to a95a5a91f8c164cc863df9103665d90c056e7624

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

a95a5a911187: updated doctest + added comment for the next time

comment:180 Changed 13 months ago by git

  • Commit changed from a95a5a91f8c164cc863df9103665d90c056e7624 to e654c3ed7751689b839547dec558fc9e375716a7

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

e654c3e11187: updated doctests: Coxeter groups don't define additional structure since they have now a supercategory doing so

comment:181 Changed 13 months ago by git

  • Commit changed from e654c3ed7751689b839547dec558fc9e375716a7 to c3b5a320e409080e967f44b4d4aa30715b06abfe

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

c3b5a32fixed a bug in the degrees in coxeter groups category

comment:182 Changed 13 months ago by git

  • Commit changed from c3b5a320e409080e967f44b4d4aa30715b06abfe to b5b8c3b2103a53cc49060ddb584fcd8048e5deab

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

b5b8c3btrac #11187 adding codegrees to finite Coxeter groups

comment:183 Changed 13 months ago by git

  • Commit changed from b5b8c3b2103a53cc49060ddb584fcd8048e5deab to 7eab06d2a6f8af4e837342b695c7d479fe140e55

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

7eab06dtrac #11187 some typos in doc of categories/complex_reflection_group

comment:184 Changed 13 months ago by git

  • Commit changed from 7eab06d2a6f8af4e837342b695c7d479fe140e55 to fae677f17b4af97addae261a4b5882ddd99a19d1

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

98f3c7611187: fixed TODO statement, removed unused import
d43a39f11187: removed commented out link for some_elements
e34c9a711187: refactor from_reduced_word and apply_*_reflection, cleanup
8e08db2Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
fae677f11187: removed 3 spurious empty lines in the doc

comment:185 Changed 13 months ago by git

  • Commit changed from fae677f17b4af97addae261a4b5882ddd99a19d1 to f048ac3d8b012474f16174ded8a23b5143db9ea0

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

49f1897trac #11187 implement coxeter_element and correct the degrees for symmetric gp
f048ac3Merge branch 'public/11187' into chapoton branch

comment:186 Changed 13 months ago by git

  • Commit changed from f048ac3d8b012474f16174ded8a23b5143db9ea0 to 7880f309bc060c42fcbdfa8e276011ce0ce88c22

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

7880f30trac #11187 one failing doctest in categories

comment:187 Changed 13 months ago by git

  • Commit changed from 7880f309bc060c42fcbdfa8e276011ce0ce88c22 to 9b7aba28093f4f811acecc67264cb011f029f299

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

568d6c411187: Generalized methods from ComplexReflectionGroup, replace apply_simple_reflection_* by a definition of simple_reflection, documentation, TODOs
9b7aba2Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187

comment:188 Changed 13 months ago by git

  • Commit changed from 9b7aba28093f4f811acecc67264cb011f029f299 to fc5f6671bf504b8a6ee073b9e7d9aba888220b03

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

fc5f667removed several cached methods

comment:189 Changed 13 months ago by git

  • Commit changed from fc5f6671bf504b8a6ee073b9e7d9aba888220b03 to b55ecb03098c873ac669c5f185c319b0d42fbbca

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

b55ecb0moved the reduced_word_in_ref for real groups there, removed a few more cached methods, documented a few optional arguments

comment:190 Changed 13 months ago by git

  • Commit changed from b55ecb03098c873ac669c5f185c319b0d42fbbca to 2aa89b2f08edb3a5351b610f56ef9a40196f9076

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

2aa89b2commenting on the todo list

comment:191 Changed 13 months ago by git

  • Commit changed from 2aa89b2f08edb3a5351b610f56ef9a40196f9076 to a7d65c6893b0026f2a680fc38fb51168136b0824

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

a7d65c6fixed one failing doctest

comment:192 Changed 13 months ago by git

  • Commit changed from a7d65c6893b0026f2a680fc38fb51168136b0824 to eb45b27d0c5e184572a7719ea22b86c65211349a

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

410d03d11187: TODO + marked indirect doctest
eb45b27Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187

comment:193 Changed 13 months ago by git

  • Commit changed from eb45b27d0c5e184572a7719ea22b86c65211349a to f4801f2edbfcf87d1b1302a406945afaae6dbb37

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

f4801f2trac 11187 making degrees be integers

comment:194 Changed 13 months ago by git

  • Commit changed from f4801f2edbfcf87d1b1302a406945afaae6dbb37 to cb0a7a0509d90826d120ae17f108d3ab15ef9b02

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

cb0a7a0trac 11187 degrees in tuple

comment:195 Changed 13 months ago by git

  • Commit changed from cb0a7a0509d90826d120ae17f108d3ab15ef9b02 to 55104aac8bac1952f924ae0198a7c79a7682ee0f

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

883f3d6sorted degrees in irred comps, sort also the fundamental invariants in the same way
55104aaMerge branch 'public/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187

comment:196 Changed 13 months ago by git

  • Commit changed from 55104aac8bac1952f924ae0198a7c79a7682ee0f to fbdabdc58bb28e12f36a8251df683d2b496df619

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

5775f50trac 11187 fixing doctest in degree, codegree, fundamental inv
fbdabdctrac 11187 codegrees in decreasing order

comment:197 Changed 13 months ago by git

  • Commit changed from fbdabdc58bb28e12f36a8251df683d2b496df619 to e22ee7ecb32cd5914f6dbb69593d240d4de1e9eb

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

854256211187: add _test_degrees (not yet finished)
256edb7Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
b29b70211187: reflecting_hyperplanes -> reflection_hyperplanes
ebbf9bf11187: further work on _test_(co)_degrees; reflecting_hyperplanes -> reflection_hyperplanes
249f619Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
e22ee7e11187: fixed typo in the code of number_of_irreducible_components

comment:198 Changed 13 months ago by git

  • Commit changed from e22ee7ecb32cd5914f6dbb69593d240d4de1e9eb to 722a7675cec4cb22aab3ec594fca457c8ec2da57

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

0e0f7b311187: fixed degrees for non irreducible finite coxeter groups; removed the constraints that the (co)degrees shall be sorted
722a76711187: fixed degrees for more specific coxeter group implementations + doctest cleanup

comment:199 Changed 13 months ago by git

  • Commit changed from 722a7675cec4cb22aab3ec594fca457c8ec2da57 to 5294cf3cf5dcfd92d95f606697a45abc9844405b

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

5294cf311187: fixed codegrees for colored permutations

comment:200 Changed 13 months ago by git

  • Commit changed from 5294cf3cf5dcfd92d95f606697a45abc9844405b to c7425f8e27bff3d13e05bd9d9a880e14817ccee9

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

c7425f8trac 11187 a typo in doc

comment:201 Changed 13 months ago by git

  • Commit changed from c7425f8e27bff3d13e05bd9d9a880e14817ccee9 to 2def1e83ba323899fa1a95391a1b17a0bda4140c

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

d41167311187: fixed degrees and codegrees for Permutations to return tuple of Integers
2def1e8Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187

comment:202 Changed 13 months ago by git

  • Commit changed from 2def1e83ba323899fa1a95391a1b17a0bda4140c to 241e6607cf62f3938019d5118aac018e9254dc24

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

241e66011187: sum -> ZZ.sum for consistency when the list of degrees is empty

comment:203 Changed 13 months ago by git

  • Commit changed from 241e6607cf62f3938019d5118aac018e9254dc24 to 82ff2882bcdee03518224a3b63a0f2ebd1dd33b3

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

82ff28811187: rename 'reflecting hyperplane' to 'reflection hyperplane' in the docstrings

comment:204 Changed 13 months ago by git

  • Commit changed from 82ff2882bcdee03518224a3b63a0f2ebd1dd33b3 to c09cd4d5e8f61909a57b74dfa09627f6aef6f130

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

c09cd4d11187: Fixed heisenbug caused by temporarily breaking the implementation of (co)degrees for testing without restoring it after

comment:205 Changed 13 months ago by git

  • Commit changed from c09cd4d5e8f61909a57b74dfa09627f6aef6f130 to 0becba3698653fedcfa8547d44fc7d45fb2fecb3

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

cf00014trac #20396 adding the invariant linear form for complex refl groups
75e8d69trac #20396 first try at simpler implementation
cdef4c3trac #20396 not yet working
8013693Merge branch 'public/20396' of git://trac.sagemath.org/sage into u/stumpc5/20396
e02cf02still in trouble about the complex case
07bd053merged 11187
bf2ea92fixed the coroots + work on invariant form
0becba3Merge branch 'public/11187' of git://trac.sagemath.org/sage into u/stumpc5/20396

comment:206 Changed 13 months ago by git

  • Commit changed from 0becba3698653fedcfa8547d44fc7d45fb2fecb3 to 8927ab98d9aa88610774567e4bde0ce8ec8c14b9

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

8927ab9matrix action is on the right!

comment:207 Changed 13 months ago by stumpc5

Okay, all tests pass!

comment:208 Changed 13 months ago by stumpc5

@trscim: For the record, the timings for iteration in E7 seem to be odd:

sage: W = ReflectionGroup(['E',7])                                              
sage: %time for w in W.iteration(algorithm="depth",tracking_words=False): pass  
CPU times: user 3.71 s, sys: 0 ns, total: 3.71 s
Wall time: 3.79 s
sage: %time for w in W.iteration(algorithm="breadth",tracking_words=False): pass
CPU times: user 8.72 s, sys: 4.84 ms, total: 8.73 s
Wall time: 8.72 s
sage: %time for w in W.iteration(algorithm="depth",tracking_words=True): pass   
CPU times: user 5.83 s, sys: 0 ns, total: 5.83 s
Wall time: 5.81 s
sage: %time for w in W.iteration(algorithm="breadth",tracking_words=True): pass 
CPU times: user 20.6 s, sys: 67.4 ms, total: 20.6 s
Wall time: 20.7 s

Also, I discussed with Nicolas yesterday, and (as what I said on the weekend), we are back at the situation that one can be much smarter concerning the ascents: Let i be an ascent of w and si and s_j commute. Then i is also an ascent of wsj. I will try to use that today, but it still seems that any test in this direction is much slower than doing the actual test for an ascent the way we already do it.

comment:209 Changed 13 months ago by git

  • Commit changed from 8927ab98d9aa88610774567e4bde0ce8ec8c14b9 to def701588736edda81a86ddd1a9cbdbc849657be

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

eb2ea75cleaned the reduced word code
6fbba18undo the change to repr methods
8497f5dfirst try to skip elements in the iteration, too slow
def7015reorganized the invariant form (still missing an improvement) + fixed two doctests

comment:210 follow-up: Changed 13 months ago by stumpc5

Travis, could you have a look at the diffset 8497f5d; I commented out that the change is actually used, see #for i in self.noncom[first], it seems to slow down the test even though much less is actually tested. Maybe you see how to get rid of the overhead the line for i in self.noncom[first] has compared to for i in range(first+1,self.n).

comment:211 follow-up: Changed 13 months ago by stumpc5

Travis: Also, you introduced _reduced_word to be a lazy attribute. This has a few drawbacks: not self._reduced_word is None in the length function in reflection_group_real.py is never triggered, and _reduced_word is now either a word in the index set (if the lazy attribute is triggered in the real case), or a word in range(n) if one has gotten it through the iterator. This way, the method reduced_word in the real case is always executed, even though the reduced word is already known.

Two solutions are:

  • either set _reduced_word to a word in the indices in the iterator for the cost of list lookups for each letter in the word
  • write the lazy attribute in the real case so that it gives a word in range(n)
  • do neither and forget about the info gotten from the iterator in the real case.

comment:212 in reply to: ↑ 210 Changed 13 months ago by tscrim

Replying to stumpc5:

Travis, could you have a look at the diffset 8497f5d; I commented out that the change is actually used, see #for i in self.noncom[first], it seems to slow down the test even though much less is actually tested. Maybe you see how to get rid of the overhead the line for i in self.noncom[first] has compared to for i in range(first+1,self.n).

My guess is because Cython is smart enough to translate

for i in range(n)

into

for(i=0; i<n; ++i)

whereas for the other, it is iterating over some object (which, a priori, may not even be iterable). In more detail, Cython doesn't even know that it is iterating over a list, just that self.noncom is. Try:

        nc = self.noncom[first]
        for i in nc:

or the variant

        nc = self.noncom[first]
        k = len(nc)
        for dummy in range(k):
            i = nc[dummy]

where you have cdef list nc and cdef int dummy, k (although I suspect the former of these is faster).

comment:213 in reply to: ↑ 211 Changed 13 months ago by tscrim

Replying to stumpc5:

Travis: Also, you introduced _reduced_word to be a lazy attribute. This has a few drawbacks: not self._reduced_word is None in the length function in reflection_group_real.py is never triggered, and _reduced_word is now either a word in the index set (if the lazy attribute is triggered in the real case), or a word in range(n) if one has gotten it through the iterator. This way, the method reduced_word in the real case is always executed, even though the reduced word is already known.

Doing this simplified the code and probably got us a small speedup. Yet, this is a bug in my implementation. However, I'm not sure how much it is worth it to compute the length of an arbitrary element without computing its reduced word. If it becomes that time critical for someone, we can get them an algorithm option for length().

This way, the method reduced_word in the real case is always executed, even though the reduced word is already known.

This doesn't make sense. If anything reduced_word() will throw an error if _reduced_word was done by the lazy_attribute. The iterator overwrites (the lazy attribute) _reduced_word if the word comes from there, so it does not get computed twice. Otherwise it was a user created word and will never be identical (not just equals) to an element created from the iterator.

Two solutions are:

  • write the lazy attribute in the real case so that it gives a word in range(n)

This is what I would do. Although we might be better off just doing our own implementation using a list (which we mutate) and starting with self.domain().

Last edited 13 months ago by tscrim (previous) (diff)

comment:214 Changed 13 months ago by git

  • Commit changed from def701588736edda81a86ddd1a9cbdbc849657be to 5a377c42bdcba53fde3504489eab396194bc509f

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

7ce6850playing with the iterator, nothing makes it faster currently...
5a377c4moved reduced word to cython + fixing doctests, added doctests for invariant form brute force

comment:215 Changed 13 months ago by stumpc5

Done with "write the lazy attribute in the real case so that it gives a word in range(n)". This is now moved to cython (speed gain ~100 times faster).

comment:216 follow-up: Changed 13 months ago by stumpc5

One big issue that is still waiting for me to fix it is that it remains unclear if we want a left or right action. The implementation is a left action, but e.g. #20402 assumes a right action. I will get to this before the weekend and would then hope for the code to become stable and some final reviewing :-)

comment:217 Changed 13 months ago by tscrim

Just FYI - you have reverted a python3 compatible change with this:

-                ....:     print('%s %s'%(w.reduced_word(), w.is_regular(W.coxeter_number())))
+                ....:     print w.reduced_word(), w.is_regular(h)

I think 2to3 can handle this, but I'm just stating a paranoia about it.

Although I would advocate for making doctests that avoid printing through an iteration altogether (really annoying failures when the iteration order changes), but this isn't really a big issue.

I will take a more detailed look at the recent changes before the weekend.

Last edited 13 months ago by tscrim (previous) (diff)

comment:218 in reply to: ↑ 216 Changed 13 months ago by nthiery

Replying to stumpc5:

One big issue that is still waiting for me to fix it is that it remains unclear if we want a left or right action. The implementation is a left action, but e.g. #20402 assumes a right action. I will get to this before the weekend and would then hope for the code to become stable and some final reviewing :-)

How much additional code would it take to make it work on both sides, allowing the user to do ReflectionGroup(..., action="right")?

comment:219 follow-ups: Changed 13 months ago by stumpc5

Just FYI - you have reverted a python3 compatible change with this

I am sure there are reasons for that python3 convention, but I do prefer simple to read code. Anyway, I can also do the other if you prefer.

How much additional code would it take to make it work on both sides,

I was afraid you'd ask that. in my eyes: too much. The elements are considered as permutations, words, and matrices at the same time (and there are even two types of words, in simples and in all reflections, and the simple words have wired properties in the non-real case), making options would pollute the code just soo much, I think, and would introduce so many possible mistakes. I prefer to have clean and fast code, that is hopefully getting better than chevie, eventually. Once that is achieved, I am getting in favour with adding another layer of user friendliness.

comment:220 in reply to: ↑ 219 Changed 13 months ago by nthiery

Replying to stumpc5:

I was afraid you'd ask that.

:-)

in my eyes: too much. The elements are considered as permutations, words, and matrices at the same time (and there are even two types of words, in simples and in all reflections, and the simple words have wired properties in the non-real case), making options would pollute the code just soo much, I think, and would introduce so many possible mistakes. I prefer to have clean and fast code, that is hopefully getting better than chevie, eventually. Once that is achieved, I am getting in favour with adding another layer of user friendliness.

Fair enough: let's focus on getting things to work.

One thing though is that a lot of features are completely symmetric (e.g. the reverse of a word is a word). The only spot where things differ is when there is an explicit action. In my experience when implementing representations of semigroups, it was often possible to abstracted everything out by manipulating elements in the code as functions whenever there is an action involved (sorry, I know, this is a big vague).

Of course, a prerequisite for implementing this seamlessly is that permutation groups themselves would allow for both left and right actions.

Cheers,

Nicolas

comment:221 Changed 13 months ago by git

  • Commit changed from 5a377c42bdcba53fde3504489eab396194bc509f to 8862849a09e7f9ecadbb181e5051edf1e9924b73

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

8862849simple_roots, simple_coroots, and fundamental_weights are now index by the index set

comment:222 in reply to: ↑ 219 ; follow-up: Changed 13 months ago by tscrim

Replying to stumpc5:

Just FYI - you have reverted a python3 compatible change with this

I am sure there are reasons for that python3 convention, but I do prefer simple to read code. Anyway, I can also do the other if you prefer.

In python3, print becomes an actual function, so it needs the parentheses (which due to Python's handling of parentheses, print(foo) gets evaluated to print foo, but having a comma means (foo, bar) becomes a tuple). So to help make the doctests 2/3 compatible and to ease the eventual transition, I do prefer the way I had it or make it into a few examples and have a doctest which does check everything (without verbose).

Last edited 13 months ago by tscrim (previous) (diff)

comment:223 Changed 13 months ago by git

  • Commit changed from 8862849a09e7f9ecadbb181e5051edf1e9924b73 to ef83323a827a477c3c7330fedb429e4dd737e044

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

ef83323clarified the fundamental weights, including an example

comment:224 in reply to: ↑ 222 Changed 13 months ago by stumpc5

Replying to tscrim:

In python3, print becomes an actual function, so it needs the parentheses

I see, doesn't make it easier to read though...

comment:225 Changed 13 months ago by git

  • Commit changed from ef83323a827a477c3c7330fedb429e4dd737e044 to 5ecc970883b3ed30314b35949956a7eba135b8ff

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

5ecc970got the action on roots and the inversion set in its left- and right-flavor

comment:226 Changed 13 months ago by git

  • Commit changed from 5ecc970883b3ed30314b35949956a7eba135b8ff to 42d6b753c5c5cb4e1e82cf0ed3520768c5dc5d6b

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

42d6b75made matrix an alias for to_matrix, as matrix returns a huge permutation matrix of the roots

comment:227 Changed 13 months ago by git

  • Commit changed from 42d6b753c5c5cb4e1e82cf0ed3520768c5dc5d6b to 8810c41ad280c06b307f059220c03f046eafa0d7

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

8810c41fixed one doctest + work on the invariant form

comment:228 Changed 13 months ago by git

  • Commit changed from 8810c41ad280c06b307f059220c03f046eafa0d7 to 2ba374053a128ead4f7288b8b302a2752211a870

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

2ba3740fixed another doctest + a typo

comment:229 Changed 13 months ago by git

  • Commit changed from 2ba374053a128ead4f7288b8b302a2752211a870 to e28bbecbd3a0b08b35f8892874dcd4d7d7f61e4e

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

e28bbecedited the length function to always use the reduced word

comment:230 Changed 13 months ago by git

  • Commit changed from e28bbecbd3a0b08b35f8892874dcd4d7d7f61e4e to dc0106628d58cb263866b9cd601696ea71f18e9f

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

a2ef2e4added a few sentences to the doc
dc01066started to add 'optional - gap3'

comment:231 Changed 13 months ago by git

  • Commit changed from dc0106628d58cb263866b9cd601696ea71f18e9f to fd0c9cc616a03de20a12aba530c96e73f8f60911

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

fd0c9ccadded optional gap3

comment:232 Changed 13 months ago by git

  • Commit changed from fd0c9cc616a03de20a12aba530c96e73f8f60911 to 1537a795f6d7a86f8c3a7a79e86d4b31405de323

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

1537a79added optional gap3

comment:233 Changed 13 months ago by git

  • Commit changed from 1537a795f6d7a86f8c3a7a79e86d4b31405de323 to a60cf31d47777353bcae92665d5e5e739fc02a29

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

a60cf31added some missing optional gap3

comment:234 Changed 13 months ago by git

  • Commit changed from a60cf31d47777353bcae92665d5e5e739fc02a29 to 4955d68d9e4dbde2d1425afbae36e619c2d6ba14

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

feebf97finalized the invariant form
4955d68added the missing optional gap3 in the cython file

comment:235 Changed 12 months ago by git

  • Commit changed from 4955d68d9e4dbde2d1425afbae36e619c2d6ba14 to c5c43bcba449039892cecb0d18238b4bf87e8a7f

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

c5c43bcthe next round of optional gap3 insertions

comment:236 Changed 12 months ago by stumpc5

  • Dependencies #20107 deleted
  • Description modified (diff)
  • Keywords days64.5 days80 added; days 64.5 removed
  • Status changed from needs_work to needs_review

comment:237 Changed 12 months ago by stumpc5

  • Description modified (diff)

comment:238 Changed 12 months ago by git

  • Commit changed from c5c43bcba449039892cecb0d18238b4bf87e8a7f to f541349eae7b8b3b6a61c57aaafd6bb8c84b3265

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

244e6ffMerge branch 'public/11187' of trac.sagemath.org:sage into public/reflection_groups-11187
f541349Some last little beautification.

comment:239 Changed 12 months ago by tscrim

I've done a few little beautification things. If my changes are good, then we can set this to a positive review.

comment:240 Changed 12 months ago by stumpc5

Thanks Travis, looking forward to have this to positive review and then merged!

comment:241 follow-up: Changed 12 months ago by tscrim

FYI - Patchbots seem happy with it.

comment:242 in reply to: ↑ 241 Changed 12 months ago by stumpc5

FYI - Patchbots seem happy with it.

yes, but it didn't test the gap3 stuff again (I just asked Frederic how to get the patchbot to also test those), and also we still have two new startup modules (though the patchbot sees them without indicating it, see http://patchbot.sagemath.org/log/11187/Ubuntu/14.04/x86_64/3.13.0-85-generic/librae/2016-04-20%2010:00:02?plugin=plugins.startup_modules&diff=/log/0/Ubuntu/14.04/x86_64/3.13.0-85-generic/librae/2016-04-12%2018%3A16%3A59&ticket=11187&base=7.2.beta4)

comment:243 follow-up: Changed 12 months ago by stumpc5

To now run the doctests # optional - gap3, it seems I cannot use the patchbot anymore. Should I then do instead

sage-patchbot/sage -tp 4 --all --long --optional=gap3

myself?

comment:244 Changed 12 months ago by tscrim

I think that will be best. I was going to try this afternoon to see if I can avoid importing those categories at startup, but I might not be successful at it.

comment:245 in reply to: ↑ 243 ; follow-up: Changed 12 months ago by dimpase

Replying to stumpc5:

To now run the doctests # optional - gap3, it seems I cannot use the patchbot anymore. Should I then do instead

sage-patchbot/sage -tp 4 --all --long --optional=gap3

myself?

I'd use --optional=gap3,sage

comment:246 in reply to: ↑ 245 Changed 12 months ago by stumpc5

Replying to dimpase:

I'd use --optional=gap3,sage

wow, you are silently reading these hundreds of comments to now help out -- impressive, thanks!

comment:247 Changed 12 months ago by tscrim

To remove generalized_coxeter_groups.py from the startup would either require making it a lazy import in coxeter_groups.py or making coxeter_groups.py not being imported at startup. I think the better strategy is to leave things how they are as an incentive to have coxeter_groups.py not be imported at startup. (Also it is only a net +1 imported at startup.)

comment:248 Changed 12 months ago by git

  • Commit changed from f541349eae7b8b3b6a61c57aaafd6bb8c84b3265 to 0db28d6c6e64818eb32fb9826ff7a41333c814d6

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

0db28d6fixed doctests that have not been run last week!

comment:249 Changed 12 months ago by stumpc5

I also copied the other optional from the patchbot:

./sage -tp 6 --all --long --optional=ccache,gap3,mpir,patchbot,python2,sage

@Frederic, is that what I should do? Would it be hard (desirable?) to tell the patchbot (maybe in config_file.json) which optional tests should in general be run?

comment:250 Changed 12 months ago by stumpc5

there are still a few doctest failures that need to be fixed:

stumpc5@findstat:~/web-findstat/sage-patchbot$ ./sage -tp 6 --long --optional=ccache,gap3,mpir,patchbot,python2,sage src/sage/combinat/root_system/coxeter_group.py
too many failed tests, not using stored timings
Running doctests with ID 2016-04-21-13-13-55-f420497c.
Git branch: t/11187/public/11187
Using --optional=ccache,gap3,mpir,patchbot,python2,sage
Doctesting 1 file using 6 threads.
sage -t --long src/sage/combinat/root_system/coxeter_group.py
**********************************************************************
File "src/sage/combinat/root_system/coxeter_group.py", line 205, in sage.combinat.root_system.coxeter_group.CoxeterGroupAsPermutationGroup.__init__
Failed example:
    TestSuite(W).run()             # optional - gap3
Expected nothing
Got:
    Failure in _test_well_generated:
    Traceback (most recent call last):
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/misc/sage_unittest.py", line 283, in run
        test_method(tester = tester)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/categories/finite_complex_reflection_groups.py", line 628, in _test_well_generated
        tester.assertEqual(self.number_of_simple_reflections(), self.rank())
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python/unittest/case.py", line 515, in assertEqual
        assertion_func(first, second, msg=msg)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python/unittest/case.py", line 508, in _baseAssertEqual
        raise self.failureException(msg)
    AssertionError: 3 != 27
    ------------------------------------------------------------
    The following tests failed: _test_well_generated
**********************************************************************
File "src/sage/combinat/root_system/coxeter_group.py", line 231, in sage.combinat.root_system.coxeter_group.CoxeterGroupAsPermutationGroup._element_class
Failed example:
    W._element_class() is W.element_class                      # optional - gap3
Exception raised:
    Traceback (most recent call last):
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.root_system.coxeter_group.CoxeterGroupAsPermutationGroup._element_class[1]>", line 1, in <module>
        W._element_class() is W.element_class                      # optional - gap3
      File "sage/structure/parent.pyx", line 854, in sage.structure.parent.Parent.__getattr__ (/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/src/build/cythonized/sage/structure/parent.c:8083)
        attr = getattr_from_other_class(self, self._category.parent_class, name)
      File "sage/structure/misc.pyx", line 253, in sage.structure.misc.getattr_from_other_class (/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/src/build/cythonized/sage/structure/misc.c:1763)
        raise dummy_attribute_error
    AttributeError: 'CoxeterMatrixGroup_with_category' object has no attribute '_element_class'
**********************************************************************
2 items had failures:
   1 of   4 in sage.combinat.root_system.coxeter_group.CoxeterGroupAsPermutationGroup.__init__
   1 of   3 in sage.combinat.root_system.coxeter_group.CoxeterGroupAsPermutationGroup._element_class
    [54 tests, 2 failures, 10.25 s]
----------------------------------------------------------------------
sage -t --long src/sage/combinat/root_system/coxeter_group.py  # 2 doctests failed

comment:251 Changed 12 months ago by stumpc5

And then, there are also a few in gap3.py, I am not sure if we have to treat them, but if anyone knows how to do it, please go ahead (this is also necessary to get the package to be optional rather than experimental...

stumpc5@findstat:~/web-findstat/sage-patchbot$ ./sage -tp 6 --long --optional=ccache,gap3,mpir,patchbot,python2,sage src/sage/interfaces/gap3.py
too many failed tests, not using stored timings
Running doctests with ID 2016-04-21-13-15-35-52e24ead.
Git branch: t/11187/public/11187
Using --optional=ccache,gap3,mpir,patchbot,python2,sage
Doctesting 1 file using 6 threads.
sage -t --long src/sage/interfaces/gap3.py
**********************************************************************
File "src/sage/interfaces/gap3.py", line 393, in sage.interfaces.gap3.Gap3._execute_line
Failed example:
    f([1,2,3])                                   #optional - gap3
Expected:
    Traceback (most recent call last):
    ...
    RuntimeError: Gap3 produced error output   
    Error, List Element: <position> must be a positive integer at
    return L[0] ... in
    ... called from
    main loop
    brk> quit;
    ...
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.gap3.Gap3._execute_line[3]>", line 1, in <module>
        f([Integer(1),Integer(2),Integer(3)])                                   #optional - gap3
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 805, in __call__
        return getattr(P, self.name())(*args)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 607, in __call__
        return self._parent.function_call(self._name, list(args), kwds)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/interfaces/gap.py", line 921, in function_call
        res = self.eval(marker+cmd)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/interfaces/gap.py", line 573, in eval
        result = Expect.eval(self, input_line, **kwds)
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 1239, in eval
        for L in code.split('\n') if L != ''])
      File "/srv/data/findstat.imp.fu-berlin.de/sage-patchbot/local/lib/python2.7/site-packages/sage/interfaces/gap.py", line 771, in _eval_line
        raise RuntimeError(message)
    RuntimeError: Gap3 produced error output
    Error, Function: <function> must be a function
    <BLANKLINE>
       executing __SAGE_LAST__:="__SAGE_LAST__";;x(sage0);;
**********************************************************************
File "src/sage/interfaces/gap3.py", line 440, in sage.interfaces.gap3.Gap3.help
Failed example:
    m                                        #optional - gap3
Expected:
    [ [ 1, 2, 3 ], [ 4, 5, 6 ] ]
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/interfaces/gap3.py", line 442, in sage.interfaces.gap3.Gap3.help
Failed example:
    m.Print()                                #optional - gap3
Expected:
    [ [ 1, 2, 3 ], [ 4, 5, 6 ] ]
Got:
    "__SAGE_LAST__"
**********************************************************************
File "src/sage/interfaces/gap3.py", line 446, in sage.interfaces.gap3.Gap3.help
Failed example:
    m                                        #optional - gap3
Expected:
    [ [ 1, 2, 3 ], [ 4, 5, 6 ] ]
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/interfaces/gap3.py", line 448, in sage.interfaces.gap3.Gap3.help
Failed example:
    m.Print()                                #optional - gap3
Expected:
    [ [ 1, 2, 3 ], [ 4, 5, 6 ] ]
Got:
    "__SAGE_LAST__"
**********************************************************************
2 items had failures:
   1 of   5 in sage.interfaces.gap3.Gap3._execute_line
   4 of  10 in sage.interfaces.gap3.Gap3.help
    [108 tests, 5 failures, 7.01 s]
----------------------------------------------------------------------
sage -t --long src/sage/interfaces/gap3.py  # 5 doctests failed
----------------------------------------------------------------------
Total time for all tests: 15.7 seconds
    cpu time: 2.2 seconds
    cumulative wall time: 7.0 seconds

}}}

comment:252 Changed 12 months ago by git

  • Commit changed from 0db28d6c6e64818eb32fb9826ff7a41333c814d6 to 7afc02c78f5c0aa58d5b638e20c135678a3bec7e

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

7afc02cFixing doctests in combinat/root_system/coxeter_group.py.

comment:253 follow-up: Changed 12 months ago by chapoton

not a good idea in general to use a bare except:

except:

rather find what exception it is supposed to catch

comment:254 Changed 12 months ago by tscrim

I fixed the errors in combinat/root_system/coxeter_group.py (to which, we should be able to do most (all) of that functionality without appealing to GAP3 and we should probably fold that into (Irreducible)RealReflectionGroup at some point). The latter errors are indicating issues with the interface, so I would just wait on those until we do #20393. Since gap3 is an experimental package, we are not strictly required to have all # optional - gap3 doctests pass.

comment:255 Changed 12 months ago by git

  • Commit changed from 7afc02c78f5c0aa58d5b638e20c135678a3bec7e to 02694256c73520e7f4dd80c455cdae133be858a7

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

0269425Blanket fix bare except block.

comment:256 in reply to: ↑ 253 ; follow-up: Changed 12 months ago by tscrim

Replying to chapoton:

not a good idea in general to use a bare except:

except:

rather find what exception it is supposed to catch

Right, good catch. I'm going to try and find the precise error that gets raised, but I've pushed a hack fix for now.

comment:257 Changed 12 months ago by git

  • Commit changed from 02694256c73520e7f4dd80c455cdae133be858a7 to 9b0ef927c17789f2dbeea432b8316f99d138cb88

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

9b0ef92Making it an AttributeError.

comment:258 in reply to: ↑ 256 Changed 12 months ago by tscrim

Replying to tscrim:

Replying to chapoton:

not a good idea in general to use a bare except:

except:

rather find what exception it is supposed to catch

Right, good catch. I'm going to try and find the precise error that gets raised, but I've pushed a hack fix for now.

It only throws an AttributeError, and I have changed it to catch only that.

comment:259 Changed 12 months ago by stumpc5

Thanks for the fixes, I am doing another round of doctests now...

comment:260 follow-up: Changed 12 months ago by stumpc5

okay, the only remaining failures are

sage -t --long src/sage/tests/cmdline.py  # 1 doctest failed
sage -t --long src/sage/repl/interpreter.py  # 1 doctest failed
sage -t --long src/sage/parallel/map_reduce.py  # 1 doctest failed
sage -t --long src/sage/interfaces/gap3.py  # 5 doctests failed
sage -t --long src/sage/repl/interface_magic.py  # 1 doctest failed

of which we are only responsible for gap3.py. I'd thus suggest to set this to positive review...

comment:261 in reply to: ↑ 260 ; follow-up: Changed 12 months ago by hivert

Replying to stumpc5:

okay, the only remaining failures are

sage -t --long src/sage/tests/cmdline.py  # 1 doctest failed
sage -t --long src/sage/repl/interpreter.py  # 1 doctest failed
sage -t --long src/sage/parallel/map_reduce.py  # 1 doctest failed
sage -t --long src/sage/interfaces/gap3.py  # 5 doctests failed
sage -t --long src/sage/repl/interface_magic.py  # 1 doctest failed

of which we are only responsible for gap3.py. I'd thus suggest to set this to positive review...

Hi travis,

Can you send me the log of the map_reduce.py error ? On what kinds of machine was it ? Is it reproducible ?

Florent

comment:262 in reply to: ↑ 261 Changed 12 months ago by stumpc5

Replying to hivert:

Replying to stumpc5:

okay, the only remaining failures are

sage -t --long src/sage/tests/cmdline.py  # 1 doctest failed
sage -t --long src/sage/repl/interpreter.py  # 1 doctest failed
sage -t --long src/sage/parallel/map_reduce.py  # 1 doctest failed
sage -t --long src/sage/interfaces/gap3.py  # 5 doctests failed
sage -t --long src/sage/repl/interface_magic.py  # 1 doctest failed

of which we are only responsible for gap3.py. I'd thus suggest to set this to positive review...

Hi travis,

...christian.

Can you send me the log of the map_reduce.py error ? On what kinds of machine was it ? Is it reproducible ?

I didn't log the test, but I can restart it now. If it stays, I will give you the details.

comment:263 follow-up: Changed 12 months ago by stumpc5

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

comment:264 in reply to: ↑ 263 ; follow-up: Changed 12 months ago by 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.

comment:265 in reply to: ↑ 264 Changed 12 months ago by chapoton

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

comment:266 Changed 12 months ago by git

  • Commit changed from 9b0ef927c17789f2dbeea432b8316f99d138cb88 to 8d765204ab597ed2fd1570c2438d5fda3eb60192

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

comment:267 Changed 12 months ago by stumpc5

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.)

comment:268 Changed 12 months ago by chapoton

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

Last edited 12 months ago by chapoton (previous) (diff)

comment:269 Changed 12 months ago by stumpc5

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.

comment:270 Changed 12 months ago by chapoton

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

comment:271 Changed 12 months ago by stumpc5

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

comment:272 Changed 12 months ago by git

  • Commit changed from 8d765204ab597ed2fd1570c2438d5fda3eb60192 to 0b6d895c865ebdb319de6d58e862e7ae1a1cb784

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

0b6d895Merge branch 'develop' into public/11187

comment:273 Changed 12 months ago by tscrim

  • Authors changed from Christian Stump to Christian Stump, Frédéric Chapoton, Nicolas Thiery, Travis Scrimshaw
  • Reviewers set to Christian Stump, Frédéric Chapoton, Nicolas Thiery, Vivian Ripoll, Travis Scrimshaw

comment:274 Changed 12 months ago by tscrim

  • Authors changed from Christian Stump, Frédéric Chapoton, Nicolas Thiery, Travis Scrimshaw to Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Travis Scrimshaw
  • Reviewers changed 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

comment:275 Changed 12 months ago by chapoton

  • Authors changed 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
  • Reviewers changed 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

comment:276 Changed 12 months ago by chapoton

  • Status changed from needs_review to positive_review

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

comment:277 Changed 12 months ago by stumpc5

Great -- thanks to all of you for participating!

comment:278 Changed 12 months ago by vbraun

  • Branch changed from public/11187 to 0b6d895c865ebdb319de6d58e862e7ae1a1cb784
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.