Opened 8 years ago
Closed 3 years 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 )
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)
Change History (279)
comment:1 Changed 8 years ago by
- Component changed from PLEASE CHANGE to combinatorics
- Status changed from new to needs_work
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
- Status changed from needs_work to needs_review
Apply only trac_11187-finite_reflection_groups-cs.patch
comment:4 Changed 8 years ago by
- Dependencies set to 8327
- Description modified (diff)
comment:5 Changed 8 years ago by
- Status changed from needs_review to needs_work
comment:6 Changed 8 years ago by
Apply only trac_11187-finite_reflection_groups-cs.patch
comment:7 Changed 8 years ago by
- Dependencies changed from 8327 to #8327
comment:8 Changed 8 years ago by
- Milestone set to sage-4.7.2
Changed 6 years ago by
comment:9 Changed 6 years ago by
+ 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 6 years ago by
- Dependencies changed from #8327 to #8327 #14137
I've factored out the CartanMatrix
class into #14137.
comment:11 follow-up: ↓ 12 Changed 6 years ago by
- Keywords days49 added
comment:12 in reply to: ↑ 11 Changed 6 years ago by
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 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:14 Changed 5 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:15 Changed 5 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:16 follow-up: ↓ 17 Changed 5 years ago by
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 5 years ago by
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 5 years ago by
Hello, no, no real need for this code, just asking a general question. Now that #10963 is over.
comment:19 Changed 5 years ago by
- 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
|
1f3fe1e | Some other fixes from changes to Sage in order to get Sage to start.
|
comment:20 Changed 5 years ago by
- Commit changed from 1f3fe1e0caa28a1cb6e2d17a61d94c89460e2bd4 to b60e61b6695fb1cde48d867390d50c32569fc2b0
comment:21 Changed 5 years ago by
- Commit changed from b60e61b6695fb1cde48d867390d50c32569fc2b0 to 1de360ff9e9c4db57e7de8b4ea45bbc4807e243c
Branch pushed to git repo; I updated commit sha1. New commits:
1de360f | trac #11187 crystallo takes 2 l
|
comment:22 Changed 5 years ago by
- Commit changed from 1de360ff9e9c4db57e7de8b4ea45bbc4807e243c to deb13e3c50d259c1708aaa815300d94045a14b07
Branch pushed to git repo; I updated commit sha1. New commits:
deb13e3 | trac #11187 trying to make doctest pass
|
comment:23 Changed 5 years ago by
- Commit changed from deb13e3c50d259c1708aaa815300d94045a14b07 to b6c1a6273f96f1b5d6103b3090eee3633b2f54b2
Branch pushed to git repo; I updated commit sha1. New commits:
b6c1a62 | trac #11187 a few more corrections
|
comment:24 Changed 5 years ago by
- Commit changed from b6c1a6273f96f1b5d6103b3090eee3633b2f54b2 to becb1cab8eacec6e60661677fb2c1fcfe2429afe
Branch pushed to git repo; I updated commit sha1. New commits:
becb1ca | trac #11187 a few more changes, removing duplicate functions
|
comment:25 Changed 5 years ago by
- Commit changed from becb1cab8eacec6e60661677fb2c1fcfe2429afe to e97998c532abc75eec6b18fecff6769601a1bade
comment:26 Changed 5 years ago by
- Commit changed from e97998c532abc75eec6b18fecff6769601a1bade to 03111c8373909a6f4ac8e7c8b63625c74b4077a5
Branch pushed to git repo; I updated commit sha1. New commits:
03111c8 | trac #11187 a little pyflakes cleanup
|
comment:27 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:28 Changed 4 years ago by
- Commit changed from 03111c8373909a6f4ac8e7c8b63625c74b4077a5 to 4a37eac384b296acde87af5bddbe39a4ba8addf0
comment:29 Changed 4 years ago by
- Commit changed from 4a37eac384b296acde87af5bddbe39a4ba8addf0 to 527cb502a4f3729472344a7fbb894502ddd03050
Branch pushed to git repo; I updated commit sha1. New commits:
527cb50 | Merge branch 'public/groups/finite_reflection_groups-11187' into 6.6.b1
|
comment:30 Changed 4 years ago by
- Cc vripoll added
comment:31 Changed 4 years ago by
- Commit changed from 527cb502a4f3729472344a7fbb894502ddd03050 to f2fbe0ad1c694936e6d15195055c261aadc17e1d
Branch pushed to git repo; I updated commit sha1. New commits:
f2fbe0a | merged in 6.8.beta2
|
comment:32 Changed 4 years ago by
- Branch changed from public/groups/finite_reflection_groups-11187 to u/stumpc5/11187
comment:33 Changed 4 years ago by
- Commit changed from f2fbe0ad1c694936e6d15195055c261aadc17e1d to ba7b5f092f41da9bbdb18455ecc5af2623dc42cc
Branch pushed to git repo; I updated commit sha1. New commits:
ba7b5f0 | made Sage not crash on startup
|
comment:34 Changed 4 years ago by
- Cc kdilks added
comment:35 Changed 4 years ago by
- Branch changed from u/stumpc5/11187 to u/stumpc5/11187-new
comment:36 Changed 4 years ago by
- Commit changed from ba7b5f092f41da9bbdb18455ecc5af2623dc42cc to 6ca40fa7459d11bf9c3564777d962a7493eb476c
comment:37 Changed 4 years ago by
- Commit changed from 6ca40fa7459d11bf9c3564777d962a7493eb476c to 33f93c6061d90dd3849525f63aa7ffaa21049492
Branch pushed to git repo; I updated commit sha1. New commits:
33f93c6 | removed the magma iterator
|
comment:38 Changed 4 years ago by
- Commit changed from 33f93c6061d90dd3849525f63aa7ffaa21049492 to 08a782138c922172d901834b691a80ec850f9bb2
Branch pushed to git repo; I updated commit sha1. New commits:
08a7821 | fixed all doctests in src/sage/combinat/root_system/complex_reflection_group.py
|
comment:39 Changed 4 years ago by
- Commit changed from 08a782138c922172d901834b691a80ec850f9bb2 to 014fec229c73bb16da7825d24633c47076f22c4c
Branch pushed to git repo; I updated commit sha1. New commits:
014fec2 | got the doctests of complex reflection group category pass
|
comment:40 Changed 4 years ago by
- Commit changed from 014fec229c73bb16da7825d24633c47076f22c4c to d3198f92cb72fd0ca67c4118ddcb00289143bd62
Branch pushed to git repo; I updated commit sha1. New commits:
d3198f9 | fixing a few bugs on coxeter number and fundamental invariants
|
comment:41 Changed 4 years ago by
- Commit changed from d3198f92cb72fd0ca67c4118ddcb00289143bd62 to 8db1deeed4893580fd7c88d42d2fb9366a895bcf
comment:42 Changed 4 years ago by
- Commit changed from 8db1deeed4893580fd7c88d42d2fb9366a895bcf to 425ca38c084a51fa56c301153165269031fb5e0d
Branch pushed to git repo; I updated commit sha1. New commits:
425ca38 | first version of coxeter groups from chevie with all tests passing!
|
comment:43 Changed 4 years ago by
- Cc SimonKing added
comment:44 Changed 4 years ago by
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 4 years ago by
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 4 years ago by
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 4 years ago by
The iterators currently cache the list of all elements of the group. Is that (always) a good idea?
comment:48 Changed 4 years ago by
- Branch changed from u/stumpc5/11187-new to u/SimonKing/11187-new
comment:49 Changed 4 years ago by
- 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:
c0a036b | Avoid creation of _reduced_word during iteration
|
comment:50 Changed 4 years ago by
- Branch changed from u/SimonKing/11187-new to u/stumpc5/11187-new
comment:51 Changed 4 years ago by
- Commit changed from c0a036b2e2f01b1be6a7d004373fafe0c399e14d to 0639665e4f6ac2bee4a4cc9dfcaed5963d910155
- Milestone changed from sage-6.4 to sage-6.8
comment:52 Changed 4 years ago by
- Commit changed from 0639665e4f6ac2bee4a4cc9dfcaed5963d910155 to 28f8abd2eb45700978592f1781a1f87e3c4d5e69
Branch pushed to git repo; I updated commit sha1. New commits:
28f8abd | added a test for well generated
|
comment:53 Changed 4 years ago by
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 4 years ago by
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 4 years ago by
- Commit changed from 28f8abd2eb45700978592f1781a1f87e3c4d5e69 to 697cd3f3077ef749bee642e0281b24fc66c3cbeb
comment:56 follow-up: ↓ 58 Changed 4 years ago by
@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
toFiniteCoxeterGroups
, its string repr changes in a not desired way. How should I treat that?
Thanks, Christian
comment:57 Changed 4 years ago by
- Commit changed from 697cd3f3077ef749bee642e0281b24fc66c3cbeb to 92ab198593eb2d772c48c4cfb80408d6da1daaa9
Branch pushed to git repo; I updated commit sha1. New commits:
92ab198 | made the category of coxeter groups a subcategory of complex reflection groups
|
comment:58 in reply to: ↑ 56 Changed 4 years ago by
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 ofFiniteCoxeterGroups
.
- Is that what I should do?
- If I simply add a method
supercategories
toFiniteCoxeterGroups
, 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 4 years ago by
- Commit changed from 92ab198593eb2d772c48c4cfb80408d6da1daaa9 to abc690c06e9f6b60bde44f689259d3787d5681ea
Branch pushed to git repo; I updated commit sha1. New commits:
ec94ae3 | added a few more doctests for CoxeterGroupsChevie
|
1878996 | work in progress: new implementations are presumably correct, but existing bug discovered
|
5716df5 | Fixed according to Travis's suggestions
|
4404133 | Fixed according to Travis's suggestions, and now actually running I think
|
8fe1bb8 | Tab fixed
|
a6dcb8b | merged with now-resolved conflicts
|
c37a5ef | things appear to work in the sense that all tests have passed.
|
1862325 | removed trailing whitespace.
|
1f29ae6 | reordered documentation, as requested by chapoton
|
abc690c | many doctests, and merged #18597
|
comment:60 follow-up: ↓ 62 Changed 4 years ago by
Hello,
- once again: doctest continuation is now
....:
and not...
- could you please put a positive review on the duplicate #9288 ?
comment:61 Changed 4 years ago by
- Commit changed from abc690c06e9f6b60bde44f689259d3787d5681ea to 742c0df5916cdd39d843553858f295df9a1ed18e
Branch pushed to git repo; I updated commit sha1. New commits:
742c0df | replaced '... ' by '....:'
|
comment:62 in reply to: ↑ 60 Changed 4 years ago by
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 4 years ago by
- Commit changed from 742c0df5916cdd39d843553858f295df9a1ed18e to df37b7e9dbec9913ad652826e165e7f534ded0d3
Branch pushed to git repo; I updated commit sha1. New commits:
ab914f5 | first version of the subword complexes as of 2015
|
cb3ca9c | improve the subword complex, in particular its documentation
|
f105602 | Merge branch 'u/stumpc5/11010' into u/stumpc5/11187-new
|
8618845 | playing, I hopefully can reorganize this again
|
ab4999f | more intelligent fix a la stump
|
df37b7e | Merge branch 't/18610/bug__circular_descent_check_in_weylgroups' into u/stumpc5/11187-new
|
comment:64 Changed 4 years ago by
- Dependencies changed from #8327 #14137 to #8327 #14137 #18590
comment:65 Changed 4 years ago by
- Dependencies changed from #8327 #14137 #18590 to #8327 #14137 #18590 #18597
comment:66 Changed 4 years ago by
- Commit changed from df37b7e9dbec9913ad652826e165e7f534ded0d3 to e180c3133fb8c90f941b3f6f4be8a10e0bb15065
Branch pushed to git repo; I updated commit sha1. New commits:
e180c31 | Revert "improve the subword complex, in particular its documentation"
|
comment:67 Changed 4 years ago by
- Commit changed from e180c3133fb8c90f941b3f6f4be8a10e0bb15065 to 450dadd370f2a8b1e6d05630d669742df3a1beef
Branch pushed to git repo; I updated commit sha1. New commits:
450dadd | Revert "first version of the subword complexes as of 2015"
|
comment:68 Changed 4 years ago by
I took out the subword complex stuff which is handled in #11010, and which is depending on this one.
comment:69 Changed 4 years ago by
- Commit changed from 450dadd370f2a8b1e6d05630d669742df3a1beef to e8e0d9ae9e5770b2088d6fb9ee6db412d21353a2
Branch pushed to git repo; I updated commit sha1. New commits:
e8e0d9a | added two doctests
|
comment:70 Changed 4 years ago by
- Dependencies changed from #8327 #14137 #18590 #18597 to #8327 #14137 #18590 #18597 #18620
comment:71 Changed 4 years ago by
- Dependencies changed from #8327 #14137 #18590 #18597 #18620 to #18620
comment:72 Changed 4 years ago by
- Commit changed from e8e0d9ae9e5770b2088d6fb9ee6db412d21353a2 to 80b9a54c6f35f6204fd226df8e95df45b05435d3
comment:73 Changed 4 years ago by
- Commit changed from 80b9a54c6f35f6204fd226df8e95df45b05435d3 to d98ee4ecfcdc337ba74ec9a008dd58a17239cfde
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
a89a90f | removed trailing whitespace.
|
ce30faf | reordered documentation, as requested by chapoton
|
21abe06 | replaced '... ' by '....:'
|
4121914 | improve the subword complex, in particular its documentation
|
b2b7f68 | more intelligent fix a la stump
|
d662af6 | Revert "improve the subword complex, in particular its documentation"
|
1ef2de7 | Revert "first version of the subword complexes as of 2015"
|
03b60e2 | added two doctests
|
2a34bef | added more doctests
|
d98ee4e | added more doctests, many more to come
|
comment:74 Changed 4 years ago by
- Commit changed from d98ee4ecfcdc337ba74ec9a008dd58a17239cfde to 294d5f8f513b74bae5de22221b069a21df1ed6f4
Branch pushed to git repo; I updated commit sha1. New commits:
294d5f8 | fixed a docstring
|
comment:75 Changed 4 years ago by
- Commit changed from 294d5f8f513b74bae5de22221b069a21df1ed6f4 to eddaef5ada286366d9dc8d1f5af329f915c9da76
comment:76 Changed 4 years ago by
- Commit changed from eddaef5ada286366d9dc8d1f5af329f915c9da76 to fb852f57be8fe8897ee2c2bad7b4847c865476e8
Branch pushed to git repo; I updated commit sha1. New commits:
fb852f5 | added back the references to __init__
|
comment:77 Changed 4 years ago by
- Commit changed from fb852f57be8fe8897ee2c2bad7b4847c865476e8 to 3783ea23dc6a102eea745f59cd2a6b48fb87d5ca
Branch pushed to git repo; I updated commit sha1. New commits:
3783ea2 | added some stuff that got lost during rebasing
|
comment:78 Changed 4 years ago by
- Commit changed from 3783ea23dc6a102eea745f59cd2a6b48fb87d5ca to 43f1dcedca38ef310502c66f4bc9147040d2972a
Branch pushed to git repo; I updated commit sha1. New commits:
c07e76e | moved from ComplexReflectionGroup/CoxeterGroupChevie to ReflectionGroup
|
f845089 | change in irreducible components
|
bbe138f | Merge branch 't/18620/9a3e5cdacac40b505f7aad0a62b9e1bec76d8f3a' into u/stumpc5/11187-new
|
43f1dce | reflection groups in the reference
|
comment:79 Changed 4 years ago by
- Commit changed from 43f1dcedca38ef310502c66f4bc9147040d2972a to 2f737ce0ee193dc7de8b89586dc3939f2686c161
Branch pushed to git repo; I updated commit sha1. New commits:
2f737ce | got the documentation run again as expected
|
comment:80 Changed 4 years ago by
- Commit changed from 2f737ce0ee193dc7de8b89586dc3939f2686c161 to 9df16f6d63f7107669e9229211f9520aeb39a7ad
Branch pushed to git repo; I updated commit sha1. New commits:
9df16f6 | cleaned the documentation of the complex reflection group category
|
comment:81 Changed 4 years ago by
- Commit changed from 9df16f6d63f7107669e9229211f9520aeb39a7ad to bfd9aa684127059d97777ad55432772322a78ebc
Branch pushed to git repo; I updated commit sha1. New commits:
bfd9aa6 | added some references to ComplexReflectionGroups
|
comment:82 Changed 4 years ago by
- Commit changed from bfd9aa684127059d97777ad55432772322a78ebc to e3b047bff2bd12f99a3fbea87f5417cd2c012745
Branch pushed to git repo; I updated commit sha1. New commits:
e3b047b | added some references to ComplexReflectionGroups
|
comment:83 Changed 4 years ago by
- Description modified (diff)
- Keywords days 64.5 added
comment:84 Changed 4 years ago by
- Commit changed from e3b047bff2bd12f99a3fbea87f5417cd2c012745 to d6736681a50cfae4bcd58ad601af43860b5733e3
comment:85 Changed 4 years ago by
- Commit changed from d6736681a50cfae4bcd58ad601af43860b5733e3 to ef3cbe7303af54c2390e04404fb6a4c2623b9c3e
Branch pushed to git repo; I updated commit sha1. New commits:
ef3cbe7 | fixed a docstring
|
comment:86 follow-up: ↓ 87 Changed 4 years ago by
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
byReturn
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 4 years ago by
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 4 years ago by
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 4 years ago by
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 4 years ago by
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 4 years ago by
- Commit changed from ef3cbe7303af54c2390e04404fb6a4c2623b9c3e to 40d22b23447993f976e9680aec23d32c4f4c34f4
comment:92 Changed 4 years ago by
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 4 years ago by
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 4 years ago by
Replace
# optional (require chevie)
by
# optional - chevie
comment:95 Changed 4 years ago by
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: ↓ 97 Changed 4 years ago by
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: ↓ 98 Changed 4 years ago by
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: ↓ 99 Changed 4 years ago by
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.
comment:99 in reply to: ↑ 98 Changed 4 years ago by
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 4 years ago by
- Description modified (diff)
comment:101 Changed 4 years ago by
- Commit changed from 40d22b23447993f976e9680aec23d32c4f4c34f4 to 1c64c9b599944baabbecea7267854493546687c7
Branch pushed to git repo; I updated commit sha1. New commits:
5a205a1 | moved right coset representatives
|
25139dd | removed the preloading of the ucf for reflection groups, and moved the new iterator into real reflection groups.
|
7df41c2 | new iterators through group
|
83f9893 | updated licence statements
|
1c64c9b | added a todo for the iterator
|
comment:102 Changed 4 years ago by
- Commit changed from 1c64c9b599944baabbecea7267854493546687c7 to 43f505667c936dc07ed42a653383e844f2555cab
comment:103 Changed 4 years ago by
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: ↓ 105 Changed 4 years ago by
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.
comment:105 in reply to: ↑ 104 Changed 4 years ago by
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 4 years ago by
- Commit changed from 43f505667c936dc07ed42a653383e844f2555cab to 29a3fa3b0f4f303db3f0ec6c0835a902563a4812
comment:107 Changed 4 years ago by
- Commit changed from 29a3fa3b0f4f303db3f0ec6c0835a902563a4812 to b123a8170cbb025cfdeb6cb783b0b49854848f32
comment:108 Changed 4 years ago by
There are a few failing doctests, some easy, some less clear.
comment:109 Changed 4 years ago by
now needs rebase, and care for the many failing doctests
comment:110 Changed 4 years ago by
- Commit changed from b123a8170cbb025cfdeb6cb783b0b49854848f32 to ab724b4cb5560b44007591ef873eb30c840dd08f
Branch pushed to git repo; I updated commit sha1. New commits:
5f8f940 | improved the reduced word for reflection groups
|
711cf2c | 8906: create file ``type`` declaring spkg optional
|
f2777b8 | 8906: create package-version.txt file containing the version
|
c5addcf | 8906: generate checksums.ini
|
af16d79 | 8906: create SPKG.txt file with package description, etc.
|
ddbe1d5 | 8906: create spkg-install script, patches and increment patch level
|
0defcc1 | 8906: add gap3 commandline option to sage startup script
|
ab724b4 | merged latest develop
|
comment:111 Changed 4 years ago by
- Commit changed from ab724b4cb5560b44007591ef873eb30c840dd08f to 963397d991abbb26605eb76cf7b3a895ba90d0ac
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
1c64c9b | added a todo for the iterator
|
d0bfd81 | fixed many doctests coming from new iteration order
|
43f5056 | reached 100% coverage and all tests passing
|
e056f65 | iteration is now stable among machines
|
29a3fa3 | reordered some doctests according to the new iteration order
|
4a13365 | trivialities
|
556ab0e | changed the reduced word computation for real groups
|
b123a81 | hope the iteration order is now stable, also added a few optional chevie, but not many
|
5f8f940 | improved the reduced word for reflection groups
|
963397d | merged develop
|
comment:112 Changed 3 years ago by
- Commit changed from 963397d991abbb26605eb76cf7b3a895ba90d0ac to 7fd11b569246a9bc351b31e84c8c65e529b9f4aa
Branch pushed to git repo; I updated commit sha1. New commits:
7fd11b5 | Merge branch 'develop' into u/stumpc5/11187-new
|
comment:113 Changed 3 years ago by
- Dependencies changed from #18620 to #18620, #8906
comment:114 Changed 3 years ago by
- Commit changed from 7fd11b569246a9bc351b31e84c8c65e529b9f4aa to 64431778120d52faa686e33c23ac0ac6c9d472a3
Branch pushed to git repo; I updated commit sha1. New commits:
6443177 | Merge branch 'develop' into u/stumpc5/11187-new
|
comment:115 Changed 3 years ago by
- Commit changed from 64431778120d52faa686e33c23ac0ac6c9d472a3 to 3f66a41fc017ceaa8806eaba8db27207e511ac2b
comment:116 Changed 3 years ago by
- Commit changed from 3f66a41fc017ceaa8806eaba8db27207e511ac2b to dd250eb43799a4d1f7670614611a9d002956955f
comment:117 Changed 3 years ago by
- Commit changed from dd250eb43799a4d1f7670614611a9d002956955f to a00d9c7c47a8689219b9ae8ae723c2b4afa890d5
Branch pushed to git repo; I updated commit sha1. New commits:
a00d9c7 | sage_eval to eval in one instance, needs to be checked at some point
|
comment:118 Changed 3 years ago by
- Commit changed from a00d9c7c47a8689219b9ae8ae723c2b4afa890d5 to 8c4204a0d70b13061de5d6602f43692bc627ec24
Branch pushed to git repo; I updated commit sha1. New commits:
8c4204a | bug fix for the fundamental invariants
|
comment:119 Changed 3 years ago by
- Commit changed from 8c4204a0d70b13061de5d6602f43692bc627ec24 to 7bb0e6cca3dd78380cfaa176f5e5384b626cb4b0
comment:120 Changed 3 years ago by
- Milestone changed from sage-6.8 to sage-7.1
comment:121 Changed 3 years ago by
- Commit changed from 7bb0e6cca3dd78380cfaa176f5e5384b626cb4b0 to 3957a563983601f143659b1a843fc656ca4a0ae3
Branch pushed to git repo; I updated commit sha1. New commits:
20d254c | fixed the same bug in two more places
|
fa40e5c | Merge branch 'u/stumpc5/19912' into u/stumpc5/11187-new
|
1df9f53 | working on a few glitches for CRGs
|
66ec881 | merged 7.1.beta0
|
3957a56 | Merge branch 'u/stumpc5/19912' into u/stumpc5/11187-new
|
comment:122 Changed 3 years ago by
- Dependencies changed from #18620, #8906 to #18620, #8906, #19912
comment:123 Changed 3 years ago by
- Commit changed from 3957a563983601f143659b1a843fc656ca4a0ae3 to 530a5c802620230b461820cb737f59dedb335f16
Branch pushed to git repo; I updated commit sha1. New commits:
530a5c8 | added two implementations of the invariant forms
|
comment:124 Changed 3 years ago by
- Commit changed from 530a5c802620230b461820cb737f59dedb335f16 to 8b39c853ac1d85e90e4ed84633c777a2e63ff701
comment:125 Changed 3 years ago by
- Commit changed from 8b39c853ac1d85e90e4ed84633c777a2e63ff701 to aa87d02482826ef4211c06e9fe2ebc13048fc482
Branch pushed to git repo; I updated commit sha1. New commits:
aa87d02 | added matrix representation for non-well-generated types
|
comment:126 Changed 3 years ago by
- Commit changed from aa87d02482826ef4211c06e9fe2ebc13048fc482 to 186d38745e7dbde9f619d7152a48bc33065c77ab
Branch pushed to git repo; I updated commit sha1. New commits:
186d387 | Merge branch 'u/stumpc5/11187-new' into test
|
comment:127 Changed 3 years ago by
- Commit changed from 186d38745e7dbde9f619d7152a48bc33065c77ab to 094c608b5eda8d16f1b84af1f439614e4f869a2e
Branch pushed to git repo; I updated commit sha1. New commits:
094c608 | added some more todos for character theory
|
comment:128 Changed 3 years ago by
- Dependencies changed from #18620, #8906, #19912 to #20107
comment:129 follow-up: ↓ 130 Changed 3 years ago by
are you aware of Geck's PyCox? http://www.mathematik.uni-stuttgart.de/~geckmf/chv1r6180.py
comment:130 in reply to: ↑ 129 Changed 3 years ago by
- 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
- http://jlms.oxfordjournals.org/content/90/3/919.abstract (paywall)
- http://arxiv.org/abs/1211.2789 (free)
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 3 years ago by
- Commit changed from 094c608b5eda8d16f1b84af1f439614e4f869a2e to 3da1ebb8a47bdd4ff2c36761b15e663c6d2485fe
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
cac3206 | improved spkg-src
|
181a352 | undoing one thing in spkg-src
|
ec01e30 | moved to experimental package
|
6e599cf | renamed optional package to experimental package in interfaces/gap3.py
|
1583f99 | replaced #20107 by :trac:20107
|
38dbd60 | replaced optional chevie by optional gap3 in combinat/root_system/coxeter_group.py
|
d4f3a1d | updated the spkg-src to delete gap3 packages that need compilation
|
7696a2c | added patch for init.g and fixed typo in patch for gap.sh
|
3d123df | merged 20107 to the newest develop
|
3da1ebb | Merge branch 'u/stumpc5/11187' into tmp
|
comment:132 Changed 3 years ago by
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 3 years ago by
- Branch changed from u/stumpc5/11187-new to u/vripoll/11187-new
comment:134 follow-up: ↓ 135 Changed 3 years ago by
- 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 theCoxeterGroups
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:
e2f919f | New iterator for real reflection groups.
|
comment:135 in reply to: ↑ 134 Changed 3 years ago by
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 3 years ago by
- Branch changed from u/vripoll/11187-new to u/stumpc5/11187-new
comment:137 Changed 3 years ago by
- Commit changed from e2f919fa6529b2444dbec4cfffde671b8927d1b7 to 48835b7db7daaf22eb903f146140d06b44dda872
Branch pushed to git repo; I updated commit sha1. New commits:
48835b7 | Merge branch 'develop' into u/stumpc5/11187
|
comment:138 Changed 3 years ago by
- Branch changed from u/stumpc5/11187-new to u/vripoll/11187-new
comment:139 Changed 3 years ago by
- 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:
e75f1a2 | Updated TODO list
|
1b798a7 | Merge branch 'u/stumpc5/11187-new' of git://trac.sagemath.org/sage into t/11187/11187-new
|
comment:140 Changed 3 years ago by
- Branch changed from u/vripoll/11187-new to u/chapoton/11187
- Commit changed from 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a to 0fc36ce6f8ae66d792fd8197313786d961f37fa4
comment:141 Changed 3 years ago by
- Commit changed from 0fc36ce6f8ae66d792fd8197313786d961f37fa4 to 220e0f03ed025daeb6372bee28477882afee6861
Branch pushed to git repo; I updated commit sha1. New commits:
220e0f0 | trac #11187 changing next to python3 syntax
|
comment:142 Changed 3 years ago by
- Commit changed from 220e0f03ed025daeb6372bee28477882afee6861 to 3270a98685af6ab39f2c3e13f695a5dd226227d6
Branch pushed to git repo; I updated commit sha1. New commits:
3270a98 | trac #11187 remove a method reflection to root in complex groups
|
comment:143 Changed 3 years ago by
- Commit changed from 3270a98685af6ab39f2c3e13f695a5dd226227d6 to b947ea518e2152eb903d88f459c183a85d3f3587
Branch pushed to git repo; I updated commit sha1. New commits:
b947ea5 | trac #11187 adding doc for reflecting hyperplane
|
comment:144 Changed 3 years ago by
- Commit changed from b947ea518e2152eb903d88f459c183a85d3f3587 to a697817ab730832f10a06ee6a1ff878be7060dd3
Branch pushed to git repo; I updated commit sha1. New commits:
a697817 | trac #11187 remove the invariant linear form in complex, for the moment
|
comment:145 Changed 3 years ago by
- Dependencies changed from #20107 to #20107 #20397
comment:146 Changed 3 years ago by
- Commit changed from a697817ab730832f10a06ee6a1ff878be7060dd3 to 9b73808d144d03319e2bde278e07f55f3e34741e
Branch pushed to git repo; I updated commit sha1. New commits:
9b73808 | trac #11187 correct a doctest in coxeter_groups
|
comment:147 Changed 3 years ago by
- Commit changed from 9b73808d144d03319e2bde278e07f55f3e34741e to 114b20b8cc80c613f656262d495441edc02a9261
Branch pushed to git repo; I updated commit sha1. New commits:
114b20b | trac #11187 adding a doctest for the invariant form
|
comment:148 Changed 3 years ago by
- Commit changed from 114b20b8cc80c613f656262d495441edc02a9261 to 4e80c52a448ead8b5072d1ba0a342de1ec116098
Branch pushed to git repo; I updated commit sha1. New commits:
71b77a1 | Merge branch 'develop' into u/tscrim/11187
|
b171922 | Merge branch 'u/stumpc5/11187-new' of git://trac.sagemath.org/sage into u/tscrim/11187
|
67e882a | Trying to get a bit more speed out and doing some code cleanup.
|
c76dd9d | Merge branch 'u/tscrim/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
|
ea14eb8 | fixed some doctests, fixed a bug in hyperplanes, work on the iterator
|
4e80c52 | Merge branch 'u/chapoton/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
|
comment:149 Changed 3 years ago by
- Commit changed from 4e80c52a448ead8b5072d1ba0a342de1ec116098 to d7343219c7e12dd870605967b9be27222a27a2df
Branch pushed to git repo; I updated commit sha1. New commits:
d734321 | trac #11187 a typo in doc in complex reflection groups
|
comment:150 Changed 3 years ago by
- Commit changed from d7343219c7e12dd870605967b9be27222a27a2df to 7d88a6a49bd3f6332e7a3f71b59739d3aeb5d843
Branch pushed to git repo; I updated commit sha1. New commits:
7d88a6a | trac #11187 again a minor doc typo in complex refl group
|
comment:151 Changed 3 years ago by
- Commit changed from 7d88a6a49bd3f6332e7a3f71b59739d3aeb5d843 to f42aba99d6597cfb7c849900cba284b43816f64e
Branch pushed to git repo; I updated commit sha1. New commits:
f42aba9 | trac #11187 a typo in doc of real-refl-groups
|
comment:152 Changed 3 years ago by
- Branch changed from u/chapoton/11187 to u/tscrim/11187
- Commit changed from f42aba99d6597cfb7c849900cba284b43816f64e to e3b52f7780447e5cfb208bfc12bd088f36d3cf47
New commits:
19af4e7 | Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
b7f5ffe | Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
a384a06 | Pulling changes from #11187 and making tests use colored permutations.
|
e3b52f7 | Merge branch 'public/categories/complex_reflection_groups-20397' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
comment:153 Changed 3 years ago by
- Branch changed from u/tscrim/11187 to u/vripoll/11187-new
- Commit changed from e3b52f7780447e5cfb208bfc12bd088f36d3cf47 to 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a
comment:154 Changed 3 years ago by
- Commit changed from 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a to 2b01e8df1562bb6415635f784f8423cea3af8fd8
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
c76dd9d | Merge branch 'u/tscrim/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
|
ea14eb8 | fixed some doctests, fixed a bug in hyperplanes, work on the iterator
|
4e80c52 | Merge branch 'u/chapoton/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
|
d734321 | trac #11187 a typo in doc in complex reflection groups
|
7d88a6a | trac #11187 again a minor doc typo in complex refl group
|
f42aba9 | trac #11187 a typo in doc of real-refl-groups
|
b7f5ffe | Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
a384a06 | Pulling changes from #11187 and making tests use colored permutations.
|
e3b52f7 | Merge branch 'public/categories/complex_reflection_groups-20397' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
2b01e8d | Merge branch 'u/tscrim/11187' of git://trac.sagemath.org/sage into t/11187/11187-new
|
comment:155 Changed 3 years ago by
- Branch changed from u/vripoll/11187-new to u/stumpc5/11187-new
comment:156 Changed 3 years ago by
- 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:
f77e89f | Getting the degrees correct.
|
14d4c39 | Merge branch 'public/20395' into public/categories/complex_reflection_groups-20397
|
79bbb3f | Merge branch 'public/categories/complex_reflection_groups-20397' into u/tscrim/reflection_groups-11187
|
a7c3db8 | Fixing up the categories and doing some other cleanup.
|
b9e0b0d | working on the iterator
|
823ab4f | Merge branch 'u/chapoton/11187' of git://trac.sagemath.org/sage into u/stumpc5/11187
|
80fe997 | finished to encorporate the new iterator
|
3538599 | Merge branch 'u/vripoll/11187-new' of git://trac.sagemath.org/sage into u/stumpc5/11187
|
90244d3 | Merge branch 'u/stumpc5/11187-new' of git://trac.sagemath.org/sage into u/tscrim/reflection_groups-11187
|
6daedef | Fixing everything else up except for well-generated being added to finite Coxeter groups.
|
comment:157 Changed 3 years ago by
- Branch changed from u/tscrim/11187 to u/chapoton/11187
- Commit changed from 6daedef45c6888f596b803977897aeec9512e4ac to 5f568f95708bd417947b28de3bf2806d423c947b
New commits:
5f568f9 | trac #11187 fixing shepard groups
|
comment:158 Changed 3 years ago by
- Branch changed from u/chapoton/11187 to u/stumpc5/11187
comment:159 Changed 3 years ago by
- 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:
a7976cb | removed ComplexReflectionGroups from global namespace
|
d4775c7 | Removing FiniteCoxeterGroups, ComplexReflectionGroups, and GeneralizedWeylGroups from startup import.
|
comment:160 Changed 3 years ago by
- Commit changed from d4775c7541edc49cc4aaace14edeec15255db700 to beadf8a46bccffc7934b1fd15d607561be937642
Branch pushed to git repo; I updated commit sha1. New commits:
beadf8a | Lazy imports must have absolute paths.
|
comment:161 Changed 3 years ago by
- Commit changed from beadf8a46bccffc7934b1fd15d607561be937642 to 185be8f64c6f3d265615b3773c1ef27378a72d2a
Branch pushed to git repo; I updated commit sha1. New commits:
f7a6dd1 | Fixing global namespace issues.
|
a53d8f1 | added somes lines in doc + correction typo shephard
|
4e71ef7 | Some typos and corrections in the descriptions
|
535d37e | removed spaces in example
|
185be8f | Merge branch 'public/categories/complex_reflection_groups-20397' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
comment:162 Changed 3 years ago by
- Commit changed from 185be8f64c6f3d265615b3773c1ef27378a72d2a to b81dc4216eeba5b4d4efa4929f8d2a10c05f7da2
comment:163 Changed 3 years ago by
- Branch changed from u/tscrim/11187 to u/chapoton/11187
- Commit changed from b81dc4216eeba5b4d4efa4929f8d2a10c05f7da2 to 2d70fabf62f9e613ffc9bf488e0dd3e9698ce02c
some tiny fixes in doc, mainly
New commits:
2b0c33a | trac #11187 replace exec by sage_eval, add more doc on reflection groups
|
7271ead | trac #11187 fixing the doc (again)
|
298cf98 | trac #1118è adding one doc
|
dffa31e | trac #11187 some more fine doc tuning
|
2ec03b0 | trac #11187 working on doc
|
7324754 | trac #11187 still working on doc
|
2d70fab | trac #11187 more doc tuning
|
comment:164 Changed 3 years ago by
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 3 years ago by
- Branch changed from u/chapoton/11187 to u/tscrim/11187
- Commit changed from 2d70fabf62f9e613ffc9bf488e0dd3e9698ce02c to 5b1f2b0353e134ee7413a9b3dee3ffa3347ed7ae
New commits:
0bcce62 | Making the iterator even faster.
|
a04b9c8 | Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
9c96d33 | added ST type casting for real groups + documentation of crg's
|
5b1f2b0 | Merge branch 'u/stumpc5/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
comment:166 Changed 3 years ago by
- Commit changed from 5b1f2b0353e134ee7413a9b3dee3ffa3347ed7ae to d4256691756e16c64b6f2f855367f5afb911a00c
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
aa07666 | trac #11187 fixing catalog
|
b2ba34c | Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
fa1b496 | Changing the indexing sets and how they are handled.
|
a1b61e6 | moderate pep8 changes
|
6da6f94 | Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
85d0ce1 | fixed a typo in the doc
|
ab7afba | marked a missing doctest + fixed a bug to show type C2
|
72082b1 | fixed a bug in noncrossing partition lattice and reflection/absolute order
|
844f450 | Merge branch 'u/stumpc5/11187' of trac.sagemath.org:sage into u/tscrim/reflection_groups-11187
|
d425669 | Fixing failures and some other things.
|
comment:167 Changed 3 years ago by
- Branch changed from u/tscrim/11187 to u/nthiery/11187
comment:168 Changed 3 years ago by
- Branch changed from u/nthiery/11187 to u/stumpc5/11187
- Commit changed from d4256691756e16c64b6f2f855367f5afb911a00c to e074050131cd483022c5783f749cbce3ac522ae5
New commits:
87cd989 | fixed one doctest in category complex reflection group
|
05a8ab4 | 11187: fixed trivial doctest failures
|
32bf1ae | 11187: cleanup of the organization of the various axioms (WellGenerated, ...) for complex/generalized reflection groups + documentation improvements
|
73276ca | Merge branch 'u/tscrim/11187' of trac.sagemath.org:sage into t/11187/11187
|
e074050 | merged
|
comment:169 Changed 3 years ago by
- Commit changed from e074050131cd483022c5783f749cbce3ac522ae5 to b572c74c3a4db2909bea5c538c8be26089068fc6
Branch pushed to git repo; I updated commit sha1. New commits:
b572c74 | fixed a missing doctest
|
comment:170 Changed 3 years ago by
Only two trivial doctests failing + the missing doctest for _test_well_generated
, though Nicolas thought he had added it.
comment:171 Changed 3 years ago by
- Branch changed from u/stumpc5/11187 to u/nthiery/11187
comment:172 Changed 3 years ago by
- Commit changed from b572c74c3a4db2909bea5c538c8be26089068fc6 to 4e4e65ed2613a4486b34219f66a34ba40183cf58
Branch pushed to git repo; I updated commit sha1. New commits:
24191b0 | 11187: factored out common code from GeneralizedCoxeterGroups and ComplexReflectionGroups
|
fced757 | trac #11187 fixing import of QQbar and prod
|
1daeaea | trac #11187 fixing degrees for finite Coxeter and Weyl
|
2791d1e | Merge branch 'u/chapoton/11187' of trac.sagemath.org:sage into t/11187/11187
|
4e4e65e | 11187: finished the generic irreducible_components; fixed trivial doctest failures
|
comment:173 Changed 3 years ago by
- Commit changed from 4e4e65ed2613a4486b34219f66a34ba40183cf58 to 6d88d923ee9bbf2a86c223f4aec60ddad4e2eb6f
Branch pushed to git repo; I updated commit sha1. New commits:
6d88d92 | 11187: oops, forgot to add one file
|
comment:174 Changed 3 years ago by
- Branch changed from u/nthiery/11187 to public/11187
comment:175 Changed 3 years ago by
- Commit changed from 6d88d923ee9bbf2a86c223f4aec60ddad4e2eb6f to 9f72be34c793f93cb0cc48f03ad7b2849ffabcad
Branch pushed to git repo; I updated commit sha1. New commits:
9f72be3 | Moving the iterator for complex reflection groups to the Cython file.
|
comment:176 Changed 3 years ago by
- Commit changed from 9f72be34c793f93cb0cc48f03ad7b2849ffabcad to ef835f68696526ba2a9dff934b65e45a1f095219
comment:177 Changed 3 years ago by
- Commit changed from ef835f68696526ba2a9dff934b65e45a1f095219 to a45d823a6544c48bdb903b143338e3d5fbbccd36
Branch pushed to git repo; I updated commit sha1. New commits:
a45d823 | fixed the allowed input types for reflection groups
|
comment:178 Changed 3 years ago by
- Commit changed from a45d823a6544c48bdb903b143338e3d5fbbccd36 to a5e263dba2601bb006e2b7510d35c0f656603a8c
Branch pushed to git repo; I updated commit sha1. New commits:
a9d858a | 11187: documentation, crosslinks, full subcategories, uniformization of simple reflections/reflections/distinguished reflections
|
211baa4 | Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
|
a5e263d | Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
|
comment:179 Changed 3 years ago by
- Commit changed from a5e263dba2601bb006e2b7510d35c0f656603a8c to a95a5a91f8c164cc863df9103665d90c056e7624
Branch pushed to git repo; I updated commit sha1. New commits:
a95a5a9 | 11187: updated doctest + added comment for the next time
|
comment:180 Changed 3 years ago by
- Commit changed from a95a5a91f8c164cc863df9103665d90c056e7624 to e654c3ed7751689b839547dec558fc9e375716a7
Branch pushed to git repo; I updated commit sha1. New commits:
e654c3e | 11187: updated doctests: Coxeter groups don't define additional structure since they have now a supercategory doing so
|
comment:181 Changed 3 years ago by
- Commit changed from e654c3ed7751689b839547dec558fc9e375716a7 to c3b5a320e409080e967f44b4d4aa30715b06abfe
Branch pushed to git repo; I updated commit sha1. New commits:
c3b5a32 | fixed a bug in the degrees in coxeter groups category
|
comment:182 Changed 3 years ago by
- Commit changed from c3b5a320e409080e967f44b4d4aa30715b06abfe to b5b8c3b2103a53cc49060ddb584fcd8048e5deab
Branch pushed to git repo; I updated commit sha1. New commits:
b5b8c3b | trac #11187 adding codegrees to finite Coxeter groups
|
comment:183 Changed 3 years ago by
- Commit changed from b5b8c3b2103a53cc49060ddb584fcd8048e5deab to 7eab06d2a6f8af4e837342b695c7d479fe140e55
Branch pushed to git repo; I updated commit sha1. New commits:
7eab06d | trac #11187 some typos in doc of categories/complex_reflection_group
|
comment:184 Changed 3 years ago by
- Commit changed from 7eab06d2a6f8af4e837342b695c7d479fe140e55 to fae677f17b4af97addae261a4b5882ddd99a19d1
Branch pushed to git repo; I updated commit sha1. New commits:
98f3c76 | 11187: fixed TODO statement, removed unused import
|
d43a39f | 11187: removed commented out link for some_elements
|
e34c9a7 | 11187: refactor from_reduced_word and apply_*_reflection, cleanup
|
8e08db2 | Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
|
fae677f | 11187: removed 3 spurious empty lines in the doc
|
comment:185 Changed 3 years ago by
- Commit changed from fae677f17b4af97addae261a4b5882ddd99a19d1 to f048ac3d8b012474f16174ded8a23b5143db9ea0
comment:186 Changed 3 years ago by
- Commit changed from f048ac3d8b012474f16174ded8a23b5143db9ea0 to 7880f309bc060c42fcbdfa8e276011ce0ce88c22
Branch pushed to git repo; I updated commit sha1. New commits:
7880f30 | trac #11187 one failing doctest in categories
|
comment:187 Changed 3 years ago by
- Commit changed from 7880f309bc060c42fcbdfa8e276011ce0ce88c22 to 9b7aba28093f4f811acecc67264cb011f029f299
Branch pushed to git repo; I updated commit sha1. New commits:
568d6c4 | 11187: Generalized methods from ComplexReflectionGroup, replace apply_simple_reflection_* by a definition of simple_reflection, documentation, TODOs
|
9b7aba2 | Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
|
comment:188 Changed 3 years ago by
- Commit changed from 9b7aba28093f4f811acecc67264cb011f029f299 to fc5f6671bf504b8a6ee073b9e7d9aba888220b03
Branch pushed to git repo; I updated commit sha1. New commits:
fc5f667 | removed several cached methods
|
comment:189 Changed 3 years ago by
- Commit changed from fc5f6671bf504b8a6ee073b9e7d9aba888220b03 to b55ecb03098c873ac669c5f185c319b0d42fbbca
Branch pushed to git repo; I updated commit sha1. New commits:
b55ecb0 | moved the reduced_word_in_ref for real groups there, removed a few more cached methods, documented a few optional arguments
|
comment:190 Changed 3 years ago by
- Commit changed from b55ecb03098c873ac669c5f185c319b0d42fbbca to 2aa89b2f08edb3a5351b610f56ef9a40196f9076
Branch pushed to git repo; I updated commit sha1. New commits:
2aa89b2 | commenting on the todo list
|
comment:191 Changed 3 years ago by
- Commit changed from 2aa89b2f08edb3a5351b610f56ef9a40196f9076 to a7d65c6893b0026f2a680fc38fb51168136b0824
Branch pushed to git repo; I updated commit sha1. New commits:
a7d65c6 | fixed one failing doctest
|
comment:192 Changed 3 years ago by
- Commit changed from a7d65c6893b0026f2a680fc38fb51168136b0824 to eb45b27d0c5e184572a7719ea22b86c65211349a
comment:193 Changed 3 years ago by
- Commit changed from eb45b27d0c5e184572a7719ea22b86c65211349a to f4801f2edbfcf87d1b1302a406945afaae6dbb37
Branch pushed to git repo; I updated commit sha1. New commits:
f4801f2 | trac 11187 making degrees be integers
|
comment:194 Changed 3 years ago by
- Commit changed from f4801f2edbfcf87d1b1302a406945afaae6dbb37 to cb0a7a0509d90826d120ae17f108d3ab15ef9b02
Branch pushed to git repo; I updated commit sha1. New commits:
cb0a7a0 | trac 11187 degrees in tuple
|
comment:195 Changed 3 years ago by
- Commit changed from cb0a7a0509d90826d120ae17f108d3ab15ef9b02 to 55104aac8bac1952f924ae0198a7c79a7682ee0f
comment:196 Changed 3 years ago by
- Commit changed from 55104aac8bac1952f924ae0198a7c79a7682ee0f to fbdabdc58bb28e12f36a8251df683d2b496df619
comment:197 Changed 3 years ago by
- Commit changed from fbdabdc58bb28e12f36a8251df683d2b496df619 to e22ee7ecb32cd5914f6dbb69593d240d4de1e9eb
Branch pushed to git repo; I updated commit sha1. New commits:
8542562 | 11187: add _test_degrees (not yet finished)
|
256edb7 | Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
|
b29b702 | 11187: reflecting_hyperplanes -> reflection_hyperplanes
|
ebbf9bf | 11187: further work on _test_(co)_degrees; reflecting_hyperplanes -> reflection_hyperplanes
|
249f619 | Merge branch 'public/11187' of trac.sagemath.org:sage into t/11187/11187
|
e22ee7e | 11187: fixed typo in the code of number_of_irreducible_components
|
comment:198 Changed 3 years ago by
- Commit changed from e22ee7ecb32cd5914f6dbb69593d240d4de1e9eb to 722a7675cec4cb22aab3ec594fca457c8ec2da57
comment:199 Changed 3 years ago by
- Commit changed from 722a7675cec4cb22aab3ec594fca457c8ec2da57 to 5294cf3cf5dcfd92d95f606697a45abc9844405b
Branch pushed to git repo; I updated commit sha1. New commits:
5294cf3 | 11187: fixed codegrees for colored permutations
|
comment:200 Changed 3 years ago by
- Commit changed from 5294cf3cf5dcfd92d95f606697a45abc9844405b to c7425f8e27bff3d13e05bd9d9a880e14817ccee9
Branch pushed to git repo; I updated commit sha1. New commits:
c7425f8 | trac 11187 a typo in doc
|
comment:201 Changed 3 years ago by
- Commit changed from c7425f8e27bff3d13e05bd9d9a880e14817ccee9 to 2def1e83ba323899fa1a95391a1b17a0bda4140c
comment:202 Changed 3 years ago by
- Commit changed from 2def1e83ba323899fa1a95391a1b17a0bda4140c to 241e6607cf62f3938019d5118aac018e9254dc24
Branch pushed to git repo; I updated commit sha1. New commits:
241e660 | 11187: sum -> ZZ.sum for consistency when the list of degrees is empty
|
comment:203 Changed 3 years ago by
- Commit changed from 241e6607cf62f3938019d5118aac018e9254dc24 to 82ff2882bcdee03518224a3b63a0f2ebd1dd33b3
Branch pushed to git repo; I updated commit sha1. New commits:
82ff288 | 11187: rename 'reflecting hyperplane' to 'reflection hyperplane' in the docstrings
|
comment:204 Changed 3 years ago by
- Commit changed from 82ff2882bcdee03518224a3b63a0f2ebd1dd33b3 to c09cd4d5e8f61909a57b74dfa09627f6aef6f130
Branch pushed to git repo; I updated commit sha1. New commits:
c09cd4d | 11187: Fixed heisenbug caused by temporarily breaking the implementation of (co)degrees for testing without restoring it after
|
comment:205 Changed 3 years ago by
- Commit changed from c09cd4d5e8f61909a57b74dfa09627f6aef6f130 to 0becba3698653fedcfa8547d44fc7d45fb2fecb3
Branch pushed to git repo; I updated commit sha1. New commits:
cf00014 | trac #20396 adding the invariant linear form for complex refl groups
|
75e8d69 | trac #20396 first try at simpler implementation
|
cdef4c3 | trac #20396 not yet working
|
8013693 | Merge branch 'public/20396' of git://trac.sagemath.org/sage into u/stumpc5/20396
|
e02cf02 | still in trouble about the complex case
|
07bd053 | merged 11187
|
bf2ea92 | fixed the coroots + work on invariant form
|
0becba3 | Merge branch 'public/11187' of git://trac.sagemath.org/sage into u/stumpc5/20396
|
comment:206 Changed 3 years ago by
- Commit changed from 0becba3698653fedcfa8547d44fc7d45fb2fecb3 to 8927ab98d9aa88610774567e4bde0ce8ec8c14b9
Branch pushed to git repo; I updated commit sha1. New commits:
8927ab9 | matrix action is on the right!
|
comment:207 Changed 3 years ago by
Okay, all tests pass!
comment:208 Changed 3 years ago by
@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 3 years ago by
- Commit changed from 8927ab98d9aa88610774567e4bde0ce8ec8c14b9 to def701588736edda81a86ddd1a9cbdbc849657be
comment:210 follow-up: ↓ 212 Changed 3 years ago by
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: ↓ 213 Changed 3 years ago by
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 3 years ago by
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 linefor i in self.noncom[first]
has compared tofor 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 3 years ago by
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 inreflection_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 inrange(n)
if one has gotten it through the iterator. This way, the methodreduced_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()
.
comment:214 Changed 3 years ago by
- Commit changed from def701588736edda81a86ddd1a9cbdbc849657be to 5a377c42bdcba53fde3504489eab396194bc509f
comment:215 Changed 3 years ago by
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: ↓ 218 Changed 3 years ago by
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 3 years ago by
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.
comment:218 in reply to: ↑ 216 Changed 3 years ago by
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: ↓ 220 ↓ 222 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
- Commit changed from 5a377c42bdcba53fde3504489eab396194bc509f to 8862849a09e7f9ecadbb181e5051edf1e9924b73
Branch pushed to git repo; I updated commit sha1. New commits:
8862849 | simple_roots, simple_coroots, and fundamental_weights are now index by the index set
|
comment:222 in reply to: ↑ 219 ; follow-up: ↓ 224 Changed 3 years ago by
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).
comment:223 Changed 3 years ago by
- Commit changed from 8862849a09e7f9ecadbb181e5051edf1e9924b73 to ef83323a827a477c3c7330fedb429e4dd737e044
Branch pushed to git repo; I updated commit sha1. New commits:
ef83323 | clarified the fundamental weights, including an example
|
comment:224 in reply to: ↑ 222 Changed 3 years ago by
Replying to tscrim:
In python3,
I see, doesn't make it easier to read though...
comment:225 Changed 3 years ago by
- Commit changed from ef83323a827a477c3c7330fedb429e4dd737e044 to 5ecc970883b3ed30314b35949956a7eba135b8ff
Branch pushed to git repo; I updated commit sha1. New commits:
5ecc970 | got the action on roots and the inversion set in its left- and right-flavor
|
comment:226 Changed 3 years ago by
- Commit changed from 5ecc970883b3ed30314b35949956a7eba135b8ff to 42d6b753c5c5cb4e1e82cf0ed3520768c5dc5d6b
Branch pushed to git repo; I updated commit sha1. New commits:
42d6b75 | made matrix an alias for to_matrix, as matrix returns a huge permutation matrix of the roots
|
comment:227 Changed 3 years ago by
- Commit changed from 42d6b753c5c5cb4e1e82cf0ed3520768c5dc5d6b to 8810c41ad280c06b307f059220c03f046eafa0d7
Branch pushed to git repo; I updated commit sha1. New commits:
8810c41 | fixed one doctest + work on the invariant form
|
comment:228 Changed 3 years ago by
- Commit changed from 8810c41ad280c06b307f059220c03f046eafa0d7 to 2ba374053a128ead4f7288b8b302a2752211a870
Branch pushed to git repo; I updated commit sha1. New commits:
2ba3740 | fixed another doctest + a typo
|
comment:229 Changed 3 years ago by
- Commit changed from 2ba374053a128ead4f7288b8b302a2752211a870 to e28bbecbd3a0b08b35f8892874dcd4d7d7f61e4e
Branch pushed to git repo; I updated commit sha1. New commits:
e28bbec | edited the length function to always use the reduced word
|
comment:230 Changed 3 years ago by
- Commit changed from e28bbecbd3a0b08b35f8892874dcd4d7d7f61e4e to dc0106628d58cb263866b9cd601696ea71f18e9f
comment:231 Changed 3 years ago by
- Commit changed from dc0106628d58cb263866b9cd601696ea71f18e9f to fd0c9cc616a03de20a12aba530c96e73f8f60911
Branch pushed to git repo; I updated commit sha1. New commits:
fd0c9cc | added optional gap3
|
comment:232 Changed 3 years ago by
- Commit changed from fd0c9cc616a03de20a12aba530c96e73f8f60911 to 1537a795f6d7a86f8c3a7a79e86d4b31405de323
Branch pushed to git repo; I updated commit sha1. New commits:
1537a79 | added optional gap3
|
comment:233 Changed 3 years ago by
- Commit changed from 1537a795f6d7a86f8c3a7a79e86d4b31405de323 to a60cf31d47777353bcae92665d5e5e739fc02a29
Branch pushed to git repo; I updated commit sha1. New commits:
a60cf31 | added some missing optional gap3
|
comment:234 Changed 3 years ago by
- Commit changed from a60cf31d47777353bcae92665d5e5e739fc02a29 to 4955d68d9e4dbde2d1425afbae36e619c2d6ba14
comment:235 Changed 3 years ago by
- Commit changed from 4955d68d9e4dbde2d1425afbae36e619c2d6ba14 to c5c43bcba449039892cecb0d18238b4bf87e8a7f
Branch pushed to git repo; I updated commit sha1. New commits:
c5c43bc | the next round of optional gap3 insertions
|
comment:236 Changed 3 years ago by
- 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 3 years ago by
- Description modified (diff)
comment:238 Changed 3 years ago by
- Commit changed from c5c43bcba449039892cecb0d18238b4bf87e8a7f to f541349eae7b8b3b6a61c57aaafd6bb8c84b3265
comment:239 Changed 3 years ago by
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 3 years ago by
Thanks Travis, looking forward to have this to positive review and then merged!
comment:241 follow-up: ↓ 242 Changed 3 years ago by
FYI - Patchbots seem happy with it.
comment:242 in reply to: ↑ 241 Changed 3 years ago by
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: ↓ 245 Changed 3 years ago by
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 3 years ago by
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: ↓ 246 Changed 3 years ago by
Replying to stumpc5:
To now run the doctests
# optional - gap3
, it seems I cannot use the patchbot anymore. Should I then do insteadsage-patchbot/sage -tp 4 --all --long --optional=gap3myself?
I'd use --optional=gap3,sage
comment:246 in reply to: ↑ 245 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
- Commit changed from f541349eae7b8b3b6a61c57aaafd6bb8c84b3265 to 0db28d6c6e64818eb32fb9826ff7a41333c814d6
Branch pushed to git repo; I updated commit sha1. New commits:
0db28d6 | fixed doctests that have not been run last week!
|
comment:249 Changed 3 years ago by
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 3 years ago by
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 3 years ago by
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 3 years ago by
- Commit changed from 0db28d6c6e64818eb32fb9826ff7a41333c814d6 to 7afc02c78f5c0aa58d5b638e20c135678a3bec7e
Branch pushed to git repo; I updated commit sha1. New commits:
7afc02c | Fixing doctests in combinat/root_system/coxeter_group.py.
|
comment:253 follow-up: ↓ 256 Changed 3 years ago by
not a good idea in general to use a bare except:
except:
rather find what exception it is supposed to catch
comment:254 Changed 3 years ago by
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 3 years ago by
- Commit changed from 7afc02c78f5c0aa58d5b638e20c135678a3bec7e to 02694256c73520e7f4dd80c455cdae133be858a7
Branch pushed to git repo; I updated commit sha1. New commits:
0269425 | Blanket fix bare except block.
|
comment:256 in reply to: ↑ 253 ; follow-up: ↓ 258 Changed 3 years ago by
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 3 years ago by
- Commit changed from 02694256c73520e7f4dd80c455cdae133be858a7 to 9b0ef927c17789f2dbeea432b8316f99d138cb88
Branch pushed to git repo; I updated commit sha1. New commits:
9b0ef92 | Making it an AttributeError.
|
comment:258 in reply to: ↑ 256 Changed 3 years ago by
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 3 years ago by
Thanks for the fixes, I am doing another round of doctests now...
comment:260 follow-up: ↓ 261 Changed 3 years ago by
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: ↓ 262 Changed 3 years ago by
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 failedof 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 3 years ago by
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 failedof 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: ↓ 264 Changed 3 years ago by
all tests pass this time, sorry for not being able to get you the error...
comment:264 in reply to: ↑ 263 ; follow-up: ↓ 265 Changed 3 years ago by
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 3 years ago by
comment:266 Changed 3 years ago by
- Commit changed from 9b0ef927c17789f2dbeea432b8316f99d138cb88 to 8d765204ab597ed2fd1570c2438d5fda3eb60192
Branch pushed to git repo; I updated commit sha1. New commits:
8d76520 | using a cached version self._number_of_reflections to avoid computing it again and again, expecially important in has_descent
|
comment:267 Changed 3 years ago by
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 3 years ago by
Please stop touching anything here if you want this to be positive reviewed soon and not in 4 more years.
comment:269 Changed 3 years ago by
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 3 years ago by
sorry for that, I am not in a good mood..
comment:271 Changed 3 years ago by
I would rebase this to beta5
, run the patchbot and do all doctests with the gap3 flag one more time.
comment:272 Changed 3 years ago by
- Commit changed from 8d765204ab597ed2fd1570c2438d5fda3eb60192 to 0b6d895c865ebdb319de6d58e862e7ae1a1cb784
Branch pushed to git repo; I updated commit sha1. New commits:
0b6d895 | Merge branch 'develop' into public/11187
|
comment:273 Changed 3 years ago by
- Reviewers set to Christian Stump, Frédéric Chapoton, Nicolas Thiery, Vivian Ripoll, Travis Scrimshaw
comment:274 Changed 3 years ago by
- 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 3 years ago by
- 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 3 years ago by
- 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 3 years ago by
Great -- thanks to all of you for participating!
comment:278 Changed 3 years ago by
- Branch changed from public/11187 to 0b6d895c865ebdb319de6d58e862e7ae1a1cb784
- Resolution set to fixed
- Status changed from positive_review to closed
Apply only trac_11187-finite_reflection_groups-cs.patch