#11187 closed enhancement (fixed)
Implementation of finite reflection groups
Reported by: | Christian Stump | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-7.2 |
Component: | combinatorics | Keywords: | reflection group, days49, days64.5, days80 |
Cc: | Vivien Ripoll, Kevin Dilks, Simon King, Jean Michel, Florent 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, GitHub, GitLab) | 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 12 years ago by
Component: | PLEASE CHANGE → combinatorics |
---|---|
Status: | new → needs_work |
comment:2 Changed 12 years ago by
comment:3 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
Apply only trac_11187-finite_reflection_groups-cs.patch
comment:4 Changed 12 years ago by
Dependencies: | → 8327 |
---|---|
Description: | modified (diff) |
comment:5 Changed 12 years ago by
Status: | needs_review → needs_work |
---|
comment:7 Changed 12 years ago by
Dependencies: | 8327 → #8327 |
---|
comment:8 Changed 12 years ago by
Milestone: | → sage-4.7.2 |
---|
Changed 10 years ago by
Attachment: | trac_11187-finite_reflection_groups-cs.patch added |
---|
comment:9 Changed 10 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 10 years ago by
Dependencies: | #8327 → #8327 #14137 |
---|
I've factored out the CartanMatrix
class into #14137.
comment:11 follow-up: 12 Changed 10 years ago by
Keywords: | days49 added |
---|
comment:12 Changed 10 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 9 years ago by
Milestone: | sage-5.11 → sage-5.12 |
---|
comment:14 Changed 9 years ago by
Milestone: | sage-6.1 → sage-6.2 |
---|
comment:15 Changed 9 years ago by
Milestone: | sage-6.2 → sage-6.3 |
---|
comment:16 follow-up: 17 Changed 9 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 Changed 9 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 9 years ago by
Hello, no, no real need for this code, just asking a general question. Now that #10963 is over.
comment:19 Changed 9 years ago by
Branch: | → public/groups/finite_reflection_groups-11187 |
---|---|
Commit: | → 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 9 years ago by
Commit: | 1f3fe1e0caa28a1cb6e2d17a61d94c89460e2bd4 → b60e61b6695fb1cde48d867390d50c32569fc2b0 |
---|
comment:21 Changed 9 years ago by
Commit: | b60e61b6695fb1cde48d867390d50c32569fc2b0 → 1de360ff9e9c4db57e7de8b4ea45bbc4807e243c |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
1de360f | trac #11187 crystallo takes 2 l
|
comment:22 Changed 9 years ago by
Commit: | 1de360ff9e9c4db57e7de8b4ea45bbc4807e243c → deb13e3c50d259c1708aaa815300d94045a14b07 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
deb13e3 | trac #11187 trying to make doctest pass
|
comment:23 Changed 9 years ago by
Commit: | deb13e3c50d259c1708aaa815300d94045a14b07 → b6c1a6273f96f1b5d6103b3090eee3633b2f54b2 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
b6c1a62 | trac #11187 a few more corrections
|
comment:24 Changed 9 years ago by
Commit: | b6c1a6273f96f1b5d6103b3090eee3633b2f54b2 → 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 9 years ago by
Commit: | becb1cab8eacec6e60661677fb2c1fcfe2429afe → e97998c532abc75eec6b18fecff6769601a1bade |
---|
comment:26 Changed 9 years ago by
Commit: | e97998c532abc75eec6b18fecff6769601a1bade → 03111c8373909a6f4ac8e7c8b63625c74b4077a5 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
03111c8 | trac #11187 a little pyflakes cleanup
|
comment:27 Changed 8 years ago by
Milestone: | sage-6.3 → sage-6.4 |
---|
comment:28 Changed 8 years ago by
Commit: | 03111c8373909a6f4ac8e7c8b63625c74b4077a5 → 4a37eac384b296acde87af5bddbe39a4ba8addf0 |
---|
comment:29 Changed 8 years ago by
Commit: | 4a37eac384b296acde87af5bddbe39a4ba8addf0 → 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 8 years ago by
Cc: | Vivien Ripoll added |
---|
comment:31 Changed 8 years ago by
Commit: | 527cb502a4f3729472344a7fbb894502ddd03050 → f2fbe0ad1c694936e6d15195055c261aadc17e1d |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
f2fbe0a | merged in 6.8.beta2
|
comment:32 Changed 8 years ago by
Branch: | public/groups/finite_reflection_groups-11187 → u/stumpc5/11187 |
---|
comment:33 Changed 8 years ago by
Commit: | f2fbe0ad1c694936e6d15195055c261aadc17e1d → ba7b5f092f41da9bbdb18455ecc5af2623dc42cc |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
ba7b5f0 | made Sage not crash on startup
|
comment:34 Changed 8 years ago by
Cc: | Kevin Dilks added |
---|
comment:35 Changed 8 years ago by
Branch: | u/stumpc5/11187 → u/stumpc5/11187-new |
---|
comment:36 Changed 8 years ago by
Commit: | ba7b5f092f41da9bbdb18455ecc5af2623dc42cc → 6ca40fa7459d11bf9c3564777d962a7493eb476c |
---|
comment:37 Changed 8 years ago by
Commit: | 6ca40fa7459d11bf9c3564777d962a7493eb476c → 33f93c6061d90dd3849525f63aa7ffaa21049492 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
33f93c6 | removed the magma iterator
|
comment:38 Changed 8 years ago by
Commit: | 33f93c6061d90dd3849525f63aa7ffaa21049492 → 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 8 years ago by
Commit: | 08a782138c922172d901834b691a80ec850f9bb2 → 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 8 years ago by
Commit: | 014fec229c73bb16da7825d24633c47076f22c4c → 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 8 years ago by
Commit: | d3198f92cb72fd0ca67c4118ddcb00289143bd62 → 8db1deeed4893580fd7c88d42d2fb9366a895bcf |
---|
comment:42 Changed 8 years ago by
Commit: | 8db1deeed4893580fd7c88d42d2fb9366a895bcf → 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 8 years ago by
Cc: | Simon King added |
---|
comment:44 Changed 8 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 8 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 8 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 8 years ago by
The iterators currently cache the list of all elements of the group. Is that (always) a good idea?
comment:48 Changed 8 years ago by
Branch: | u/stumpc5/11187-new → u/SimonKing/11187-new |
---|
comment:49 Changed 8 years ago by
Commit: | 425ca38c084a51fa56c301153165269031fb5e0d → 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 8 years ago by
Branch: | u/SimonKing/11187-new → u/stumpc5/11187-new |
---|
comment:51 Changed 8 years ago by
Commit: | c0a036b2e2f01b1be6a7d004373fafe0c399e14d → 0639665e4f6ac2bee4a4cc9dfcaed5963d910155 |
---|---|
Milestone: | sage-6.4 → sage-6.8 |
comment:52 Changed 8 years ago by
Commit: | 0639665e4f6ac2bee4a4cc9dfcaed5963d910155 → 28f8abd2eb45700978592f1781a1f87e3c4d5e69 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
28f8abd | added a test for well generated
|
comment:53 Changed 8 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 8 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 8 years ago by
Commit: | 28f8abd2eb45700978592f1781a1f87e3c4d5e69 → 697cd3f3077ef749bee642e0281b24fc66c3cbeb |
---|
comment:56 follow-up: 58 Changed 8 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 8 years ago by
Commit: | 697cd3f3077ef749bee642e0281b24fc66c3cbeb → 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 Changed 8 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 8 years ago by
Commit: | 92ab198593eb2d772c48c4cfb80408d6da1daaa9 → 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 8 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 8 years ago by
Commit: | abc690c06e9f6b60bde44f689259d3787d5681ea → 742c0df5916cdd39d843553858f295df9a1ed18e |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
742c0df | replaced '... ' by '....:'
|
comment:62 Changed 8 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 8 years ago by
Commit: | 742c0df5916cdd39d843553858f295df9a1ed18e → 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 8 years ago by
Dependencies: | #8327 #14137 → #8327 #14137 #18590 |
---|
comment:65 Changed 8 years ago by
Dependencies: | #8327 #14137 #18590 → #8327 #14137 #18590 #18597 |
---|
comment:66 Changed 8 years ago by
Commit: | df37b7e9dbec9913ad652826e165e7f534ded0d3 → 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 8 years ago by
Commit: | e180c3133fb8c90f941b3f6f4be8a10e0bb15065 → 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 8 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 8 years ago by
Commit: | 450dadd370f2a8b1e6d05630d669742df3a1beef → e8e0d9ae9e5770b2088d6fb9ee6db412d21353a2 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
e8e0d9a | added two doctests
|
comment:70 Changed 8 years ago by
Dependencies: | #8327 #14137 #18590 #18597 → #8327 #14137 #18590 #18597 #18620 |
---|
comment:71 Changed 8 years ago by
Dependencies: | #8327 #14137 #18590 #18597 #18620 → #18620 |
---|
comment:72 Changed 8 years ago by
Commit: | e8e0d9ae9e5770b2088d6fb9ee6db412d21353a2 → 80b9a54c6f35f6204fd226df8e95df45b05435d3 |
---|
comment:73 Changed 8 years ago by
Commit: | 80b9a54c6f35f6204fd226df8e95df45b05435d3 → 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 8 years ago by
Commit: | d98ee4ecfcdc337ba74ec9a008dd58a17239cfde → 294d5f8f513b74bae5de22221b069a21df1ed6f4 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
294d5f8 | fixed a docstring
|
comment:75 Changed 8 years ago by
Commit: | 294d5f8f513b74bae5de22221b069a21df1ed6f4 → eddaef5ada286366d9dc8d1f5af329f915c9da76 |
---|
comment:76 Changed 8 years ago by
Commit: | eddaef5ada286366d9dc8d1f5af329f915c9da76 → fb852f57be8fe8897ee2c2bad7b4847c865476e8 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
fb852f5 | added back the references to __init__
|
comment:77 Changed 8 years ago by
Commit: | fb852f57be8fe8897ee2c2bad7b4847c865476e8 → 3783ea23dc6a102eea745f59cd2a6b48fb87d5ca |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
3783ea2 | added some stuff that got lost during rebasing
|
comment:78 Changed 8 years ago by
Commit: | 3783ea23dc6a102eea745f59cd2a6b48fb87d5ca → 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 8 years ago by
Commit: | 43f1dcedca38ef310502c66f4bc9147040d2972a → 2f737ce0ee193dc7de8b89586dc3939f2686c161 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
2f737ce | got the documentation run again as expected
|
comment:80 Changed 8 years ago by
Commit: | 2f737ce0ee193dc7de8b89586dc3939f2686c161 → 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 8 years ago by
Commit: | 9df16f6d63f7107669e9229211f9520aeb39a7ad → bfd9aa684127059d97777ad55432772322a78ebc |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
bfd9aa6 | added some references to ComplexReflectionGroups
|
comment:82 Changed 8 years ago by
Commit: | bfd9aa684127059d97777ad55432772322a78ebc → e3b047bff2bd12f99a3fbea87f5417cd2c012745 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
e3b047b | added some references to ComplexReflectionGroups
|
comment:83 Changed 8 years ago by
Description: | modified (diff) |
---|---|
Keywords: | days 64.5 added |
comment:84 Changed 8 years ago by
Commit: | e3b047bff2bd12f99a3fbea87f5417cd2c012745 → d6736681a50cfae4bcd58ad601af43860b5733e3 |
---|
comment:85 Changed 8 years ago by
Commit: | d6736681a50cfae4bcd58ad601af43860b5733e3 → ef3cbe7303af54c2390e04404fb6a4c2623b9c3e |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
ef3cbe7 | fixed a docstring
|
comment:86 follow-up: 87 Changed 8 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 Changed 8 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 8 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 8 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 8 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 8 years ago by
Commit: | ef3cbe7303af54c2390e04404fb6a4c2623b9c3e → 40d22b23447993f976e9680aec23d32c4f4c34f4 |
---|
comment:92 Changed 8 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 8 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:95 Changed 8 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 8 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 follow-up: 98 Changed 8 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 follow-up: 99 Changed 8 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 Changed 8 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 8 years ago by
Description: | modified (diff) |
---|
comment:101 Changed 8 years ago by
Commit: | 40d22b23447993f976e9680aec23d32c4f4c34f4 → 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 8 years ago by
Commit: | 1c64c9b599944baabbecea7267854493546687c7 → 43f505667c936dc07ed42a653383e844f2555cab |
---|
comment:103 Changed 8 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 8 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 Changed 8 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 8 years ago by
Commit: | 43f505667c936dc07ed42a653383e844f2555cab → 29a3fa3b0f4f303db3f0ec6c0835a902563a4812 |
---|
comment:107 Changed 8 years ago by
Commit: | 29a3fa3b0f4f303db3f0ec6c0835a902563a4812 → b123a8170cbb025cfdeb6cb783b0b49854848f32 |
---|
comment:110 Changed 8 years ago by
Commit: | b123a8170cbb025cfdeb6cb783b0b49854848f32 → 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 8 years ago by
Commit: | ab724b4cb5560b44007591ef873eb30c840dd08f → 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 7 years ago by
Commit: | 963397d991abbb26605eb76cf7b3a895ba90d0ac → 7fd11b569246a9bc351b31e84c8c65e529b9f4aa |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
7fd11b5 | Merge branch 'develop' into u/stumpc5/11187-new
|
comment:113 Changed 7 years ago by
Dependencies: | #18620 → #18620, #8906 |
---|
comment:114 Changed 7 years ago by
Commit: | 7fd11b569246a9bc351b31e84c8c65e529b9f4aa → 64431778120d52faa686e33c23ac0ac6c9d472a3 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
6443177 | Merge branch 'develop' into u/stumpc5/11187-new
|
comment:115 Changed 7 years ago by
Commit: | 64431778120d52faa686e33c23ac0ac6c9d472a3 → 3f66a41fc017ceaa8806eaba8db27207e511ac2b |
---|
comment:116 Changed 7 years ago by
Commit: | 3f66a41fc017ceaa8806eaba8db27207e511ac2b → dd250eb43799a4d1f7670614611a9d002956955f |
---|
comment:117 Changed 7 years ago by
Commit: | dd250eb43799a4d1f7670614611a9d002956955f → 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 7 years ago by
Commit: | a00d9c7c47a8689219b9ae8ae723c2b4afa890d5 → 8c4204a0d70b13061de5d6602f43692bc627ec24 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
8c4204a | bug fix for the fundamental invariants
|
comment:119 Changed 7 years ago by
Commit: | 8c4204a0d70b13061de5d6602f43692bc627ec24 → 7bb0e6cca3dd78380cfaa176f5e5384b626cb4b0 |
---|
comment:120 Changed 7 years ago by
Milestone: | sage-6.8 → sage-7.1 |
---|
comment:121 Changed 7 years ago by
Commit: | 7bb0e6cca3dd78380cfaa176f5e5384b626cb4b0 → 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 7 years ago by
Dependencies: | #18620, #8906 → #18620, #8906, #19912 |
---|
comment:123 Changed 7 years ago by
Commit: | 3957a563983601f143659b1a843fc656ca4a0ae3 → 530a5c802620230b461820cb737f59dedb335f16 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
530a5c8 | added two implementations of the invariant forms
|
comment:124 Changed 7 years ago by
Commit: | 530a5c802620230b461820cb737f59dedb335f16 → 8b39c853ac1d85e90e4ed84633c777a2e63ff701 |
---|
comment:125 Changed 7 years ago by
Commit: | 8b39c853ac1d85e90e4ed84633c777a2e63ff701 → aa87d02482826ef4211c06e9fe2ebc13048fc482 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
aa87d02 | added matrix representation for non-well-generated types
|
comment:126 Changed 7 years ago by
Commit: | aa87d02482826ef4211c06e9fe2ebc13048fc482 → 186d38745e7dbde9f619d7152a48bc33065c77ab |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
186d387 | Merge branch 'u/stumpc5/11187-new' into test
|
comment:127 Changed 7 years ago by
Commit: | 186d38745e7dbde9f619d7152a48bc33065c77ab → 094c608b5eda8d16f1b84af1f439614e4f869a2e |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
094c608 | added some more todos for character theory
|
comment:128 Changed 7 years ago by
Dependencies: | #18620, #8906, #19912 → #20107 |
---|
comment:129 follow-up: 130 Changed 7 years ago by
are you aware of Geck's PyCox? http://www.mathematik.uni-stuttgart.de/~geckmf/chv1r6180.py
comment:130 Changed 7 years ago by
Cc: | Jean Michel 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 7 years ago by
Commit: | 094c608b5eda8d16f1b84af1f439614e4f869a2e → 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 7 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 7 years ago by
Branch: | u/stumpc5/11187-new → u/vripoll/11187-new |
---|
comment:134 follow-up: 135 Changed 7 years ago by
Cc: | Florent Hivert added |
---|---|
Commit: | 3da1ebb8a47bdd4ff2c36761b15e663c6d2485fe → e2f919fa6529b2444dbec4cfffde671b8927d1b7 |
Milestone: | sage-7.1 → 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 Changed 7 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 7 years ago by
Branch: | u/vripoll/11187-new → u/stumpc5/11187-new |
---|
comment:137 Changed 7 years ago by
Commit: | e2f919fa6529b2444dbec4cfffde671b8927d1b7 → 48835b7db7daaf22eb903f146140d06b44dda872 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
48835b7 | Merge branch 'develop' into u/stumpc5/11187
|
comment:138 Changed 7 years ago by
Branch: | u/stumpc5/11187-new → u/vripoll/11187-new |
---|
comment:139 Changed 7 years ago by
Commit: | 48835b7db7daaf22eb903f146140d06b44dda872 → 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a |
---|
For the record: as discussed last night, the first goal would be to separate the category stuff into its own ticket (@tscrim 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 7 years ago by
Branch: | u/vripoll/11187-new → u/chapoton/11187 |
---|---|
Commit: | 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a → 0fc36ce6f8ae66d792fd8197313786d961f37fa4 |
comment:141 Changed 7 years ago by
Commit: | 0fc36ce6f8ae66d792fd8197313786d961f37fa4 → 220e0f03ed025daeb6372bee28477882afee6861 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
220e0f0 | trac #11187 changing next to python3 syntax
|
comment:142 Changed 7 years ago by
Commit: | 220e0f03ed025daeb6372bee28477882afee6861 → 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 7 years ago by
Commit: | 3270a98685af6ab39f2c3e13f695a5dd226227d6 → b947ea518e2152eb903d88f459c183a85d3f3587 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
b947ea5 | trac #11187 adding doc for reflecting hyperplane
|
comment:144 Changed 7 years ago by
Commit: | b947ea518e2152eb903d88f459c183a85d3f3587 → 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 7 years ago by
Dependencies: | #20107 → #20107 #20397 |
---|
comment:146 Changed 7 years ago by
Commit: | a697817ab730832f10a06ee6a1ff878be7060dd3 → 9b73808d144d03319e2bde278e07f55f3e34741e |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
9b73808 | trac #11187 correct a doctest in coxeter_groups
|
comment:147 Changed 7 years ago by
Commit: | 9b73808d144d03319e2bde278e07f55f3e34741e → 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 7 years ago by
Commit: | 114b20b8cc80c613f656262d495441edc02a9261 → 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 7 years ago by
Commit: | 4e80c52a448ead8b5072d1ba0a342de1ec116098 → 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 7 years ago by
Commit: | d7343219c7e12dd870605967b9be27222a27a2df → 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 7 years ago by
Commit: | 7d88a6a49bd3f6332e7a3f71b59739d3aeb5d843 → 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 7 years ago by
Branch: | u/chapoton/11187 → u/tscrim/11187 |
---|---|
Commit: | f42aba99d6597cfb7c849900cba284b43816f64e → 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 7 years ago by
Branch: | u/tscrim/11187 → u/vripoll/11187-new |
---|---|
Commit: | e3b52f7780447e5cfb208bfc12bd088f36d3cf47 → 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a |
comment:154 Changed 7 years ago by
Commit: | 1b798a72cdd02cfa90c59ec0f9dd555b8b06e33a → 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 7 years ago by
Branch: | u/vripoll/11187-new → u/stumpc5/11187-new |
---|
comment:156 Changed 7 years ago by
Branch: | u/stumpc5/11187-new → u/tscrim/11187 |
---|---|
Commit: | 2b01e8df1562bb6415635f784f8423cea3af8fd8 → 6daedef45c6888f596b803977897aeec9512e4ac |
Dependencies: | #20107 #20397 → #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 7 years ago by
Branch: | u/tscrim/11187 → u/chapoton/11187 |
---|---|
Commit: | 6daedef45c6888f596b803977897aeec9512e4ac → 5f568f95708bd417947b28de3bf2806d423c947b |
New commits:
5f568f9 | trac #11187 fixing shepard groups
|
comment:158 Changed 7 years ago by
Branch: | u/chapoton/11187 → u/stumpc5/11187 |
---|
comment:159 Changed 7 years ago by
Branch: | u/stumpc5/11187 → u/tscrim/11187 |
---|---|
Commit: | 5f568f95708bd417947b28de3bf2806d423c947b → 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 7 years ago by
Commit: | d4775c7541edc49cc4aaace14edeec15255db700 → beadf8a46bccffc7934b1fd15d607561be937642 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
beadf8a | Lazy imports must have absolute paths.
|
comment:161 Changed 7 years ago by
Commit: | beadf8a46bccffc7934b1fd15d607561be937642 → 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 7 years ago by
Commit: | 185be8f64c6f3d265615b3773c1ef27378a72d2a → b81dc4216eeba5b4d4efa4929f8d2a10c05f7da2 |
---|
comment:163 Changed 7 years ago by
Branch: | u/tscrim/11187 → u/chapoton/11187 |
---|---|
Commit: | b81dc4216eeba5b4d4efa4929f8d2a10c05f7da2 → 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 7 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 7 years ago by
Branch: | u/chapoton/11187 → u/tscrim/11187 |
---|---|
Commit: | 2d70fabf62f9e613ffc9bf488e0dd3e9698ce02c → 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 7 years ago by
Commit: | 5b1f2b0353e134ee7413a9b3dee3ffa3347ed7ae → 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 7 years ago by
Branch: | u/tscrim/11187 → u/nthiery/11187 |
---|
comment:168 Changed 7 years ago by
Branch: | u/nthiery/11187 → u/stumpc5/11187 |
---|---|
Commit: | d4256691756e16c64b6f2f855367f5afb911a00c → 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 7 years ago by
Commit: | e074050131cd483022c5783f749cbce3ac522ae5 → b572c74c3a4db2909bea5c538c8be26089068fc6 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
b572c74 | fixed a missing doctest
|
comment:170 Changed 7 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 7 years ago by
Branch: | u/stumpc5/11187 → u/nthiery/11187 |
---|
comment:172 Changed 7 years ago by
Commit: | b572c74c3a4db2909bea5c538c8be26089068fc6 → 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 7 years ago by
Commit: | 4e4e65ed2613a4486b34219f66a34ba40183cf58 → 6d88d923ee9bbf2a86c223f4aec60ddad4e2eb6f |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
6d88d92 | 11187: oops, forgot to add one file
|
comment:174 Changed 7 years ago by
Branch: | u/nthiery/11187 → public/11187 |
---|
comment:175 Changed 7 years ago by
Commit: | 6d88d923ee9bbf2a86c223f4aec60ddad4e2eb6f → 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 7 years ago by
Commit: | 9f72be34c793f93cb0cc48f03ad7b2849ffabcad → ef835f68696526ba2a9dff934b65e45a1f095219 |
---|
comment:177 Changed 7 years ago by
Commit: | ef835f68696526ba2a9dff934b65e45a1f095219 → a45d823a6544c48bdb903b143338e3d5fbbccd36 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
a45d823 | fixed the allowed input types for reflection groups
|
comment:178 Changed 7 years ago by
Commit: | a45d823a6544c48bdb903b143338e3d5fbbccd36 → 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 7 years ago by
Commit: | a5e263dba2601bb006e2b7510d35c0f656603a8c → 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 7 years ago by
Commit: | a95a5a91f8c164cc863df9103665d90c056e7624 → 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 7 years ago by
Commit: | e654c3ed7751689b839547dec558fc9e375716a7 → 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 7 years ago by
Commit: | c3b5a320e409080e967f44b4d4aa30715b06abfe → b5b8c3b2103a53cc49060ddb584fcd8048e5deab |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
b5b8c3b | trac #11187 adding codegrees to finite Coxeter groups
|
comment:183 Changed 7 years ago by
Commit: | b5b8c3b2103a53cc49060ddb584fcd8048e5deab → 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 7 years ago by
Commit: | 7eab06d2a6f8af4e837342b695c7d479fe140e55 → 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 7 years ago by
Commit: | fae677f17b4af97addae261a4b5882ddd99a19d1 → f048ac3d8b012474f16174ded8a23b5143db9ea0 |
---|
comment:186 Changed 7 years ago by
Commit: | f048ac3d8b012474f16174ded8a23b5143db9ea0 → 7880f309bc060c42fcbdfa8e276011ce0ce88c22 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
7880f30 | trac #11187 one failing doctest in categories
|
comment:187 Changed 7 years ago by
Commit: | 7880f309bc060c42fcbdfa8e276011ce0ce88c22 → 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 7 years ago by
Commit: | 9b7aba28093f4f811acecc67264cb011f029f299 → fc5f6671bf504b8a6ee073b9e7d9aba888220b03 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
fc5f667 | removed several cached methods
|
comment:189 Changed 7 years ago by
Commit: | fc5f6671bf504b8a6ee073b9e7d9aba888220b03 → 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 7 years ago by
Commit: | b55ecb03098c873ac669c5f185c319b0d42fbbca → 2aa89b2f08edb3a5351b610f56ef9a40196f9076 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
2aa89b2 | commenting on the todo list
|
comment:191 Changed 7 years ago by
Commit: | 2aa89b2f08edb3a5351b610f56ef9a40196f9076 → a7d65c6893b0026f2a680fc38fb51168136b0824 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
a7d65c6 | fixed one failing doctest
|
comment:192 Changed 7 years ago by
Commit: | a7d65c6893b0026f2a680fc38fb51168136b0824 → eb45b27d0c5e184572a7719ea22b86c65211349a |
---|
comment:193 Changed 7 years ago by
Commit: | eb45b27d0c5e184572a7719ea22b86c65211349a → f4801f2edbfcf87d1b1302a406945afaae6dbb37 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
f4801f2 | trac 11187 making degrees be integers
|
comment:194 Changed 7 years ago by
Commit: | f4801f2edbfcf87d1b1302a406945afaae6dbb37 → cb0a7a0509d90826d120ae17f108d3ab15ef9b02 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
cb0a7a0 | trac 11187 degrees in tuple
|
comment:195 Changed 7 years ago by
Commit: | cb0a7a0509d90826d120ae17f108d3ab15ef9b02 → 55104aac8bac1952f924ae0198a7c79a7682ee0f |
---|
comment:196 Changed 7 years ago by
Commit: | 55104aac8bac1952f924ae0198a7c79a7682ee0f → fbdabdc58bb28e12f36a8251df683d2b496df619 |
---|
comment:197 Changed 7 years ago by
Commit: | fbdabdc58bb28e12f36a8251df683d2b496df619 → 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 7 years ago by
Commit: | e22ee7ecb32cd5914f6dbb69593d240d4de1e9eb → 722a7675cec4cb22aab3ec594fca457c8ec2da57 |
---|
comment:199 Changed 7 years ago by
Commit: | 722a7675cec4cb22aab3ec594fca457c8ec2da57 → 5294cf3cf5dcfd92d95f606697a45abc9844405b |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
5294cf3 | 11187: fixed codegrees for colored permutations
|
comment:200 Changed 7 years ago by
Commit: | 5294cf3cf5dcfd92d95f606697a45abc9844405b → c7425f8e27bff3d13e05bd9d9a880e14817ccee9 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
c7425f8 | trac 11187 a typo in doc
|
comment:201 Changed 7 years ago by
Commit: | c7425f8e27bff3d13e05bd9d9a880e14817ccee9 → 2def1e83ba323899fa1a95391a1b17a0bda4140c |
---|
comment:202 Changed 7 years ago by
Commit: | 2def1e83ba323899fa1a95391a1b17a0bda4140c → 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 7 years ago by
Commit: | 241e6607cf62f3938019d5118aac018e9254dc24 → 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 7 years ago by
Commit: | 82ff2882bcdee03518224a3b63a0f2ebd1dd33b3 → 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 7 years ago by
Commit: | c09cd4d5e8f61909a57b74dfa09627f6aef6f130 → 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 7 years ago by
Commit: | 0becba3698653fedcfa8547d44fc7d45fb2fecb3 → 8927ab98d9aa88610774567e4bde0ce8ec8c14b9 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
8927ab9 | matrix action is on the right!
|
comment:208 Changed 7 years ago by
@tscrim: 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 7 years ago by
Commit: | 8927ab98d9aa88610774567e4bde0ce8ec8c14b9 → def701588736edda81a86ddd1a9cbdbc849657be |
---|
comment:210 follow-up: 212 Changed 7 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 7 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 Changed 7 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 Changed 7 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 7 years ago by
Commit: | def701588736edda81a86ddd1a9cbdbc849657be → 5a377c42bdcba53fde3504489eab396194bc509f |
---|
comment:215 Changed 7 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 7 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 7 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 Changed 7 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 7 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 Changed 7 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 7 years ago by
Commit: | 5a377c42bdcba53fde3504489eab396194bc509f → 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 follow-up: 224 Changed 7 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 7 years ago by
Commit: | 8862849a09e7f9ecadbb181e5051edf1e9924b73 → ef83323a827a477c3c7330fedb429e4dd737e044 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
ef83323 | clarified the fundamental weights, including an example
|
comment:224 Changed 7 years ago by
Replying to tscrim:
In python3,
I see, doesn't make it easier to read though...
comment:225 Changed 7 years ago by
Commit: | ef83323a827a477c3c7330fedb429e4dd737e044 → 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 7 years ago by
Commit: | 5ecc970883b3ed30314b35949956a7eba135b8ff → 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 7 years ago by
Commit: | 42d6b753c5c5cb4e1e82cf0ed3520768c5dc5d6b → 8810c41ad280c06b307f059220c03f046eafa0d7 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
8810c41 | fixed one doctest + work on the invariant form
|
comment:228 Changed 7 years ago by
Commit: | 8810c41ad280c06b307f059220c03f046eafa0d7 → 2ba374053a128ead4f7288b8b302a2752211a870 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
2ba3740 | fixed another doctest + a typo
|
comment:229 Changed 7 years ago by
Commit: | 2ba374053a128ead4f7288b8b302a2752211a870 → 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 7 years ago by
Commit: | e28bbecbd3a0b08b35f8892874dcd4d7d7f61e4e → dc0106628d58cb263866b9cd601696ea71f18e9f |
---|
comment:231 Changed 7 years ago by
Commit: | dc0106628d58cb263866b9cd601696ea71f18e9f → fd0c9cc616a03de20a12aba530c96e73f8f60911 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
fd0c9cc | added optional gap3
|
comment:232 Changed 7 years ago by
Commit: | fd0c9cc616a03de20a12aba530c96e73f8f60911 → 1537a795f6d7a86f8c3a7a79e86d4b31405de323 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
1537a79 | added optional gap3
|
comment:233 Changed 7 years ago by
Commit: | 1537a795f6d7a86f8c3a7a79e86d4b31405de323 → a60cf31d47777353bcae92665d5e5e739fc02a29 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
a60cf31 | added some missing optional gap3
|
comment:234 Changed 7 years ago by
Commit: | a60cf31d47777353bcae92665d5e5e739fc02a29 → 4955d68d9e4dbde2d1425afbae36e619c2d6ba14 |
---|
comment:235 Changed 7 years ago by
Commit: | 4955d68d9e4dbde2d1425afbae36e619c2d6ba14 → c5c43bcba449039892cecb0d18238b4bf87e8a7f |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
c5c43bc | the next round of optional gap3 insertions
|
comment:236 Changed 7 years ago by
Dependencies: | #20107 |
---|---|
Description: | modified (diff) |
Keywords: | days64.5 days80 added; days 64.5 removed |
Status: | needs_work → needs_review |
comment:237 Changed 7 years ago by
Description: | modified (diff) |
---|
comment:238 Changed 7 years ago by
Commit: | c5c43bcba449039892cecb0d18238b4bf87e8a7f → f541349eae7b8b3b6a61c57aaafd6bb8c84b3265 |
---|
comment:239 Changed 7 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 7 years ago by
Thanks Travis, looking forward to have this to positive review and then merged!
comment:242 Changed 7 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 7 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 7 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 follow-up: 246 Changed 7 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 Changed 7 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 7 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 7 years ago by
Commit: | f541349eae7b8b3b6a61c57aaafd6bb8c84b3265 → 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 7 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 7 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 7 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 7 years ago by
Commit: | 0db28d6c6e64818eb32fb9826ff7a41333c814d6 → 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 7 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 7 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 7 years ago by
Commit: | 7afc02c78f5c0aa58d5b638e20c135678a3bec7e → 02694256c73520e7f4dd80c455cdae133be858a7 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
0269425 | Blanket fix bare except block.
|
comment:256 follow-up: 258 Changed 7 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 7 years ago by
Commit: | 02694256c73520e7f4dd80c455cdae133be858a7 → 9b0ef927c17789f2dbeea432b8316f99d138cb88 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
9b0ef92 | Making it an AttributeError.
|
comment:258 Changed 7 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 7 years ago by
Thanks for the fixes, I am doing another round of doctests now...
comment:260 follow-up: 261 Changed 7 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 follow-up: 262 Changed 7 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 Changed 7 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 7 years ago by
all tests pass this time, sorry for not being able to get you the error...
comment:264 follow-up: 265 Changed 7 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 Changed 7 years ago by
comment:266 Changed 7 years ago by
Commit: | 9b0ef927c17789f2dbeea432b8316f99d138cb88 → 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 7 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 7 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 7 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:271 Changed 7 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 7 years ago by
Commit: | 8d765204ab597ed2fd1570c2438d5fda3eb60192 → 0b6d895c865ebdb319de6d58e862e7ae1a1cb784 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
0b6d895 | Merge branch 'develop' into public/11187
|
comment:273 Changed 7 years ago by
Authors: | Christian Stump → Christian Stump, Frédéric Chapoton, Nicolas Thiery, Travis Scrimshaw |
---|---|
Reviewers: | → Christian Stump, Frédéric Chapoton, Nicolas Thiery, Vivian Ripoll, Travis Scrimshaw |
comment:274 Changed 7 years ago by
Authors: | Christian Stump, Frédéric Chapoton, Nicolas Thiery, Travis Scrimshaw → Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Travis Scrimshaw |
---|---|
Reviewers: | Christian Stump, Frédéric Chapoton, Nicolas Thiery, Vivian Ripoll, Travis Scrimshaw → Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Vivien Ripoll, Travis Scrimshaw |
comment:275 Changed 7 years ago by
Authors: | Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Travis Scrimshaw → Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Travis Scrimshaw |
---|---|
Reviewers: | Christian Stump, Frédéric Chapoton, Nicolas Thiéry, Vivien Ripoll, Travis Scrimshaw → Christian Stump, Frédéric Chapoton, Nicolas M. Thiéry, Vivien Ripoll, Travis Scrimshaw |
comment:276 Changed 7 years ago by
Status: | needs_review → positive_review |
---|
ok, guys, I think it is time. I am setting to positive review
in the name of all authors/reviewers.
comment:278 Changed 7 years ago by
Branch: | public/11187 → 0b6d895c865ebdb319de6d58e862e7ae1a1cb784 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Apply only trac_11187-finite_reflection_groups-cs.patch